Skip to content

Commit

Permalink
[Function] Lock the function when parsing call site info
Browse files Browse the repository at this point in the history
Summary:
DWARF-parsing methods in SymbolFileDWARF which update module state
typically take the module lock. ParseCallEdgesInFunction doesn't do
this, but higher-level locking within lldb::Function (which owns the
storage for parsed call edges) is necessary.

The lack of locking could explain some as-of-yet unreproducible crashes
which occur in Function::GetTailCallingEdges(). In these crashes, the
`m_call_edges` vector is non-empty but contains a nullptr, which
shouldn't be possible. (If this vector is non-empty, it _must_ contain a
non-null unique_ptr.)

This may address rdar://55622443 and rdar://65119458.

Reviewers: jasonmolenda, friss, jingham

Subscribers: aprantl, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D83359
  • Loading branch information
vedantk committed Jul 9, 2020
1 parent 0b72b9d commit 6cfc90b
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 0 deletions.
5 changes: 5 additions & 0 deletions lldb/include/lldb/Symbol/Function.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
#include "lldb/Utility/UserID.h"
#include "llvm/ADT/ArrayRef.h"

#include <mutex>

namespace lldb_private {

class ExecutionContext;
Expand Down Expand Up @@ -655,6 +657,9 @@ class Function : public UserID, public SymbolContextScope {
uint32_t
m_prologue_byte_size; ///< Compute the prologue size once and cache it

std::mutex
m_call_edges_lock; ///< Exclusive lock that controls read/write
/// access to m_call_edges and m_call_edges_resolved.
bool m_call_edges_resolved = false; ///< Whether call site info has been
/// parsed.
std::vector<std::unique_ptr<CallEdge>> m_call_edges; ///< Outgoing call edges.
Expand Down
5 changes: 5 additions & 0 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3844,6 +3844,11 @@ SymbolFileDWARF::CollectCallEdges(ModuleSP module, DWARFDIE function_die) {

std::vector<std::unique_ptr<lldb_private::CallEdge>>
SymbolFileDWARF::ParseCallEdgesInFunction(UserID func_id) {
// ParseCallEdgesInFunction must be called at the behest of an exclusively
// locked lldb::Function instance. Storage for parsed call edges is owned by
// the lldb::Function instance: locking at the SymbolFile level would be too
// late, because the act of storing results from ParseCallEdgesInFunction
// would be racy.
DWARFDIE func_die = GetDIE(func_id.GetID());
if (func_die.IsValid())
return CollectCallEdges(GetObjectFile()->GetModule(), func_die);
Expand Down
2 changes: 2 additions & 0 deletions lldb/source/Symbol/Function.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,8 @@ void Function::GetEndLineSourceInfo(FileSpec &source_file, uint32_t &line_no) {
}

llvm::ArrayRef<std::unique_ptr<CallEdge>> Function::GetCallEdges() {
std::lock_guard<std::mutex> guard(m_call_edges_lock);

if (m_call_edges_resolved)
return m_call_edges;

Expand Down

0 comments on commit 6cfc90b

Please sign in to comment.