friss created this revision.
friss added reviewers: jingham, clayborg.
In r259902, LLDB started injecting all the locals in every expression
evaluation. This fixed a bunch of issues, but also caused others, mostly
performance regressions on some codebases. The regressions were bad
enough that we added a setting in r274783 to control the behavior and
we have been shipping with the setting off to avoid the perf regressions.
This patch changes the logic injecting the local variables to only inject
the ones present in the expression typed by the user. The approach is
fairly simple and just scans the typed expression for every local name.
Hopefully this gives us the best of both world as it just realizes the
types of the variables really used by the expression.
Landing this requires the 2 other issues I pointed out today to be addressed
but I wanted to gather comments right away.
https://reviews.llvm.org/D46551
Files:
packages/Python/lldbsuite/test/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
source/Expression/ExpressionSourceCode.cpp
Index: source/Expression/ExpressionSourceCode.cpp
===================================================================
--- source/Expression/ExpressionSourceCode.cpp
+++ source/Expression/ExpressionSourceCode.cpp
@@ -9,6 +9,9 @@
#include "lldb/Expression/ExpressionSourceCode.h"
+#include "llvm/ADT/StringRef.h"
+#include "clang/Basic/CharInfo.h"
+
#include "Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.h"
#include "Plugins/ExpressionParser/Clang/ClangPersistentVariables.h"
#include "lldb/Symbol/Block.h"
@@ -165,14 +168,29 @@
}
}
+static bool ExprBodyContainsVar(llvm::StringRef var, llvm::StringRef body) {
+ int from = 0;
+ while ((from = body.find(var, from)) != llvm::StringRef::npos) {
+ if ((from != 0 && clang::isIdentifierBody(body[from-1])) ||
+ (from + var.size() != body.size() &&
+ clang::isIdentifierBody(body[from+var.size()]))) {
+ ++from;
+ continue;
+ }
+ return true;
+ }
+ return false;
+}
+
static void AddLocalVariableDecls(const lldb::VariableListSP &var_list_sp,
- StreamString &stream) {
+ StreamString &stream, const std::string &expr) {
for (size_t i = 0; i < var_list_sp->GetSize(); i++) {
lldb::VariableSP var_sp = var_list_sp->GetVariableAtIndex(i);
ConstString var_name = var_sp->GetName();
if (!var_name || var_name == ConstString("this") ||
- var_name == ConstString(".block_descriptor"))
+ var_name == ConstString(".block_descriptor") ||
+ !ExprBodyContainsVar(var_name.AsCString(), expr))
continue;
stream.Printf("using $__lldb_local_vars::%s;\n", var_name.AsCString());
@@ -260,7 +278,7 @@
if (target->GetInjectLocalVariables(&exe_ctx)) {
lldb::VariableListSP var_list_sp =
frame->GetInScopeVariableList(false, true);
- AddLocalVariableDecls(var_list_sp, lldb_local_var_decls);
+ AddLocalVariableDecls(var_list_sp, lldb_local_var_decls, m_body);
}
}
}
Index: packages/Python/lldbsuite/test/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
===================================================================
--- packages/Python/lldbsuite/test/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
+++ packages/Python/lldbsuite/test/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
@@ -155,7 +155,9 @@
frame = thread.GetSelectedFrame()
self.assertTrue(frame.IsValid())
+ self.enable_expression_log()
val = frame.EvaluateExpression("a")
+ self.disable_expression_log_and_check_for_locals(['a'])
self.assertTrue(val.IsValid())
self.assertEqual(val.GetValueAsUnsigned(), 12345)
@@ -189,6 +191,12 @@
self.assertTrue(val.IsValid())
self.assertEqual(val.GetValueAsUnsigned(), 10003)
+ self.enable_expression_log()
+ val = frame.EvaluateExpression("c-b")
+ self.disable_expression_log_and_check_for_locals(['c','b'])
+ self.assertTrue(val.IsValid())
+ self.assertEqual(val.GetValueAsUnsigned(), 1)
+
self.process.Continue()
self.assertTrue(
self.process.GetState() == lldb.eStateStopped,
@@ -211,6 +219,13 @@
self.assertTrue(val.IsValid())
self.assertEqual(val.GetValueAsUnsigned(), 778899)
+ self.enable_expression_log()
+ val = frame.EvaluateExpression("a+b")
+ self.disable_expression_log_and_check_for_locals(['a','b'])
+ self.assertTrue(val.IsValid())
+ self.assertEqual(val.GetValueAsUnsigned(), 3)
+
+
def _load_exe(self):
self.build()
@@ -234,7 +249,9 @@
frame = thread.GetSelectedFrame()
self.assertTrue(frame.IsValid())
+ self.enable_expression_log()
val = frame.EvaluateExpression("a")
+ self.disable_expression_log_and_check_for_locals([])
self.assertTrue(val.IsValid())
self.assertEqual(val.GetValueAsUnsigned(), 112233)
@@ -245,3 +262,23 @@
val = frame.EvaluateExpression("c")
self.assertTrue(val.IsValid())
self.assertEqual(val.GetValueAsUnsigned(), 778899)
+
+ def enable_expression_log(self):
+ log_file = os.path.join(self.getBuildDir(), "expr.log")
+ self.runCmd("log enable -f '%s' lldb expr" % (log_file))
+
+ def disable_expression_log_and_check_for_locals(self, variables):
+ log_file = os.path.join(self.getBuildDir(), "expr.log")
+ self.runCmd("log disable lldb expr")
+ local_var_regex = re.compile(r".*__lldb_local_vars::(.*);")
+ matched = []
+ with open(log_file, 'r') as log:
+ for line in log:
+ if line.find('LLDB_BODY_START') != -1:
+ break
+ m = re.match(local_var_regex, line)
+ if m:
+ self.assertIn(m.group(1), variables)
+ matched.append(m.group(1))
+ self.assertEqual([item for item in variables if item not in matched],
+ [])
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
