https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/174618
>From 37d0feab15f74270df211ab592fa785bb1a0a2fc Mon Sep 17 00:00:00 2001 From: Michael Buch <[email protected]> Date: Tue, 6 Jan 2026 16:41:03 +0000 Subject: [PATCH 1/3] [WIP][lldb][Format] Make FormatEntity::Format a member function --- lldb/include/lldb/Core/FormatEntity.h | 28 ++++++++--- lldb/source/Core/Debugger.cpp | 5 +- lldb/source/Core/FormatEntity.cpp | 49 +++++++++---------- lldb/source/Core/IOHandlerCursesGUI.cpp | 18 ++++--- lldb/source/Core/Statusline.cpp | 4 +- .../DataFormatters/FormattersHelpers.cpp | 6 ++- lldb/source/DataFormatters/TypeSummary.cpp | 7 +-- lldb/source/Target/StackFrame.cpp | 5 +- lldb/source/Target/Thread.cpp | 5 +- lldb/unittests/Core/FormatEntityTest.cpp | 2 +- 10 files changed, 74 insertions(+), 55 deletions(-) diff --git a/lldb/include/lldb/Core/FormatEntity.h b/lldb/include/lldb/Core/FormatEntity.h index 107c30a000979..433c265df5084 100644 --- a/lldb/include/lldb/Core/FormatEntity.h +++ b/lldb/include/lldb/Core/FormatEntity.h @@ -229,14 +229,26 @@ struct Entry { bool deref = false; }; -bool Format(const Entry &entry, Stream &s, const SymbolContext *sc, - const ExecutionContext *exe_ctx, const Address *addr, - ValueObject *valobj, bool function_changed, bool initial_function); - -bool FormatStringRef(const llvm::StringRef &format, Stream &s, - const SymbolContext *sc, const ExecutionContext *exe_ctx, - const Address *addr, ValueObject *valobj, - bool function_changed, bool initial_function); +class Formatter { +public: + bool Format(const Entry &entry, Stream &s, const SymbolContext *sc, + const ExecutionContext *exe_ctx, const Address *addr, + ValueObject *valobj, bool function_changed, + bool initial_function); + + bool FormatStringRef(const llvm::StringRef &format, Stream &s, + const SymbolContext *sc, const ExecutionContext *exe_ctx, + const Address *addr, ValueObject *valobj, + bool function_changed, bool initial_function); + +private: + bool DumpValue(Stream &s, const SymbolContext *sc, + const ExecutionContext *exe_ctx, + const FormatEntity::Entry &entry, ValueObject *valobj); + + bool FormatFunctionNameForLanguage(Stream &s, const ExecutionContext *exe_ctx, + const SymbolContext *sc); +}; Status Parse(const llvm::StringRef &format, Entry &entry); diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index 99f4a728e3f17..6ab032ec60e7a 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -1607,8 +1607,9 @@ bool Debugger::FormatDisassemblerAddress(const FormatEntity::Entry *format, (prev_sc->function == nullptr && prev_sc->symbol == nullptr)) { initial_function = true; } - return FormatEntity::Format(*format, s, sc, exe_ctx, addr, nullptr, - function_changed, initial_function); + return FormatEntity::Formatter{}.Format(*format, s, sc, exe_ctx, addr, + nullptr, function_changed, + initial_function); } void Debugger::AssertCallback(llvm::StringRef message, diff --git a/lldb/source/Core/FormatEntity.cpp b/lldb/source/Core/FormatEntity.cpp index faafb5eb6e4f2..12c77760b16e7 100644 --- a/lldb/source/Core/FormatEntity.cpp +++ b/lldb/source/Core/FormatEntity.cpp @@ -754,14 +754,15 @@ static bool DumpValueWithLLVMFormat(Stream &s, llvm::StringRef options, return true; } -static bool DumpValue(Stream &s, const SymbolContext *sc, - const ExecutionContext *exe_ctx, - const FormatEntity::Entry &entry, ValueObject *valobj) { +bool FormatEntity::Formatter::DumpValue(Stream &s, const SymbolContext *sc, + const ExecutionContext *exe_ctx, + const FormatEntity::Entry &entry, + ValueObject *valobj) { if (valobj == nullptr) return false; Log *log = GetLog(LLDBLog::DataFormatters); - Format custom_format = eFormatInvalid; + enum Format custom_format = eFormatInvalid; ValueObject::ValueObjectRepresentationStyle val_obj_display = entry.string.empty() ? ValueObject::eValueObjectRepresentationStyleValue @@ -1057,8 +1058,8 @@ static bool DumpValue(Stream &s, const SymbolContext *sc, success &= item->DumpPrintableRepresentation(s, val_obj_display, custom_format); } else { - success &= FormatEntity::FormatStringRef( - special_directions, s, sc, exe_ctx, nullptr, item, false, false); + success &= FormatStringRef(special_directions, s, sc, exe_ctx, nullptr, + item, false, false); } if (--max_num_children == 0) { @@ -1272,9 +1273,8 @@ static bool HandleFunctionNameWithArgs(Stream &s, return true; } -static bool FormatFunctionNameForLanguage(Stream &s, - const ExecutionContext *exe_ctx, - const SymbolContext *sc) { +bool FormatEntity::Formatter::FormatFunctionNameForLanguage( + Stream &s, const ExecutionContext *exe_ctx, const SymbolContext *sc) { assert(sc); Language *language_plugin = nullptr; @@ -1294,37 +1294,36 @@ static bool FormatFunctionNameForLanguage(Stream &s, StreamString name_stream; const bool success = - FormatEntity::Format(format, name_stream, sc, exe_ctx, /*addr=*/nullptr, - /*valobj=*/nullptr, /*function_changed=*/false, - /*initial_function=*/false); + Format(format, name_stream, sc, exe_ctx, /*addr=*/nullptr, + /*valobj=*/nullptr, /*function_changed=*/false, + /*initial_function=*/false); if (success) s << name_stream.GetString(); return success; } -bool FormatEntity::FormatStringRef(const llvm::StringRef &format_str, Stream &s, - const SymbolContext *sc, - const ExecutionContext *exe_ctx, - const Address *addr, ValueObject *valobj, - bool function_changed, - bool initial_function) { +bool FormatEntity::Formatter::FormatStringRef( + const llvm::StringRef &format_str, Stream &s, const SymbolContext *sc, + const ExecutionContext *exe_ctx, const Address *addr, ValueObject *valobj, + bool function_changed, bool initial_function) { if (!format_str.empty()) { FormatEntity::Entry root; Status error = FormatEntity::Parse(format_str, root); if (error.Success()) { - return FormatEntity::Format(root, s, sc, exe_ctx, addr, valobj, - function_changed, initial_function); + return Format(root, s, sc, exe_ctx, addr, valobj, function_changed, + initial_function); } } return false; } -bool FormatEntity::Format(const Entry &entry, Stream &s, - const SymbolContext *sc, - const ExecutionContext *exe_ctx, const Address *addr, - ValueObject *valobj, bool function_changed, - bool initial_function) { +bool FormatEntity::Formatter::Format(const Entry &entry, Stream &s, + const SymbolContext *sc, + const ExecutionContext *exe_ctx, + const Address *addr, ValueObject *valobj, + bool function_changed, + bool initial_function) { switch (entry.type) { case Entry::Type::Invalid: case Entry::Type::ParentNumber: // Only used for diff --git a/lldb/source/Core/IOHandlerCursesGUI.cpp b/lldb/source/Core/IOHandlerCursesGUI.cpp index 3976630ccb429..b4716289a3c90 100644 --- a/lldb/source/Core/IOHandlerCursesGUI.cpp +++ b/lldb/source/Core/IOHandlerCursesGUI.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "lldb/Core/IOHandlerCursesGUI.h" +#include "lldb/Core/FormatEntity.h" #include "lldb/Host/Config.h" #if LLDB_ENABLE_CURSES @@ -5061,8 +5062,8 @@ class FrameTreeDelegate : public TreeDelegate { const SymbolContext &sc = frame_sp->GetSymbolContext(eSymbolContextEverything); ExecutionContext exe_ctx(frame_sp); - if (FormatEntity::Format(m_format, strm, &sc, &exe_ctx, nullptr, - nullptr, false, false)) { + if (FormatEntity::Formatter{}.Format(m_format, strm, &sc, &exe_ctx, + nullptr, nullptr, false, false)) { int right_pad = 1; window.PutCStringTruncated(right_pad, strm.GetString().str().c_str()); } @@ -5119,8 +5120,8 @@ class ThreadTreeDelegate : public TreeDelegate { if (thread_sp) { StreamString strm; ExecutionContext exe_ctx(thread_sp); - if (FormatEntity::Format(m_format, strm, nullptr, &exe_ctx, nullptr, - nullptr, false, false)) { + if (FormatEntity::Formatter{}.Format(m_format, strm, nullptr, &exe_ctx, + nullptr, nullptr, false, false)) { int right_pad = 1; window.PutCStringTruncated(right_pad, strm.GetString().str().c_str()); } @@ -5218,8 +5219,8 @@ class ThreadsTreeDelegate : public TreeDelegate { if (process_sp && process_sp->IsAlive()) { StreamString strm; ExecutionContext exe_ctx(process_sp); - if (FormatEntity::Format(m_format, strm, nullptr, &exe_ctx, nullptr, - nullptr, false, false)) { + if (FormatEntity::Formatter{}.Format(m_format, strm, nullptr, &exe_ctx, + nullptr, nullptr, false, false)) { int right_pad = 1; window.PutCStringTruncated(right_pad, strm.GetString().str().c_str()); } @@ -6750,8 +6751,9 @@ class StatusBarWindowDelegate : public WindowDelegate { if (StateIsStoppedState(state, true)) { StreamString strm; - if (thread && FormatEntity::Format(m_format, strm, nullptr, &exe_ctx, - nullptr, nullptr, false, false)) { + if (thread && + FormatEntity::Formatter{}.Format(m_format, strm, nullptr, &exe_ctx, + nullptr, nullptr, false, false)) { window.MoveCursor(40, 0); window.PutCStringTruncated(1, strm.GetString().str().c_str()); } diff --git a/lldb/source/Core/Statusline.cpp b/lldb/source/Core/Statusline.cpp index 922aada07e979..80e584e36388b 100644 --- a/lldb/source/Core/Statusline.cpp +++ b/lldb/source/Core/Statusline.cpp @@ -149,8 +149,8 @@ void Statusline::Redraw(std::optional<ExecutionContextRef> exe_ctx_ref) { StreamString stream; FormatEntity::Entry format = m_debugger.GetStatuslineFormat(); - FormatEntity::Format(format, stream, &sym_ctx, &exe_ctx, nullptr, nullptr, - false, false); + FormatEntity::Formatter{}.Format(format, stream, &sym_ctx, &exe_ctx, nullptr, + nullptr, false, false); Draw(stream.GetString().str()); } diff --git a/lldb/source/DataFormatters/FormattersHelpers.cpp b/lldb/source/DataFormatters/FormattersHelpers.cpp index 028fc5da73dc0..a21fb61d31122 100644 --- a/lldb/source/DataFormatters/FormattersHelpers.cpp +++ b/lldb/source/DataFormatters/FormattersHelpers.cpp @@ -10,6 +10,7 @@ #include "lldb/DataFormatters/FormattersHelpers.h" +#include "lldb/Core/FormatEntity.h" #include "lldb/Core/Module.h" #include "lldb/Target/StackFrame.h" #include "lldb/Target/Target.h" @@ -148,6 +149,7 @@ void lldb_private::formatters::DumpCxxSmartPtrPointerSummary( bool lldb_private::formatters::ContainerSizeSummaryProvider( ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) { - return FormatEntity::FormatStringRef("size=${svar%#}", stream, nullptr, - nullptr, nullptr, &valobj, false, false); + return FormatEntity::Formatter{}.FormatStringRef("size=${svar%#}", stream, + nullptr, nullptr, nullptr, + &valobj, false, false); } diff --git a/lldb/source/DataFormatters/TypeSummary.cpp b/lldb/source/DataFormatters/TypeSummary.cpp index 6aa290698cd12..1817783cd3af8 100644 --- a/lldb/source/DataFormatters/TypeSummary.cpp +++ b/lldb/source/DataFormatters/TypeSummary.cpp @@ -9,6 +9,7 @@ #include "lldb/DataFormatters/TypeSummary.h" #include "FormatterBytecode.h" +#include "lldb/Core/FormatEntity.h" #include "lldb/lldb-enumerations.h" #include "lldb/lldb-public.h" @@ -105,9 +106,9 @@ bool StringSummaryFormat::FormatObject(ValueObject *valobj, std::string &retval, retval = std::string(s.GetString()); return true; } else { - if (FormatEntity::Format(m_format, s, &sc, &exe_ctx, - &sc.line_entry.range.GetBaseAddress(), valobj, - false, false)) { + if (FormatEntity::Formatter{}.Format(m_format, s, &sc, &exe_ctx, + &sc.line_entry.range.GetBaseAddress(), + valobj, false, false)) { retval.assign(std::string(s.GetString())); return true; } else { diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp index 92d0099be972a..8abb511bd7b44 100644 --- a/lldb/source/Target/StackFrame.cpp +++ b/lldb/source/Target/StackFrame.cpp @@ -1936,8 +1936,9 @@ bool StackFrame::DumpUsingFormat(Stream &strm, StreamString s; s.PutCString(frame_marker); - if (format && FormatEntity::Format(*format, s, &m_sc, &exe_ctx, nullptr, - nullptr, false, false)) { + if (format && + FormatEntity::Formatter{}.Format(*format, s, &m_sc, &exe_ctx, nullptr, + nullptr, false, false)) { strm.PutCString(s.GetString()); return true; } diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp index b833918c27818..335a699584bad 100644 --- a/lldb/source/Target/Thread.cpp +++ b/lldb/source/Target/Thread.cpp @@ -1745,8 +1745,9 @@ bool Thread::DumpUsingFormat(Stream &strm, uint32_t frame_idx, } } - return FormatEntity::Format(*format, strm, frame_sp ? &frame_sc : nullptr, - &exe_ctx, nullptr, nullptr, false, false); + return FormatEntity::Formatter{}.Format( + *format, strm, frame_sp ? &frame_sc : nullptr, &exe_ctx, nullptr, nullptr, + false, false); } void Thread::DumpUsingSettingsFormat(Stream &strm, uint32_t frame_idx, diff --git a/lldb/unittests/Core/FormatEntityTest.cpp b/lldb/unittests/Core/FormatEntityTest.cpp index 309294a501883..ffa00240c1a1a 100644 --- a/lldb/unittests/Core/FormatEntityTest.cpp +++ b/lldb/unittests/Core/FormatEntityTest.cpp @@ -27,7 +27,7 @@ static Expected<std::string> Format(StringRef format_str) { if (status.Fail()) return status.ToError(); - FormatEntity::Format(format, stream, nullptr, nullptr, nullptr, nullptr, + FormatEntity::Formatter{}.Format(format, stream, nullptr, nullptr, nullptr, nullptr, false, false); return stream.GetString().str(); } >From 7cfa90c80b63630667b8a598a71dcc0a805de66b Mon Sep 17 00:00:00 2001 From: Michael Buch <[email protected]> Date: Tue, 6 Jan 2026 16:38:54 +0000 Subject: [PATCH 2/3] [WIP][lldb][Format] Reject recursive format entities --- lldb/include/lldb/Core/FormatEntity.h | 13 +++++++++++++ lldb/source/Core/FormatEntity.cpp | 13 +++++++++++++ .../Shell/Settings/TestCxxFrameFormatRecursive.test | 4 ---- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/lldb/include/lldb/Core/FormatEntity.h b/lldb/include/lldb/Core/FormatEntity.h index 433c265df5084..e357a6d3b823b 100644 --- a/lldb/include/lldb/Core/FormatEntity.h +++ b/lldb/include/lldb/Core/FormatEntity.h @@ -11,6 +11,7 @@ #include "lldb/lldb-enumerations.h" #include "lldb/lldb-types.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/SmallVector.h" #include <algorithm> #include <cstddef> @@ -248,6 +249,18 @@ class Formatter { bool FormatFunctionNameForLanguage(Stream &s, const ExecutionContext *exe_ctx, const SymbolContext *sc); + + bool IsInvalidRecursiveFormat(Entry::Type type); + + auto PushEntryType(Entry::Type type) { + m_entry_type_stack.push_back(type); + return llvm::scope_exit([this] { + assert(!m_entry_type_stack.empty()); + m_entry_type_stack.pop_back(); + }); + } + + llvm::SmallVector<Entry::Type, 1> m_entry_type_stack; }; Status Parse(const llvm::StringRef &format, Entry &entry); diff --git a/lldb/source/Core/FormatEntity.cpp b/lldb/source/Core/FormatEntity.cpp index 12c77760b16e7..3554d934850e9 100644 --- a/lldb/source/Core/FormatEntity.cpp +++ b/lldb/source/Core/FormatEntity.cpp @@ -1324,6 +1324,11 @@ bool FormatEntity::Formatter::Format(const Entry &entry, Stream &s, const Address *addr, ValueObject *valobj, bool function_changed, bool initial_function) { + if (IsInvalidRecursiveFormat(entry.type)) + return false; + + auto token = PushEntryType(entry.type); + switch (entry.type) { case Entry::Type::Invalid: case Entry::Type::ParentNumber: // Only used for @@ -2717,3 +2722,11 @@ Status FormatEntity::Parse(const llvm::StringRef &format_str, Entry &entry) { llvm::StringRef modifiable_format(format_str); return ParseInternal(modifiable_format, entry, 0); } + +bool FormatEntity::Formatter::IsInvalidRecursiveFormat(Entry::Type type) { + // It is expected that Scope and Root format entities recursively call Format. + if (llvm::is_contained({Entry::Type::Scope, Entry::Type::Root}, type)) + return false; + + return llvm::is_contained(m_entry_type_stack, type); +} diff --git a/lldb/test/Shell/Settings/TestCxxFrameFormatRecursive.test b/lldb/test/Shell/Settings/TestCxxFrameFormatRecursive.test index 465b1bb28e327..effc47ed228a2 100644 --- a/lldb/test/Shell/Settings/TestCxxFrameFormatRecursive.test +++ b/lldb/test/Shell/Settings/TestCxxFrameFormatRecursive.test @@ -1,7 +1,3 @@ -# Flaky on Linux, see https://github.com/llvm/llvm-project/issues/142726 -# UNSUPPORTED: system-linux -# XFAIL: * - # Test disallowed variables inside the # plugin.cplusplus.display.function-name-format setting. >From 3d691729eb79ac38f48dded84c9115e376da1ef9 Mon Sep 17 00:00:00 2001 From: Michael Buch <[email protected]> Date: Tue, 6 Jan 2026 17:03:00 +0000 Subject: [PATCH 3/3] fixup --- lldb/source/Core/FormatEntity.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lldb/source/Core/FormatEntity.cpp b/lldb/source/Core/FormatEntity.cpp index 3554d934850e9..50fa1c10b20fe 100644 --- a/lldb/source/Core/FormatEntity.cpp +++ b/lldb/source/Core/FormatEntity.cpp @@ -1324,8 +1324,12 @@ bool FormatEntity::Formatter::Format(const Entry &entry, Stream &s, const Address *addr, ValueObject *valobj, bool function_changed, bool initial_function) { - if (IsInvalidRecursiveFormat(entry.type)) + if (IsInvalidRecursiveFormat(entry.type)) { + LLDB_LOG(GetLog(LLDBLog::DataFormatters), + "Error: detected recursive format entity: {0}", + FormatEntity::Entry::TypeToCString(entry.type)); return false; + } auto token = PushEntryType(entry.type); _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
