Author: Vedant Kumar Date: 2020-07-09T10:37:09-07:00 New Revision: 6cfc90b9b791a023368b9384f57c2c3120894247
URL: https://github.com/llvm/llvm-project/commit/6cfc90b9b791a023368b9384f57c2c3120894247 DIFF: https://github.com/llvm/llvm-project/commit/6cfc90b9b791a023368b9384f57c2c3120894247.diff LOG: [Function] Lock the function when parsing call site info 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 Added: Modified: lldb/include/lldb/Symbol/Function.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Symbol/Function.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Symbol/Function.h b/lldb/include/lldb/Symbol/Function.h index e548b8d626a3..bfaa1a13359c 100644 --- a/lldb/include/lldb/Symbol/Function.h +++ b/lldb/include/lldb/Symbol/Function.h @@ -17,6 +17,8 @@ #include "lldb/Utility/UserID.h" #include "llvm/ADT/ArrayRef.h" +#include <mutex> + namespace lldb_private { class ExecutionContext; @@ -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. diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 7dd084f24e3e..9f64e5255fd5 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -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); diff --git a/lldb/source/Symbol/Function.cpp b/lldb/source/Symbol/Function.cpp index 533fdd465c42..67013f6dd8b1 100644 --- a/lldb/source/Symbol/Function.cpp +++ b/lldb/source/Symbol/Function.cpp @@ -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; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits