jingham created this revision.
jingham added a reviewer: davide.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
For instance, breakpoints run an expression when evaluating a condition. If
each of these expression evaluations incremented the result numbering, you
could end up running an expression, and getting:
(lldb) expression foo
$1 = ...
Then do a continue, and then:
(lldb) expression foo
$14532 =
That would be disconcerting...
There was code (but no test, alas) to make this work, but when we refactored
the PersistentExpression class, this got broken.
This patch goes back to using the original method of computing the number. But
I also changed the API's a bit. When you went to get a result variable name in
the current method you had to do:
auto prefix = persistent_expression_state->GetPersistentVariablePrefix();
ConstString persistent_variable_name =
persistent_expression_state->GetNextPersistentVariableName(target,
prefix);
The persistent variable prefix is only ever used to make the persistent
variable name, so having this be done in two steps is unnecessary. It was also
odd that you needed the target - that was only because we the "next result
variable index" had been errantly moved into the target. So I made
GetNextPersistentVariableName the public interface, and passed in only the
is_error bool. I left the GetPersistentVariablePrefix in as a protected method
- it doesn't do anything for C, but it is used for swift.
I also added a test.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D76532
Files:
lldb/include/lldb/Expression/ExpressionVariable.h
lldb/include/lldb/Target/Target.h
lldb/source/Core/ValueObject.cpp
lldb/source/Expression/ExpressionVariable.cpp
lldb/source/Expression/Materializer.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/source/Target/ABI.cpp
lldb/test/API/commands/expression/result_numbering/Makefile
lldb/test/API/commands/expression/result_numbering/TestResultNumbering.py
lldb/test/API/commands/expression/result_numbering/main.c
Index: lldb/test/API/commands/expression/result_numbering/main.c
===================================================================
--- /dev/null
+++ lldb/test/API/commands/expression/result_numbering/main.c
@@ -0,0 +1,18 @@
+#include <stdio.h>
+
+int
+call_me(int input)
+{
+ return input;
+}
+
+int
+main()
+{
+ int value = call_me(0); // Set a breakpoint here
+ while (value < 10)
+ {
+ printf("Add conditions to this breakpoint: %d.\n", value++);
+ }
+ return 0;
+}
Index: lldb/test/API/commands/expression/result_numbering/TestResultNumbering.py
===================================================================
--- /dev/null
+++ lldb/test/API/commands/expression/result_numbering/TestResultNumbering.py
@@ -0,0 +1,48 @@
+"""
+Make sure running internal expressions doesn't
+influence the result variable numbering.
+"""
+
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class TestExpressionResultNumbering(TestBase):
+
+ mydir = TestBase.compute_mydir(__file__)
+
+ NO_DEBUG_INFO_TESTCASE = True
+
+ def test_sample_rename_this(self):
+ self.build()
+ self.main_source_file = lldb.SBFileSpec("main.c")
+ self.do_numbering_test()
+
+ def do_numbering_test(self):
+ (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+ "Set a breakpoint here", self.main_source_file)
+
+ bkpt = target.BreakpointCreateBySourceRegex("Add conditions to this breakpoint",
+ self.main_source_file)
+ self.assertEqual(bkpt.GetNumLocations(), 1, "Set the breakpoint")
+
+ bkpt.SetCondition("call_me(value) < 6")
+
+ # Get the number of the last expression:
+ result = thread.frames[0].EvaluateExpression("call_me(200)")
+ self.assertTrue(result.GetError().Success(), "Our expression succeeded")
+ name = result.GetName()
+ ordinal = int(name[1:])
+
+ process.Continue()
+
+ # The condition evaluation had to run a 4 expressions, but we haven't
+ # run any user expressions.
+ result = thread.frames[0].EvaluateExpression("call_me(200)")
+ self.assertTrue(result.GetError().Success(), "Our expression succeeded the second time")
+ after_name = result.GetName()
+ after_ordinal = int(after_name[1:])
+ self.assertEqual(ordinal + 1, after_ordinal)
Index: lldb/test/API/commands/expression/result_numbering/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/commands/expression/result_numbering/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -std=c99
+
+include Makefile.rules
Index: lldb/source/Target/ABI.cpp
===================================================================
--- lldb/source/Target/ABI.cpp
+++ lldb/source/Target/ABI.cpp
@@ -97,10 +97,8 @@
if (!persistent_expression_state)
return {};
- auto prefix = persistent_expression_state->GetPersistentVariablePrefix();
ConstString persistent_variable_name =
- persistent_expression_state->GetNextPersistentVariableName(target,
- prefix);
+ persistent_expression_state->GetNextPersistentVariableName();
lldb::ValueObjectSP const_valobj_sp;
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -924,9 +924,7 @@
}
ConstString ClangUserExpression::ResultDelegate::GetName() {
- auto prefix = m_persistent_state->GetPersistentVariablePrefix();
- return m_persistent_state->GetNextPersistentVariableName(*m_target_sp,
- prefix);
+ return m_persistent_state->GetNextPersistentVariableName(false);
}
void ClangUserExpression::ResultDelegate::DidDematerialize(
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h
@@ -51,9 +51,7 @@
void RemovePersistentVariable(lldb::ExpressionVariableSP variable) override;
- llvm::StringRef GetPersistentVariablePrefix(bool is_error) const override {
- return "$";
- }
+ virtual ConstString GetNextPersistentVariableName(bool is_error = false);
/// Returns the next file name that should be used for user expressions.
std::string GetNextExprFileName() {
@@ -80,6 +78,12 @@
return m_hand_loaded_clang_modules;
}
+protected:
+ llvm::StringRef
+ GetPersistentVariablePrefix(bool is_error = false) const override {
+ return "$";
+ }
+
private:
/// The counter used by GetNextExprFileName.
uint32_t m_next_user_file_id = 0;
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
@@ -108,3 +108,14 @@
}
return m_ast_importer_sp;
}
+
+ConstString
+ClangPersistentVariables::GetNextPersistentVariableName(bool is_error) {
+ llvm::SmallString<64> name;
+ {
+ llvm::raw_svector_ostream os(name);
+ os << GetPersistentVariablePrefix(is_error)
+ << m_next_persistent_variable_id++;
+ }
+ return ConstString(name);
+}
Index: lldb/source/Expression/Materializer.cpp
===================================================================
--- lldb/source/Expression/Materializer.cpp
+++ lldb/source/Expression/Materializer.cpp
@@ -881,11 +881,9 @@
return;
}
- ConstString name =
- m_delegate
- ? m_delegate->GetName()
- : persistent_state->GetNextPersistentVariableName(
- *target_sp, persistent_state->GetPersistentVariablePrefix());
+ ConstString name = m_delegate
+ ? m_delegate->GetName()
+ : persistent_state->GetNextPersistentVariableName();
lldb::ExpressionVariableSP ret = persistent_state->CreatePersistentVariable(
exe_scope, name, m_type, map.GetByteOrder(), map.GetAddressByteSize());
Index: lldb/source/Expression/ExpressionVariable.cpp
===================================================================
--- lldb/source/Expression/ExpressionVariable.cpp
+++ lldb/source/Expression/ExpressionVariable.cpp
@@ -76,13 +76,3 @@
}
}
}
-
-ConstString PersistentExpressionState::GetNextPersistentVariableName(
- Target &target, llvm::StringRef Prefix) {
- llvm::SmallString<64> name;
- {
- llvm::raw_svector_ostream os(name);
- os << Prefix << target.GetNextPersistentVariableIndex();
- }
- return ConstString(name);
-}
Index: lldb/source/Core/ValueObject.cpp
===================================================================
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -3270,9 +3270,7 @@
if (!persistent_state)
return nullptr;
- auto prefix = persistent_state->GetPersistentVariablePrefix();
- ConstString name =
- persistent_state->GetNextPersistentVariableName(*target_sp, prefix);
+ ConstString name = persistent_state->GetNextPersistentVariableName();
ValueObjectSP const_result_sp =
ValueObjectConstResult::Create(target_sp.get(), GetValue(), name);
Index: lldb/include/lldb/Target/Target.h
===================================================================
--- lldb/include/lldb/Target/Target.h
+++ lldb/include/lldb/Target/Target.h
@@ -1095,11 +1095,6 @@
lldb::ExpressionVariableSP GetPersistentVariable(ConstString name);
- /// Return the next available number for numbered persistent variables.
- unsigned GetNextPersistentVariableIndex() {
- return m_next_persistent_variable_index++;
- }
-
lldb::addr_t GetPersistentSymbol(ConstString name);
/// This method will return the address of the starting function for
Index: lldb/include/lldb/Expression/ExpressionVariable.h
===================================================================
--- lldb/include/lldb/Expression/ExpressionVariable.h
+++ lldb/include/lldb/Expression/ExpressionVariable.h
@@ -221,11 +221,7 @@
uint32_t addr_byte_size) = 0;
/// Return a new persistent variable name with the specified prefix.
- ConstString GetNextPersistentVariableName(Target &target,
- llvm::StringRef prefix);
-
- virtual llvm::StringRef
- GetPersistentVariablePrefix(bool is_error = false) const = 0;
+ virtual ConstString GetNextPersistentVariableName(bool is_error = false) = 0;
virtual void
RemovePersistentVariable(lldb::ExpressionVariableSP variable) = 0;
@@ -237,6 +233,10 @@
void RegisterExecutionUnit(lldb::IRExecutionUnitSP &execution_unit_sp);
+protected:
+ virtual llvm::StringRef
+ GetPersistentVariablePrefix(bool is_error = false) const = 0;
+
private:
LLVMCastKind m_kind;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
