From 83cc13b7ef6a4073791e872715d1b3517dd5d193 Mon Sep 17 00:00:00 2001
From: Vedant Kumar <vsk@apple.com>
Date: Tue, 7 Jul 2020 16:54:09 -0700
Subject: [PATCH] [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
(cherry picked from commit 6cfc90b9b791a023368b9384f57c2c3120894247)
---
lldb/include/lldb/Symbol/Function.h | 5 +++++
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 5 +++++
lldb/source/Symbol/Function.cpp | 2 ++
3 files changed, 12 insertions(+)
diff --git a/lldb/include/lldb/Symbol/Function.h b/lldb/include/lldb/Symbol/Function.h
index fc99904610386..ffbbf058eb15e 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;
@@ -636,6 +638,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 a52837992eeec..bd94a3e978175 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -4030,6 +4030,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 558bdb79add29..9411ace85130e 100644
--- a/lldb/source/Symbol/Function.cpp
+++ b/lldb/source/Symbol/Function.cpp
@@ -305,6 +305,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;