[Lldb-commits] [lldb] [lldb] Update dwim-print to support limited variable expression paths (PR #117452)
Dave Lee via lldb-commits
lldb-commits at lists.llvm.org
Mon Dec 2 11:16:32 PST 2024
https://github.com/kastiglione updated https://github.com/llvm/llvm-project/pull/117452
>From cce676f2e8e0ed7a81ae1fa78082765cb698de85 Mon Sep 17 00:00:00 2001
From: Dave Lee <davelee.com at gmail.com>
Date: Sat, 23 Nov 2024 14:37:10 -0800
Subject: [PATCH 1/3] [lldb] Update dwim-print to support limited variable
expression paths
`frame variable` supports nested variable access, which the API calls "variable
expression paths". This change updates `dwim-print` to support a subset of supported
variable expression paths.
Consider the expression `a->b`. In C++, the arrow operator can be overloaded, and where
that is the case, expression evaluation must be used to evaluate it, not frame variable.
Likewise, the subscript operator can be overloaded.
To avoid those cases, this change introduces a limited support for variable expression
paths. Use of the dot operator is allowed.
Additionally, this change allows `dwim-print` to directly access children of `this` and
`self` (see AllowDirectIVarAccess). This functionality is also provided by the same
`GetValueForVariableExpressionPath` method.
rdar://104348908
---
.../Commands/CommandObjectDWIMPrint.cpp | 21 +++++++--
lldb/test/API/commands/dwim-print/Makefile | 2 +-
.../API/commands/dwim-print/TestDWIMPrint.py | 43 ++++++++++++++++---
lldb/test/API/commands/dwim-print/main.c | 14 ------
lldb/test/API/commands/dwim-print/main.cpp | 22 ++++++++++
5 files changed, 76 insertions(+), 26 deletions(-)
delete mode 100644 lldb/test/API/commands/dwim-print/main.c
create mode 100644 lldb/test/API/commands/dwim-print/main.cpp
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 62c4e74d853ad1..842ff6e1466d63 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -151,10 +151,23 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
result.SetStatus(eReturnStatusSuccessFinishResult);
};
- // First, try `expr` as the name of a frame variable.
- if (frame) {
- auto valobj_sp = frame->FindVariable(ConstString(expr));
- if (valobj_sp && valobj_sp->GetError().Success()) {
+ // First, try `expr` as a _limited_ frame variable expression path: only the
+ // dot operator (`.`) is permitted for this case.
+ //
+ // This is limited to support only unambiguous expression paths. Of note,
+ // expression paths are not attempted if the expression contain either the
+ // arrow operator (`->`) or the subscript operator (`[]`). This is because
+ // both operators can be overloaded in C++, and could result in ambiguity in
+ // how the expression is handled. Additionally, `*` and `&` are not supported.
+ bool try_variable_path = expr.find_first_of("*&->[]") == StringRef::npos;
+ if (frame && try_variable_path) {
+ Status status;
+ VariableSP var_sp;
+ auto valobj_sp = frame->GetValueForVariableExpressionPath(
+ expr, eDynamicDontRunTarget,
+ StackFrame::eExpressionPathOptionsAllowDirectIVarAccess, var_sp,
+ status);
+ if (valobj_sp && status.Success() && valobj_sp->GetError().Success()) {
if (!suppress_result) {
if (auto persisted_valobj = valobj_sp->Persist())
valobj_sp = persisted_valobj;
diff --git a/lldb/test/API/commands/dwim-print/Makefile b/lldb/test/API/commands/dwim-print/Makefile
index 10495940055b63..99998b20bcb050 100644
--- a/lldb/test/API/commands/dwim-print/Makefile
+++ b/lldb/test/API/commands/dwim-print/Makefile
@@ -1,3 +1,3 @@
-C_SOURCES := main.c
+CXX_SOURCES := main.cpp
include Makefile.rules
diff --git a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
index b40924a182e37d..492d49f008a9e4 100644
--- a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -16,7 +16,7 @@ def _run_cmd(self, cmd: str) -> str:
self.ci.HandleCommand(cmd, result)
return result.GetOutput().rstrip()
- VAR_IDENT = re.compile(r"(?:\$\d+|\w+) = ")
+ VAR_IDENT = re.compile(r"(?:\$\d+|[\w.]+) = ")
def _strip_result_var(self, string: str) -> str:
"""
@@ -121,30 +121,39 @@ def test_empty_expression(self):
def test_nested_values(self):
"""Test dwim-print with nested values (structs, etc)."""
self.build()
- lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c"))
+ lldbutil.run_to_source_breakpoint(
+ self, "break here", lldb.SBFileSpec("main.cpp")
+ )
self.runCmd("settings set auto-one-line-summaries false")
self._expect_cmd(f"dwim-print s", "frame variable")
self._expect_cmd(f"dwim-print (struct Structure)s", "expression")
def test_summary_strings(self):
- """Test dwim-print with nested values (structs, etc)."""
+ """Test dwim-print with type summaries."""
self.build()
- lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c"))
+ lldbutil.run_to_source_breakpoint(
+ self, "break here", lldb.SBFileSpec("main.cpp")
+ )
self.runCmd("settings set auto-one-line-summaries false")
self.runCmd("type summary add -e -s 'stub summary' Structure")
self._expect_cmd(f"dwim-print s", "frame variable")
self._expect_cmd(f"dwim-print (struct Structure)s", "expression")
+ self.runCmd("type summary delete Structure")
def test_void_result(self):
"""Test dwim-print does not surface an error message for void expressions."""
self.build()
- lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c"))
+ lldbutil.run_to_source_breakpoint(
+ self, "break here", lldb.SBFileSpec("main.cpp")
+ )
self.expect("dwim-print (void)15", matching=False, patterns=["(?i)error"])
def test_preserves_persistent_variables(self):
"""Test dwim-print does not delete persistent variables."""
self.build()
- lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c"))
+ lldbutil.run_to_source_breakpoint(
+ self, "break here", lldb.SBFileSpec("main.cpp")
+ )
self.expect("dwim-print int $i = 15")
# Run the same expression twice and verify success. This ensures the
# first command does not delete the persistent variable.
@@ -154,5 +163,25 @@ def test_preserves_persistent_variables(self):
def test_missing_type(self):
"""The expected output of po opaque is its address (no error)"""
self.build()
- lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c"))
+ lldbutil.run_to_source_breakpoint(
+ self, "break here", lldb.SBFileSpec("main.cpp")
+ )
self.expect("dwim-print -O -- opaque", substrs=["0x"])
+
+ def test_variable_expression_path(self):
+ """Test dwim-print supports certain variable expression paths."""
+ self.build()
+ lldbutil.run_to_source_breakpoint(
+ self, "break here", lldb.SBFileSpec("main.cpp")
+ )
+ self.runCmd("settings set auto-one-line-summaries false")
+ self._expect_cmd("dwim-print w.s", "frame variable")
+ self._expect_cmd("dwim-print wp->s", "expression")
+
+ def test_direct_child_access(self):
+ """Test dwim-print supports accessing members/ivars without qualification."""
+ self.build()
+ lldbutil.run_to_source_breakpoint(
+ self, "break inside", lldb.SBFileSpec("main.cpp")
+ )
+ self._expect_cmd("dwim-print number", "frame variable")
diff --git a/lldb/test/API/commands/dwim-print/main.c b/lldb/test/API/commands/dwim-print/main.c
deleted file mode 100644
index 6bfe645c7c6e09..00000000000000
--- a/lldb/test/API/commands/dwim-print/main.c
+++ /dev/null
@@ -1,14 +0,0 @@
-struct Structure {
- int number;
-};
-
-struct Opaque;
-int puts(const char *s);
-
-int main(int argc, char **argv) {
- struct Structure s;
- s.number = 30;
- struct Opaque *opaque = &s;
- puts("break here");
- return 0;
-}
diff --git a/lldb/test/API/commands/dwim-print/main.cpp b/lldb/test/API/commands/dwim-print/main.cpp
new file mode 100644
index 00000000000000..d1abb5a85dd450
--- /dev/null
+++ b/lldb/test/API/commands/dwim-print/main.cpp
@@ -0,0 +1,22 @@
+extern "C" int puts(const char *s);
+
+struct Structure {
+ int number = 30;
+ void f() { puts("break inside"); }
+};
+
+struct Wrapper {
+ Structure s;
+};
+
+struct Opaque;
+
+int main(int argc, char **argv) {
+ Structure s;
+ Wrapper w;
+ Wrapper *wp = &w;
+ Opaque *opaque = (Opaque *)(void *)&s;
+ puts("break here");
+ s.f();
+ return 0;
+}
>From 0d10c4a903eb94e9b533314e8702bc2b735af48f Mon Sep 17 00:00:00 2001
From: Dave Lee <davelee.com at gmail.com>
Date: Sat, 23 Nov 2024 17:03:39 -0800
Subject: [PATCH 2/3] Use command's dynamic type value
---
lldb/source/Commands/CommandObjectDWIMPrint.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 842ff6e1466d63..f6d1faf37bec2f 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -161,10 +161,10 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
// how the expression is handled. Additionally, `*` and `&` are not supported.
bool try_variable_path = expr.find_first_of("*&->[]") == StringRef::npos;
if (frame && try_variable_path) {
- Status status;
VariableSP var_sp;
+ Status status;
auto valobj_sp = frame->GetValueForVariableExpressionPath(
- expr, eDynamicDontRunTarget,
+ expr, eval_options.GetUseDynamic(),
StackFrame::eExpressionPathOptionsAllowDirectIVarAccess, var_sp,
status);
if (valobj_sp && status.Success() && valobj_sp->GetError().Success()) {
>From 162e822ed513e79978cb5e800c988591d66a440a Mon Sep 17 00:00:00 2001
From: Dave Lee <davelee.com at gmail.com>
Date: Mon, 2 Dec 2024 11:15:42 -0800
Subject: [PATCH 3/3] Add const to bool var
---
lldb/source/Commands/CommandObjectDWIMPrint.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index f6d1faf37bec2f..d4d038d28f6755 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -159,7 +159,8 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
// arrow operator (`->`) or the subscript operator (`[]`). This is because
// both operators can be overloaded in C++, and could result in ambiguity in
// how the expression is handled. Additionally, `*` and `&` are not supported.
- bool try_variable_path = expr.find_first_of("*&->[]") == StringRef::npos;
+ const bool try_variable_path =
+ expr.find_first_of("*&->[]") == StringRef::npos;
if (frame && try_variable_path) {
VariableSP var_sp;
Status status;
More information about the lldb-commits
mailing list