Skip to content

[lldb] Fix data race in statusline format handling #142489

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 4, 2025
Merged

[lldb] Fix data race in statusline format handling #142489

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6076f77
[lldb] Fix data race in statusline format handling
JDevlieghere Jun 3, 2025
92662d8
Formatting
JDevlieghere Jun 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions lldb/include/lldb/Core/Debugger.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion lldb/include/lldb/Core/FormatEntity.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,8 @@ struct Entry {
return true;
}

operator bool() const { return type != Type::Invalid; }

std::vector<Entry> &GetChildren();

std::string string;
Expand All @@ -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;
Expand Down
12 changes: 6 additions & 6 deletions lldb/include/lldb/Interpreter/OptionValue.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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); }
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 1 addition & 3 deletions lldb/include/lldb/Target/Language.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 17 additions & 14 deletions lldb/source/Core/Debugger.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -1536,8 +1536,11 @@ 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;
Expand Down
8 changes: 4 additions & 4 deletions lldb/source/Core/Disassembler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1037,8 +1037,8 @@ 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 =
exe_ctx->GetTargetRef().GetDebugger().GetDisassemblyFormat();
format = exe_ctx->GetTargetRef().GetDebugger().GetDisassemblyFormat();
disassembly_format = &format;
} else {
FormatEntity::Parse("${addr}: ", format);
disassembly_format = &format;
Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Core/FormatEntity.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions lldb/source/Core/Statusline.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
6 changes: 3 additions & 3 deletions lldb/source/Interpreter/OptionValue.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks safe.

return {};
}

const RegularExpression *OptionValue::GetRegexValue() const {
Expand Down
8 changes: 4 additions & 4 deletions lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -2078,7 +2078,7 @@ static PluginProperties &GetGlobalPluginProperties() {
return g_settings;
}

const FormatEntity::Entry *CPlusPlusLanguage::GetFunctionNameFormat() const {
FormatEntity::Entry CPlusPlusLanguage::GetFunctionNameFormat() const {
return GetGlobalPluginProperties().GetFunctionNameFormat();
}

Expand Down
6 changes: 3 additions & 3 deletions lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(); }
Expand Down
7 changes: 5 additions & 2 deletions lldb/source/Target/StackFrame.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
12 changes: 8 additions & 4 deletions lldb/source/Target/Thread.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Target/ThreadPlanTracer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion lldb/unittests/Core/DebuggerTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Loading