Author: Michael Buch Date: 2026-01-08T16:25:42Z New Revision: 1d07609894f0b07bf739a5ac2c07066ec8c3399c
URL: https://github.com/llvm/llvm-project/commit/1d07609894f0b07bf739a5ac2c07066ec8c3399c DIFF: https://github.com/llvm/llvm-project/commit/1d07609894f0b07bf739a5ac2c07066ec8c3399c.diff LOG: [lldb][Format] Reject recursive format entities (#174750) Depends on: * https://github.com/llvm/llvm-project/pull/174618 If a format entity calls back into `Format` and passes it a format entity type that we're already in the process of parsing, we are likely going to run into infinite recursion and blow the stack. I think this is only an issue when a format entity calls Format on a format string provided by the user (otherwise we're in control of the recursion). An example of this can be seen in the test-case adjusted by this patch. This seems to be causing actual crashes in the field, so this patch adds basic tracking to `Formatter::Format` that checks whether we're recursively parsing the same entity. This may very well be intended by some entities (e.g., `Root` and `Scope`), so there is an escape hatch for those. There's also a special case where `Variable` causes a recursive format (which I pointed out in a source comment). We could narrow the scope of what kind of recursion is allowed by adding a `UserProvidedFormatChild` (or similar) flag to `Entry`, and only disallow recursing on those kinds of entries. For now I just use an exemption list in `IsInvalidRecursiveFormat`. Adding a unit-test for this is unfortunately tricky because the only format entity that currently suffers from this is `${function.name-with-args}`, which requires a language plugin and valid target. If we really wanted to we could probably mock all of those, but the shell test provides test coverage for the previously crashing case. rdar://166890120 Added: Modified: lldb/include/lldb/Core/FormatEntity.h lldb/source/Core/FormatEntity.cpp lldb/test/Shell/Settings/TestCxxFrameFormatRecursive.test Removed: ################################################################################ diff --git a/lldb/include/lldb/Core/FormatEntity.h b/lldb/include/lldb/Core/FormatEntity.h index 1a4b3db30ad22..e01009a44aac7 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,11 +249,30 @@ class Formatter { bool FormatFunctionNameForLanguage(Stream &s); + /// Returns \c true if \a Format has been called for an \a Entry + /// with the specified \c type recusrively. Some types are permitted + /// to be formatted recursively, in which case this function returns + /// \c false. + bool IsInvalidRecursiveFormat(Entry::Type type); + + /// While the returned \a llvm::scope_exit is alive, the specified \c type + /// is tracked by this \c Formatter object for recursion. Once the returned + /// scope guard is destructed, the entry stops being tracked. + 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(); + }); + } + const SymbolContext *const m_sc = nullptr; const ExecutionContext *const m_exe_ctx = nullptr; const Address *const m_addr = nullptr; const bool m_function_changed = false; const bool m_initial_function = false; + + 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 6afff104d3cc9..3646f88ac9206 100644 --- a/lldb/source/Core/FormatEntity.cpp +++ b/lldb/source/Core/FormatEntity.cpp @@ -1312,6 +1312,15 @@ bool FormatEntity::Formatter::FormatStringRef(const llvm::StringRef &format_str, bool FormatEntity::Formatter::Format(const Entry &entry, Stream &s, ValueObject *valobj) { + if (IsInvalidRecursiveFormat(entry.type)) { + LLDB_LOG(GetLog(LLDBLog::DataFormatters), + "Error: detected recursive format entity: {0}", + FormatEntity::Entry::TypeToCString(entry.type)); + return false; + } + + auto entry_stack_guard = PushEntryType(entry.type); + switch (entry.type) { case Entry::Type::Invalid: case Entry::Type::ParentNumber: // Only used for @@ -2704,3 +2713,19 @@ 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. + // + // Variable may also be formatted recursively in some special cases. The main + // use-case being array summary strings, in which case Format will call itself + // with the subrange ValueObject and apply a freshly created Variable entry. + // E.g., ${var[1-3]} will format the [1-3] range with ${var%S}. + static constexpr std::array s_permitted_recursive_entities = { + Entry::Type::Scope, Entry::Type::Root, Entry::Type::Variable}; + + if (llvm::is_contained(s_permitted_recursive_entities, 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..017f8c227fb95 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. @@ -23,5 +19,4 @@ run bt # CHECK: bt -# CHECK-NOT: custom-frame -# CHECK: main +# CHECK: custom-frame 'main _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
