https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/142489
>From 6076f7778f3f10d7360d8f0b156992809de73094 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jo...@devlieghere.com>
Date: Tue, 3 Jun 2025 10:44:43 -0700
Subject: [PATCH] [lldb] Fix data race in statusline format handling
This fixes a data race between the main thread and the default event
handler thread. The statusline format option value was protected by a
mutex, but it was returned as a pointer, allowing one thread to access
it while another was modifying it.
Avoid the data race by returning format values by value instead of by
pointer.
---
lldb/include/lldb/Core/Debugger.h | 12 ++++----
lldb/include/lldb/Core/FormatEntity.h | 4 ++-
lldb/include/lldb/Interpreter/OptionValue.h | 12 ++++----
lldb/include/lldb/Target/Language.h | 4 +--
lldb/source/Core/Debugger.cpp | 30 ++++++++++---------
lldb/source/Core/Disassembler.cpp | 7 +++--
lldb/source/Core/FormatEntity.cpp | 4 +--
lldb/source/Core/Statusline.cpp | 8 ++---
lldb/source/Interpreter/OptionValue.cpp | 6 ++--
.../Language/CPlusPlus/CPlusPlusLanguage.cpp | 8 ++---
.../Language/CPlusPlus/CPlusPlusLanguage.h | 6 ++--
lldb/source/Target/StackFrame.cpp | 7 +++--
lldb/source/Target/Thread.cpp | 12 +++++---
lldb/source/Target/ThreadPlanTracer.cpp | 4 +--
lldb/unittests/Core/DebuggerTest.cpp | 2 +-
15 files changed, 68 insertions(+), 58 deletions(-)
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index c9e5310cded1a..d73aba1e3ce58 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -243,17 +243,17 @@ class Debugger : public
std::enable_shared_from_this<Debugger>,
bool GetAutoConfirm() const;
- const FormatEntity::Entry *GetDisassemblyFormat() const;
+ FormatEntity::Entry GetDisassemblyFormat() const;
- const FormatEntity::Entry *GetFrameFormat() const;
+ FormatEntity::Entry GetFrameFormat() const;
- const FormatEntity::Entry *GetFrameFormatUnique() const;
+ FormatEntity::Entry GetFrameFormatUnique() const;
uint64_t GetStopDisassemblyMaxSize() const;
- const FormatEntity::Entry *GetThreadFormat() const;
+ FormatEntity::Entry GetThreadFormat() const;
- const FormatEntity::Entry *GetThreadStopFormat() const;
+ FormatEntity::Entry GetThreadStopFormat() const;
lldb::ScriptLanguage GetScriptLanguage() const;
@@ -297,7 +297,7 @@ class Debugger : public
std::enable_shared_from_this<Debugger>,
bool GetShowStatusline() const;
- const FormatEntity::Entry *GetStatuslineFormat() const;
+ FormatEntity::Entry GetStatuslineFormat() const;
bool SetStatuslineFormat(const FormatEntity::Entry &format);
llvm::StringRef GetSeparator() const;
diff --git a/lldb/include/lldb/Core/FormatEntity.h
b/lldb/include/lldb/Core/FormatEntity.h
index 1aed3c6ff9e9d..18257161eec7b 100644
--- a/lldb/include/lldb/Core/FormatEntity.h
+++ b/lldb/include/lldb/Core/FormatEntity.h
@@ -205,6 +205,8 @@ struct Entry {
return true;
}
+ operator bool() const { return type != Type::Invalid; }
+
std::vector<Entry> &GetChildren();
std::string string;
@@ -217,7 +219,7 @@ struct Entry {
size_t level = 0;
/// @}
- Type type;
+ Type type = Type::Invalid;
lldb::Format fmt = lldb::eFormatDefault;
lldb::addr_t number = 0;
bool deref = false;
diff --git a/lldb/include/lldb/Interpreter/OptionValue.h
b/lldb/include/lldb/Interpreter/OptionValue.h
index e3c139155b0ef..f293a3a33bfa0 100644
--- a/lldb/include/lldb/Interpreter/OptionValue.h
+++ b/lldb/include/lldb/Interpreter/OptionValue.h
@@ -284,6 +284,8 @@ class OptionValue {
return GetStringValue();
if constexpr (std::is_same_v<T, ArchSpec>)
return GetArchSpecValue();
+ if constexpr (std::is_same_v<T, FormatEntity::Entry>)
+ return GetFormatEntityValue();
if constexpr (std::is_enum_v<T>)
if (std::optional<int64_t> value = GetEnumerationValue())
return static_cast<T>(*value);
@@ -295,11 +297,9 @@ class OptionValue {
typename std::remove_pointer<T>::type>::type,
std::enable_if_t<std::is_pointer_v<T>, bool> = true>
T GetValueAs() const {
- if constexpr (std::is_same_v<U, FormatEntity::Entry>)
- return GetFormatEntity();
- if constexpr (std::is_same_v<U, RegularExpression>)
- return GetRegexValue();
- return {};
+ static_assert(std::is_same_v<U, RegularExpression>,
+ "only for RegularExpression");
+ return GetRegexValue();
}
bool SetValueAs(bool v) { return SetBooleanValue(v); }
@@ -382,7 +382,7 @@ class OptionValue {
std::optional<UUID> GetUUIDValue() const;
bool SetUUIDValue(const UUID &uuid);
- const FormatEntity::Entry *GetFormatEntity() const;
+ FormatEntity::Entry GetFormatEntityValue() const;
bool SetFormatEntityValue(const FormatEntity::Entry &entry);
const RegularExpression *GetRegexValue() const;
diff --git a/lldb/include/lldb/Target/Language.h
b/lldb/include/lldb/Target/Language.h
index d62871bd7ed70..3d0aa326d5a6d 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -495,9 +495,7 @@ class Language : public PluginInterface {
/// Python uses \b except. Defaults to \b catch.
virtual llvm::StringRef GetCatchKeyword() const { return "catch"; }
- virtual const FormatEntity::Entry *GetFunctionNameFormat() const {
- return nullptr;
- }
+ virtual FormatEntity::Entry GetFunctionNameFormat() const { return {}; }
protected:
// Classes that inherit from Language can see and modify these
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 519a2528ca7e0..de60d25b93112 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -294,19 +294,19 @@ bool Debugger::GetAutoConfirm() const {
idx, g_debugger_properties[idx].default_uint_value != 0);
}
-const FormatEntity::Entry *Debugger::GetDisassemblyFormat() const {
+FormatEntity::Entry Debugger::GetDisassemblyFormat() const {
constexpr uint32_t idx = ePropertyDisassemblyFormat;
- return GetPropertyAtIndexAs<const FormatEntity::Entry *>(idx);
+ return GetPropertyAtIndexAs<FormatEntity::Entry>(idx, {});
}
-const FormatEntity::Entry *Debugger::GetFrameFormat() const {
+FormatEntity::Entry Debugger::GetFrameFormat() const {
constexpr uint32_t idx = ePropertyFrameFormat;
- return GetPropertyAtIndexAs<const FormatEntity::Entry *>(idx);
+ return GetPropertyAtIndexAs<FormatEntity::Entry>(idx, {});
}
-const FormatEntity::Entry *Debugger::GetFrameFormatUnique() const {
+FormatEntity::Entry Debugger::GetFrameFormatUnique() const {
constexpr uint32_t idx = ePropertyFrameFormatUnique;
- return GetPropertyAtIndexAs<const FormatEntity::Entry *>(idx);
+ return GetPropertyAtIndexAs<FormatEntity::Entry>(idx, {});
}
uint64_t Debugger::GetStopDisassemblyMaxSize() const {
@@ -350,14 +350,14 @@ void Debugger::SetPrompt(llvm::StringRef p) {
GetCommandInterpreter().UpdatePrompt(new_prompt);
}
-const FormatEntity::Entry *Debugger::GetThreadFormat() const {
+FormatEntity::Entry Debugger::GetThreadFormat() const {
constexpr uint32_t idx = ePropertyThreadFormat;
- return GetPropertyAtIndexAs<const FormatEntity::Entry *>(idx);
+ return GetPropertyAtIndexAs<FormatEntity::Entry>(idx, {});
}
-const FormatEntity::Entry *Debugger::GetThreadStopFormat() const {
+FormatEntity::Entry Debugger::GetThreadStopFormat() const {
constexpr uint32_t idx = ePropertyThreadStopFormat;
- return GetPropertyAtIndexAs<const FormatEntity::Entry *>(idx);
+ return GetPropertyAtIndexAs<FormatEntity::Entry>(idx, {});
}
lldb::ScriptLanguage Debugger::GetScriptLanguage() const {
@@ -492,9 +492,9 @@ bool Debugger::GetShowStatusline() const {
idx, g_debugger_properties[idx].default_uint_value != 0);
}
-const FormatEntity::Entry *Debugger::GetStatuslineFormat() const {
+FormatEntity::Entry Debugger::GetStatuslineFormat() const {
constexpr uint32_t idx = ePropertyStatuslineFormat;
- return GetPropertyAtIndexAs<const FormatEntity::Entry *>(idx);
+ return GetPropertyAtIndexAs<FormatEntity::Entry>(idx, {});
}
bool Debugger::SetStatuslineFormat(const FormatEntity::Entry &format) {
@@ -1536,8 +1536,10 @@ bool Debugger::FormatDisassemblerAddress(const
FormatEntity::Entry *format,
FormatEntity::Entry format_entry;
if (format == nullptr) {
- if (exe_ctx != nullptr && exe_ctx->HasTargetScope())
- format = exe_ctx->GetTargetRef().GetDebugger().GetDisassemblyFormat();
+ if (exe_ctx != nullptr && exe_ctx->HasTargetScope()) {
+ format_entry =
exe_ctx->GetTargetRef().GetDebugger().GetDisassemblyFormat();
+ format = &format_entry;
+ }
if (format == nullptr) {
FormatEntity::Parse("${addr}: ", format_entry);
format = &format_entry;
diff --git a/lldb/source/Core/Disassembler.cpp
b/lldb/source/Core/Disassembler.cpp
index dce3d59457c0e..6ecbd1f696f44 100644
--- a/lldb/source/Core/Disassembler.cpp
+++ b/lldb/source/Core/Disassembler.cpp
@@ -310,8 +310,8 @@ void Disassembler::PrintInstructions(Debugger &debugger,
const ArchSpec &arch,
const FormatEntity::Entry *disassembly_format = nullptr;
FormatEntity::Entry format;
if (exe_ctx.HasTargetScope()) {
- disassembly_format =
- exe_ctx.GetTargetRef().GetDebugger().GetDisassemblyFormat();
+ format = exe_ctx.GetTargetRef().GetDebugger().GetDisassemblyFormat();
+ disassembly_format = &format;
} else {
FormatEntity::Parse("${addr}: ", format);
disassembly_format = &format;
@@ -1037,8 +1037,9 @@ void InstructionList::Dump(Stream *s, bool show_address,
bool show_bytes,
const FormatEntity::Entry *disassembly_format = nullptr;
FormatEntity::Entry format;
if (exe_ctx && exe_ctx->HasTargetScope()) {
- disassembly_format =
+ format =
exe_ctx->GetTargetRef().GetDebugger().GetDisassemblyFormat();
+ disassembly_format = &format;
} else {
FormatEntity::Parse("${addr}: ", format);
disassembly_format = &format;
diff --git a/lldb/source/Core/FormatEntity.cpp
b/lldb/source/Core/FormatEntity.cpp
index 4dcfa43a7bb04..3c591ba15a075 100644
--- a/lldb/source/Core/FormatEntity.cpp
+++ b/lldb/source/Core/FormatEntity.cpp
@@ -1279,13 +1279,13 @@ static bool FormatFunctionNameForLanguage(Stream &s,
if (!language_plugin)
return false;
- const auto *format = language_plugin->GetFunctionNameFormat();
+ FormatEntity::Entry format = language_plugin->GetFunctionNameFormat();
if (!format)
return false;
StreamString name_stream;
const bool success =
- FormatEntity::Format(*format, name_stream, sc, exe_ctx, /*addr=*/nullptr,
+ FormatEntity::Format(format, name_stream, sc, exe_ctx, /*addr=*/nullptr,
/*valobj=*/nullptr, /*function_changed=*/false,
/*initial_function=*/false);
if (success)
diff --git a/lldb/source/Core/Statusline.cpp b/lldb/source/Core/Statusline.cpp
index 8b3c8d1ccfa80..52f6134123b76 100644
--- a/lldb/source/Core/Statusline.cpp
+++ b/lldb/source/Core/Statusline.cpp
@@ -144,9 +144,9 @@ void Statusline::Redraw(bool update) {
}
StreamString stream;
- if (auto *format = m_debugger.GetStatuslineFormat())
- FormatEntity::Format(*format, stream, &symbol_ctx, &exe_ctx, nullptr,
- nullptr, false, false);
+ FormatEntity::Entry format = m_debugger.GetStatuslineFormat();
+ FormatEntity::Format(format, stream, &symbol_ctx, &exe_ctx, nullptr, nullptr,
+ false, false);
- Draw(std::string(stream.GetString()));
+ Draw(stream.GetString().str());
}
diff --git a/lldb/source/Interpreter/OptionValue.cpp
b/lldb/source/Interpreter/OptionValue.cpp
index 28bc57a07ac71..aa118107f27c5 100644
--- a/lldb/source/Interpreter/OptionValue.cpp
+++ b/lldb/source/Interpreter/OptionValue.cpp
@@ -380,11 +380,11 @@ bool OptionValue::SetLanguageValue(lldb::LanguageType
new_language) {
return false;
}
-const FormatEntity::Entry *OptionValue::GetFormatEntity() const {
+FormatEntity::Entry OptionValue::GetFormatEntityValue() const {
std::lock_guard<std::mutex> lock(m_mutex);
if (const OptionValueFormatEntity *option_value = GetAsFormatEntity())
- return &option_value->GetCurrentValue();
- return nullptr;
+ return option_value->GetCurrentValue();
+ return {};
}
const RegularExpression *OptionValue::GetRegexValue() const {
diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
index 7485577ae67e0..0f18abb47591d 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -2066,9 +2066,9 @@ class PluginProperties : public Properties {
m_collection_sp->Initialize(g_language_cplusplus_properties);
}
- const FormatEntity::Entry *GetFunctionNameFormat() const {
- return GetPropertyAtIndexAs<const FormatEntity::Entry *>(
- ePropertyFunctionNameFormat);
+ FormatEntity::Entry GetFunctionNameFormat() const {
+ return GetPropertyAtIndexAs<FormatEntity::Entry>(
+ ePropertyFunctionNameFormat, {});
}
};
} // namespace
@@ -2078,7 +2078,7 @@ static PluginProperties &GetGlobalPluginProperties() {
return g_settings;
}
-const FormatEntity::Entry *CPlusPlusLanguage::GetFunctionNameFormat() const {
+FormatEntity::Entry CPlusPlusLanguage::GetFunctionNameFormat() const {
return GetGlobalPluginProperties().GetFunctionNameFormat();
}
diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
index 575f76c3101ed..22acdf3e8efe9 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -93,8 +93,8 @@ class CPlusPlusLanguage : public Language {
static llvm::StringRef GetPluginNameStatic() { return "cplusplus"; }
bool SymbolNameFitsToLanguage(Mangled mangled) const override;
-
- bool DemangledNameContainsPath(llvm::StringRef path,
+
+ bool DemangledNameContainsPath(llvm::StringRef path,
ConstString demangled) const override;
ConstString
@@ -136,7 +136,7 @@ class CPlusPlusLanguage : public Language {
llvm::StringRef GetInstanceVariableName() override { return "this"; }
- const FormatEntity::Entry *GetFunctionNameFormat() const override;
+ FormatEntity::Entry GetFunctionNameFormat() const override;
// PluginInterface protocol
llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
diff --git a/lldb/source/Target/StackFrame.cpp
b/lldb/source/Target/StackFrame.cpp
index ab5cd0b27c789..52a54020bc468 100644
--- a/lldb/source/Target/StackFrame.cpp
+++ b/lldb/source/Target/StackFrame.cpp
@@ -1937,12 +1937,15 @@ void StackFrame::DumpUsingSettingsFormat(Stream *strm,
bool show_unique,
ExecutionContext exe_ctx(shared_from_this());
const FormatEntity::Entry *frame_format = nullptr;
+ FormatEntity::Entry format_entry;
Target *target = exe_ctx.GetTargetPtr();
if (target) {
if (show_unique) {
- frame_format = target->GetDebugger().GetFrameFormatUnique();
+ format_entry = target->GetDebugger().GetFrameFormatUnique();
+ frame_format = &format_entry;
} else {
- frame_format = target->GetDebugger().GetFrameFormat();
+ format_entry = target->GetDebugger().GetFrameFormat();
+ frame_format = &format_entry;
}
}
if (!DumpUsingFormat(*strm, frame_format, frame_marker)) {
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index b0e0f1e67c060..c68894808eacc 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -1668,10 +1668,14 @@ void Thread::DumpUsingSettingsFormat(Stream &strm,
uint32_t frame_idx,
ExecutionContext exe_ctx(shared_from_this());
const FormatEntity::Entry *thread_format;
- if (stop_format)
- thread_format = exe_ctx.GetTargetRef().GetDebugger().GetThreadStopFormat();
- else
- thread_format = exe_ctx.GetTargetRef().GetDebugger().GetThreadFormat();
+ FormatEntity::Entry format_entry;
+ if (stop_format) {
+ format_entry = exe_ctx.GetTargetRef().GetDebugger().GetThreadStopFormat();
+ thread_format = &format_entry;
+ } else {
+ format_entry = exe_ctx.GetTargetRef().GetDebugger().GetThreadFormat();
+ thread_format = &format_entry;
+ }
assert(thread_format);
diff --git a/lldb/source/Target/ThreadPlanTracer.cpp
b/lldb/source/Target/ThreadPlanTracer.cpp
index ab63cc7f6c223..c5a9c5b806c30 100644
--- a/lldb/source/Target/ThreadPlanTracer.cpp
+++ b/lldb/source/Target/ThreadPlanTracer.cpp
@@ -174,11 +174,11 @@ void ThreadPlanAssemblyTracer::Log() {
const bool show_control_flow_kind = true;
Instruction *instruction =
instruction_list.GetInstructionAtIndex(0).get();
- const FormatEntity::Entry *disassemble_format =
+ FormatEntity::Entry disassemble_format =
m_process.GetTarget().GetDebugger().GetDisassemblyFormat();
instruction->Dump(stream_sp.get(), max_opcode_byte_size, show_address,
show_bytes, show_control_flow_kind, nullptr, nullptr,
- nullptr, disassemble_format, 0);
+ nullptr, &disassemble_format, 0);
}
}
}
diff --git a/lldb/unittests/Core/DebuggerTest.cpp
b/lldb/unittests/Core/DebuggerTest.cpp
index df7d999788553..4dccd912c63ae 100644
--- a/lldb/unittests/Core/DebuggerTest.cpp
+++ b/lldb/unittests/Core/DebuggerTest.cpp
@@ -46,7 +46,7 @@ TEST_F(DebuggerTest, TestSettings) {
FormatEntity::Entry format("foo");
EXPECT_TRUE(debugger_sp->SetStatuslineFormat(format));
- EXPECT_EQ(debugger_sp->GetStatuslineFormat()->string, "foo");
+ EXPECT_EQ(debugger_sp->GetStatuslineFormat().string, "foo");
Debugger::Destroy(debugger_sp);
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
