llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Dave Lee (kastiglione) <details> <summary>Changes</summary> --- Full diff: https://github.com/llvm/llvm-project/pull/181199.diff 3 Files Affected: - (modified) lldb/source/DataFormatters/TypeSynthetic.cpp (+33-1) - (modified) lldb/test/API/functionalities/data-formatter/bytecode-synthetic/TestBytecodeSynthetic.py (+20-1) - (modified) lldb/test/API/functionalities/data-formatter/bytecode-synthetic/main.cpp (+4-1) ``````````diff diff --git a/lldb/source/DataFormatters/TypeSynthetic.cpp b/lldb/source/DataFormatters/TypeSynthetic.cpp index 10655cdca6f05..32d035eb8a90b 100644 --- a/lldb/source/DataFormatters/TypeSynthetic.cpp +++ b/lldb/source/DataFormatters/TypeSynthetic.cpp @@ -22,6 +22,7 @@ #include "lldb/Utility/StreamString.h" #include "llvm/Support/Error.h" #include <cstdint> +#include <optional> using namespace lldb; using namespace lldb_private; @@ -291,10 +292,41 @@ lldb::ChildCacheState BytecodeSyntheticChildren::FrontEnd::Update() { return ChildCacheState::eRefetch; } + // If the top of the stack is a 0 or 1, that value is popped of the stack and + // treated as a return value of eRefetch or eReuse respectively. If the top of + // the stack is any other value, it stays on the stack and becomes part of + // `self`. + // + // This dynamic logic can lead to errors for synthetic formatter authors. + // Consider the case where an `update` implementation places the number of + // children last on the stack. LLDB will _sometimes_ (but not always) consume + // that value as a ChildCacheState value. This would cause downstream problems + // in `num_children`, because the count won't be on the stack. + // + // Bytecode authors are encouraged to explicitly push a ChildCacheState value + // on to the stack. + std::optional<ChildCacheState> cache_state = std::nullopt; + const FormatterBytecode::DataStackElement &top = data.back(); + if (auto *u = std::get_if<uint64_t>(&top)) + if (*u == 0 || *u == 1) + cache_state = static_cast<ChildCacheState>(*u); + if (auto *i = std::get_if<int64_t>(&top)) + if (*i == 0 || *i == 1) + cache_state = static_cast<ChildCacheState>(*i); + + if (cache_state) { + data.pop_back(); + if (cache_state == ChildCacheState::eReuse) + LLDB_LOG(GetLog(LLDBLog::DataFormatters), + "Bytecode formatter returned eReuse from `update` (type: `{0}`, " + "name: `{1}`)", + m_backend.GetDisplayTypeName(), m_backend.GetName()); + } + if (data.size() > 0) m_self = std::move(data); - return ChildCacheState::eRefetch; + return cache_state.value_or(ChildCacheState::eRefetch); } llvm::Expected<uint32_t> diff --git a/lldb/test/API/functionalities/data-formatter/bytecode-synthetic/TestBytecodeSynthetic.py b/lldb/test/API/functionalities/data-formatter/bytecode-synthetic/TestBytecodeSynthetic.py index c5c2811c59439..cd4be83ad8563 100644 --- a/lldb/test/API/functionalities/data-formatter/bytecode-synthetic/TestBytecodeSynthetic.py +++ b/lldb/test/API/functionalities/data-formatter/bytecode-synthetic/TestBytecodeSynthetic.py @@ -5,8 +5,9 @@ class TestCase(TestBase): + @skipUnlessDarwin - def test(self): + def test_synthetic(self): self.build() if self.TraceOn(): self.expect("log enable -v lldb formatters") @@ -21,3 +22,21 @@ def test(self): self.assertEqual(account.child[0].name, "username") self.expect("v acc", matching=False, substrs=["password"]) + + @skipUnlessDarwin + def test_update_reuse(self): + self.build() + + log = self.getBuildArtifact("formatter.log") + self.runCmd(f"log enable lldb formatters -f {log}") + + _, _, thread, _ = lldbutil.run_to_source_breakpoint( + self, "break here", lldb.SBFileSpec("main.cpp") + ) + + frame = thread.selected_frame + account = frame.var("acc") + self.assertEqual(account.num_children, 1) + + self.filecheck(f"platform shell cat {log}", __file__) + # CHECK: Bytecode formatter returned eReuse from `update` (type: `Account`, name: `acc`) diff --git a/lldb/test/API/functionalities/data-formatter/bytecode-synthetic/main.cpp b/lldb/test/API/functionalities/data-formatter/bytecode-synthetic/main.cpp index 3e6ae77bbca83..051e44c937bed 100644 --- a/lldb/test/API/functionalities/data-formatter/bytecode-synthetic/main.cpp +++ b/lldb/test/API/functionalities/data-formatter/bytecode-synthetic/main.cpp @@ -16,10 +16,13 @@ int main(int argc, char **argv) { __attribute__((used, section("__DATA_CONST,__lldbformatters"))) unsigned char _Account_synthetic[] = "\x01" // version - "\x15" // remaining record size + "\x19" // remaining record size "\x07" // type name size "Account" // type name "\x00" // flags + "\x06" // sig_update + "\x02" // program size + "\x20\x01" // `return eReuse` "\x02" // sig_get_num_children "\x02" // program size "\x20\x01" // `return 1` `````````` </details> https://github.com/llvm/llvm-project/pull/181199 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
