[Lldb-commits] [lldb] 1d07609 - [lldb][Format] Reject recursive format entities (#174750)

[via lldb-commits]

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