Skip to content

Commit 488438e

Browse files
JDevlieghereJDevlieghere
committed
[lldb] Fix data race in statusline format handling (llvm#142489)
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. (cherry picked from commit 6760857)
1 parent eef7ce4 commit 488438e

File tree

15 files changed

+69
-59
lines changed

15 files changed

+69
-59
lines changed

‎lldb/include/lldb/Core/Debugger.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -256,17 +256,17 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
256256

257257
bool GetAutoConfirm() const;
258258

259-
const FormatEntity::Entry *GetDisassemblyFormat() const;
259+
FormatEntity::Entry GetDisassemblyFormat() const;
260260

261-
const FormatEntity::Entry *GetFrameFormat() const;
261+
FormatEntity::Entry GetFrameFormat() const;
262262

263-
const FormatEntity::Entry *GetFrameFormatUnique() const;
263+
FormatEntity::Entry GetFrameFormatUnique() const;
264264

265265
uint64_t GetStopDisassemblyMaxSize() const;
266266

267-
const FormatEntity::Entry *GetThreadFormat() const;
267+
FormatEntity::Entry GetThreadFormat() const;
268268

269-
const FormatEntity::Entry *GetThreadStopFormat() const;
269+
FormatEntity::Entry GetThreadStopFormat() const;
270270

271271
lldb::ScriptLanguage GetScriptLanguage() const;
272272

@@ -310,7 +310,7 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
310310

311311
bool GetShowStatusline() const;
312312

313-
const FormatEntity::Entry *GetStatuslineFormat() const;
313+
FormatEntity::Entry GetStatuslineFormat() const;
314314
bool SetStatuslineFormat(const FormatEntity::Entry &format);
315315

316316
llvm::StringRef GetSeparator() const;

‎lldb/include/lldb/Core/FormatEntity.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,8 @@ struct Entry {
205205
return true;
206206
}
207207

208+
operator bool() const { return type != Type::Invalid; }
209+
208210
std::vector<Entry> &GetChildren();
209211

210212
std::string string;
@@ -217,7 +219,7 @@ struct Entry {
217219
size_t level = 0;
218220
/// @}
219221

220-
Type type;
222+
Type type = Type::Invalid;
221223
lldb::Format fmt = lldb::eFormatDefault;
222224
lldb::addr_t number = 0;
223225
bool deref = false;

‎lldb/include/lldb/Interpreter/OptionValue.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,8 @@ class OptionValue {
292292
return GetStringValue();
293293
if constexpr (std::is_same_v<T, ArchSpec>)
294294
return GetArchSpecValue();
295+
if constexpr (std::is_same_v<T, FormatEntity::Entry>)
296+
return GetFormatEntityValue();
295297
if constexpr (std::is_enum_v<T>)
296298
if (std::optional<int64_t> value = GetEnumerationValue())
297299
return static_cast<T>(*value);
@@ -303,11 +305,9 @@ class OptionValue {
303305
typename std::remove_pointer<T>::type>::type,
304306
std::enable_if_t<std::is_pointer_v<T>, bool> = true>
305307
T GetValueAs() const {
306-
if constexpr (std::is_same_v<U, FormatEntity::Entry>)
307-
return GetFormatEntity();
308-
if constexpr (std::is_same_v<U, RegularExpression>)
309-
return GetRegexValue();
310-
return {};
308+
static_assert(std::is_same_v<U, RegularExpression>,
309+
"only for RegularExpression");
310+
return GetRegexValue();
311311
}
312312

313313
bool SetValueAs(bool v) { return SetBooleanValue(v); }
@@ -390,7 +390,7 @@ class OptionValue {
390390
std::optional<UUID> GetUUIDValue() const;
391391
bool SetUUIDValue(const UUID &uuid);
392392

393-
const FormatEntity::Entry *GetFormatEntity() const;
393+
FormatEntity::Entry GetFormatEntityValue() const;
394394
bool SetFormatEntityValue(const FormatEntity::Entry &entry);
395395

396396
const RegularExpression *GetRegexValue() const;

‎lldb/include/lldb/Target/Language.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -399,9 +399,7 @@ class Language : public PluginInterface {
399399
/// Python uses \b except. Defaults to \b catch.
400400
virtual llvm::StringRef GetCatchKeyword() const { return "catch"; }
401401

402-
virtual const FormatEntity::Entry *GetFunctionNameFormat() const {
403-
return nullptr;
404-
}
402+
virtual FormatEntity::Entry GetFunctionNameFormat() const { return {}; }
405403

406404
protected:
407405
// Classes that inherit from Language can see and modify these

‎lldb/source/Core/Debugger.cpp

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -290,19 +290,19 @@ bool Debugger::GetAutoConfirm() const {
290290
idx, g_debugger_properties[idx].default_uint_value != 0);
291291
}
292292

293-
const FormatEntity::Entry *Debugger::GetDisassemblyFormat() const {
293+
FormatEntity::Entry Debugger::GetDisassemblyFormat() const {
294294
constexpr uint32_t idx = ePropertyDisassemblyFormat;
295-
return GetPropertyAtIndexAs<const FormatEntity::Entry *>(idx);
295+
return GetPropertyAtIndexAs<FormatEntity::Entry>(idx, {});
296296
}
297297

298-
const FormatEntity::Entry *Debugger::GetFrameFormat() const {
298+
FormatEntity::Entry Debugger::GetFrameFormat() const {
299299
constexpr uint32_t idx = ePropertyFrameFormat;
300-
return GetPropertyAtIndexAs<const FormatEntity::Entry *>(idx);
300+
return GetPropertyAtIndexAs<FormatEntity::Entry>(idx, {});
301301
}
302302

303-
const FormatEntity::Entry *Debugger::GetFrameFormatUnique() const {
303+
FormatEntity::Entry Debugger::GetFrameFormatUnique() const {
304304
constexpr uint32_t idx = ePropertyFrameFormatUnique;
305-
return GetPropertyAtIndexAs<const FormatEntity::Entry *>(idx);
305+
return GetPropertyAtIndexAs<FormatEntity::Entry>(idx, {});
306306
}
307307

308308
uint64_t Debugger::GetStopDisassemblyMaxSize() const {
@@ -346,14 +346,14 @@ void Debugger::SetPrompt(llvm::StringRef p) {
346346
GetCommandInterpreter().UpdatePrompt(new_prompt);
347347
}
348348

349-
const FormatEntity::Entry *Debugger::GetThreadFormat() const {
349+
FormatEntity::Entry Debugger::GetThreadFormat() const {
350350
constexpr uint32_t idx = ePropertyThreadFormat;
351-
return GetPropertyAtIndexAs<const FormatEntity::Entry *>(idx);
351+
return GetPropertyAtIndexAs<FormatEntity::Entry>(idx, {});
352352
}
353353

354-
const FormatEntity::Entry *Debugger::GetThreadStopFormat() const {
354+
FormatEntity::Entry Debugger::GetThreadStopFormat() const {
355355
constexpr uint32_t idx = ePropertyThreadStopFormat;
356-
return GetPropertyAtIndexAs<const FormatEntity::Entry *>(idx);
356+
return GetPropertyAtIndexAs<FormatEntity::Entry>(idx, {});
357357
}
358358

359359
lldb::ScriptLanguage Debugger::GetScriptLanguage() const {
@@ -480,9 +480,9 @@ bool Debugger::GetShowStatusline() const {
480480
idx, g_debugger_properties[idx].default_uint_value != 0);
481481
}
482482

483-
const FormatEntity::Entry *Debugger::GetStatuslineFormat() const {
483+
FormatEntity::Entry Debugger::GetStatuslineFormat() const {
484484
constexpr uint32_t idx = ePropertyStatuslineFormat;
485-
return GetPropertyAtIndexAs<const FormatEntity::Entry *>(idx);
485+
return GetPropertyAtIndexAs<FormatEntity::Entry>(idx, {});
486486
}
487487

488488
llvm::StringRef Debugger::GetSeparator() const {
@@ -1539,8 +1539,11 @@ bool Debugger::FormatDisassemblerAddress(const FormatEntity::Entry *format,
15391539
FormatEntity::Entry format_entry;
15401540

15411541
if (format == nullptr) {
1542-
if (exe_ctx != nullptr && exe_ctx->HasTargetScope())
1543-
format = exe_ctx->GetTargetRef().GetDebugger().GetDisassemblyFormat();
1542+
if (exe_ctx != nullptr && exe_ctx->HasTargetScope()) {
1543+
format_entry =
1544+
exe_ctx->GetTargetRef().GetDebugger().GetDisassemblyFormat();
1545+
format = &format_entry;
1546+
}
15441547
if (format == nullptr) {
15451548
FormatEntity::Parse("${addr}: ", format_entry);
15461549
format = &format_entry;

‎lldb/source/Core/Disassembler.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -309,8 +309,8 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch,
309309
const FormatEntity::Entry *disassembly_format = nullptr;
310310
FormatEntity::Entry format;
311311
if (exe_ctx.HasTargetScope()) {
312-
disassembly_format =
313-
exe_ctx.GetTargetRef().GetDebugger().GetDisassemblyFormat();
312+
format = exe_ctx.GetTargetRef().GetDebugger().GetDisassemblyFormat();
313+
disassembly_format = &format;
314314
} else {
315315
FormatEntity::Parse("${addr}: ", format);
316316
disassembly_format = &format;
@@ -1013,8 +1013,8 @@ void InstructionList::Dump(Stream *s, bool show_address, bool show_bytes,
10131013
const FormatEntity::Entry *disassembly_format = nullptr;
10141014
FormatEntity::Entry format;
10151015
if (exe_ctx && exe_ctx->HasTargetScope()) {
1016-
disassembly_format =
1017-
exe_ctx->GetTargetRef().GetDebugger().GetDisassemblyFormat();
1016+
format = exe_ctx->GetTargetRef().GetDebugger().GetDisassemblyFormat();
1017+
disassembly_format = &format;
10181018
} else {
10191019
FormatEntity::Parse("${addr}: ", format);
10201020
disassembly_format = &format;

‎lldb/source/Core/FormatEntity.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1279,13 +1279,13 @@ static bool FormatFunctionNameForLanguage(Stream &s,
12791279
if (!language_plugin)
12801280
return false;
12811281

1282-
const auto *format = language_plugin->GetFunctionNameFormat();
1282+
FormatEntity::Entry format = language_plugin->GetFunctionNameFormat();
12831283
if (!format)
12841284
return false;
12851285

12861286
StreamString name_stream;
12871287
const bool success =
1288-
FormatEntity::Format(*format, name_stream, sc, exe_ctx, /*addr=*/nullptr,
1288+
FormatEntity::Format(format, name_stream, sc, exe_ctx, /*addr=*/nullptr,
12891289
/*valobj=*/nullptr, /*function_changed=*/false,
12901290
/*initial_function=*/false);
12911291
if (success)

‎lldb/source/Core/Statusline.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,9 +146,9 @@ void Statusline::Redraw(bool update) {
146146
}
147147

148148
StreamString stream;
149-
if (auto *format = m_debugger.GetStatuslineFormat())
150-
FormatEntity::Format(*format, stream, &symbol_ctx, &exe_ctx, nullptr,
151-
nullptr, false, false);
149+
FormatEntity::Entry format = m_debugger.GetStatuslineFormat();
150+
FormatEntity::Format(format, stream, &symbol_ctx, &exe_ctx, nullptr, nullptr,
151+
false, false);
152152

153-
Draw(std::string(stream.GetString()));
153+
Draw(stream.GetString().str());
154154
}

‎lldb/source/Interpreter/OptionValue.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -380,11 +380,11 @@ bool OptionValue::SetLanguageValue(lldb::LanguageType new_language) {
380380
return false;
381381
}
382382

383-
const FormatEntity::Entry *OptionValue::GetFormatEntity() const {
383+
FormatEntity::Entry OptionValue::GetFormatEntityValue() const {
384384
std::lock_guard<std::mutex> lock(m_mutex);
385385
if (const OptionValueFormatEntity *option_value = GetAsFormatEntity())
386-
return &option_value->GetCurrentValue();
387-
return nullptr;
386+
return option_value->GetCurrentValue();
387+
return {};
388388
}
389389

390390
const RegularExpression *OptionValue::GetRegexValue() const {

‎lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2079,9 +2079,9 @@ class PluginProperties : public Properties {
20792079
m_collection_sp->Initialize(g_language_cplusplus_properties);
20802080
}
20812081

2082-
const FormatEntity::Entry *GetFunctionNameFormat() const {
2083-
return GetPropertyAtIndexAs<const FormatEntity::Entry *>(
2084-
ePropertyFunctionNameFormat);
2082+
FormatEntity::Entry GetFunctionNameFormat() const {
2083+
return GetPropertyAtIndexAs<FormatEntity::Entry>(
2084+
ePropertyFunctionNameFormat, {});
20852085
}
20862086
};
20872087
} // namespace
@@ -2091,7 +2091,7 @@ static PluginProperties &GetGlobalPluginProperties() {
20912091
return g_settings;
20922092
}
20932093

2094-
const FormatEntity::Entry *CPlusPlusLanguage::GetFunctionNameFormat() const {
2094+
FormatEntity::Entry CPlusPlusLanguage::GetFunctionNameFormat() const {
20952095
return GetGlobalPluginProperties().GetFunctionNameFormat();
20962096
}
20972097

0 commit comments

Comments
 (0)