[Lldb-commits] [lldb] 67d67eb - Internal expressions shouldn't increment the result variable numbering.
Jim Ingham via lldb-commits
lldb-commits at lists.llvm.org
Mon Mar 23 13:31:25 PDT 2020
Author: Jim Ingham
Date: 2020-03-23T13:30:37-07:00
New Revision: 67d67ebe8f2535c5de75c62820f7713f87d07307
URL: https://github.com/llvm/llvm-project/commit/67d67ebe8f2535c5de75c62820f7713f87d07307
DIFF: https://github.com/llvm/llvm-project/commit/67d67ebe8f2535c5de75c62820f7713f87d07307.diff
LOG: Internal expressions shouldn't increment the result variable numbering.
There an option: EvaluateExpressionOptions::SetResultIsInternal to indicate
whether the result number should be returned to the pool or not. It
got broken when the PersistentExpressionState was refactored.
This fixes the issue and provides a test of the behavior.
Differential Revision: https://reviews.llvm.org/D76532
Added:
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
Modified:
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
Removed:
################################################################################
diff --git a/lldb/include/lldb/Expression/ExpressionVariable.h b/lldb/include/lldb/Expression/ExpressionVariable.h
index c523176e003f..60062d212bad 100644
--- a/lldb/include/lldb/Expression/ExpressionVariable.h
+++ b/lldb/include/lldb/Expression/ExpressionVariable.h
@@ -221,11 +221,7 @@ class PersistentExpressionState : public ExpressionVariableList {
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 @@ class PersistentExpressionState : public ExpressionVariableList {
void RegisterExecutionUnit(lldb::IRExecutionUnitSP &execution_unit_sp);
+protected:
+ virtual llvm::StringRef
+ GetPersistentVariablePrefix(bool is_error = false) const = 0;
+
private:
LLVMCastKind m_kind;
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 77cda4998192..cc74fe0f3d74 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -1100,11 +1100,6 @@ class Target : public std::enable_shared_from_this<Target>,
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
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index e1d0ca941108..4c9c44ea15f4 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -3270,9 +3270,7 @@ ValueObjectSP ValueObject::Persist() {
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);
diff --git a/lldb/source/Expression/ExpressionVariable.cpp b/lldb/source/Expression/ExpressionVariable.cpp
index 7c27c0f249ec..d95f0745cf4b 100644
--- a/lldb/source/Expression/ExpressionVariable.cpp
+++ b/lldb/source/Expression/ExpressionVariable.cpp
@@ -76,13 +76,3 @@ void PersistentExpressionState::RegisterExecutionUnit(
}
}
}
-
-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);
-}
diff --git a/lldb/source/Expression/Materializer.cpp b/lldb/source/Expression/Materializer.cpp
index 33c061effca4..8e96891257e4 100644
--- a/lldb/source/Expression/Materializer.cpp
+++ b/lldb/source/Expression/Materializer.cpp
@@ -881,11 +881,9 @@ class EntityResultVariable : public Materializer::Entity {
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());
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
index 3cbedf80755a..42afac9edb0d 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
@@ -108,3 +108,14 @@ ClangPersistentVariables::GetClangASTImporter() {
}
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);
+}
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h
index 12268b6549aa..f6298911dd6c 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h
@@ -51,9 +51,7 @@ class ClangPersistentVariables : public PersistentExpressionState {
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 @@ class ClangPersistentVariables : public PersistentExpressionState {
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;
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
index 6d781934c174..b246fc374d1c 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -924,9 +924,7 @@ void ClangUserExpression::ClangUserExpressionHelper::CommitPersistentDecls() {
}
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(
diff --git a/lldb/source/Target/ABI.cpp b/lldb/source/Target/ABI.cpp
index cb7eca280a39..4320eb93adfc 100644
--- a/lldb/source/Target/ABI.cpp
+++ b/lldb/source/Target/ABI.cpp
@@ -97,10 +97,8 @@ ValueObjectSP ABI::GetReturnValueObject(Thread &thread, CompilerType &ast_type,
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;
diff --git a/lldb/test/API/commands/expression/result_numbering/Makefile b/lldb/test/API/commands/expression/result_numbering/Makefile
new file mode 100644
index 000000000000..695335e068c0
--- /dev/null
+++ b/lldb/test/API/commands/expression/result_numbering/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -std=c99
+
+include Makefile.rules
diff --git a/lldb/test/API/commands/expression/result_numbering/TestResultNumbering.py b/lldb/test/API/commands/expression/result_numbering/TestResultNumbering.py
new file mode 100644
index 000000000000..cd6b9c43775c
--- /dev/null
+++ b/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)
diff --git a/lldb/test/API/commands/expression/result_numbering/main.c b/lldb/test/API/commands/expression/result_numbering/main.c
new file mode 100644
index 000000000000..0f5853c99fb1
--- /dev/null
+++ b/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;
+}
More information about the lldb-commits
mailing list