[lldb] fix a problem in the ValueObject::GetExpressionPath method

[sedymrak]

Consider the following program:

int main() {
  int foo[2][3][4];
  int (*bar)[3][4] = foo;
  return 0;
}

If we:

• compile this program
• launch an LLDB debugging session
• launch the process and let it stop at the return 0; statement
  then the following LLDB command:

(lldb) script lldb.frame.FindVariable("bar").GetChildAtIndex(0).get_expr_path()

will produce the following output:

bar->[0]

What we were expecting:

• a valid expression in the C programming language
• that would allow us (in the scope of the main function) access the appropriate object.

What we've got is a string that does not represent a valid expression in the C programming language.

This pull-request proposes a fix to this problem.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Matej Košík (sedymrak)

Changes

Consider the following program:

int main() {
  int foo[2][3][4];
  int (*bar)[3][4] = foo;
  return 0;
}

If we:

• compile this program
• launch an LLDB debugging session
• launch the process and let it stop at the return 0; statement
  then the following LLDB command:

(lldb) script lldb.frame.FindVariable("bar").GetChildAtIndex(0).get_expr_path()

will produce the following output:

bar->[0]

What we were expecting:

• a valid expression in the C programming language
• that would allow us (in the scope of the main function) access the appropriate object.

What we've got is a string that does not represent a valid expression in the C programming language.

This pull-request proposes a fix to this problem.

---

Full diff: https://github.com/llvm/llvm-project/pull/171521.diff

4 Files Affected:

• (modified) lldb/source/ValueObject/ValueObject.cpp (+14)
• (added) lldb/test/API/python_api/value/get_expr_path/Makefile (+3)
• (added) lldb/test/API/python_api/value/get_expr_path/TestValueAPIGetExpressionPath.py (+50)
• (added) lldb/test/API/python_api/value/get_expr_path/main.c (+5)

diff --git a/lldb/source/ValueObject/ValueObject.cpp b/lldb/source/ValueObject/ValueObject.cpp
index aeea32f19ee2c..3bbb917e51976 100644
--- a/lldb/source/ValueObject/ValueObject.cpp
+++ b/lldb/source/ValueObject/ValueObject.cpp
@@ -2138,6 +2138,20 @@ void ValueObject::GetExpressionPath(Stream &s,
 
   ValueObject *parent = GetParent();
 
+  if (parent) {
+    lldb_private::CompilerType parentType = parent->GetCompilerType();
+    const bool parentTypeIsPointer = parentType.IsPointerType();
+    const bool pointeeOfParentTypeIsArray =
+        parentType.GetPointeeType().IsArrayType(nullptr, nullptr, nullptr);
+    if (parentTypeIsPointer && pointeeOfParentTypeIsArray) {
+      s.PutCString("(*");
+      parent->GetExpressionPath(s, epformat);
+      s.PutCString(")");
+      s.PutCString(GetName().GetCString());
+      return;
+    }
+  }
+
   if (parent)
     parent->GetExpressionPath(s, epformat);
 
diff --git a/lldb/test/API/python_api/value/get_expr_path/Makefile b/lldb/test/API/python_api/value/get_expr_path/Makefile
new file mode 100644
index 0000000000000..10495940055b6
--- /dev/null
+++ b/lldb/test/API/python_api/value/get_expr_path/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
diff --git a/lldb/test/API/python_api/value/get_expr_path/TestValueAPIGetExpressionPath.py b/lldb/test/API/python_api/value/get_expr_path/TestValueAPIGetExpressionPath.py
new file mode 100644
index 0000000000000..227588c412587
--- /dev/null
+++ b/lldb/test/API/python_api/value/get_expr_path/TestValueAPIGetExpressionPath.py
@@ -0,0 +1,50 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ValueAPIGetExpressionPath(TestBase):
+    def test(self):
+        self.build()
+
+        _, _, thread, _ = lldbutil.run_to_source_breakpoint(
+            self, "Break at this line", lldb.SBFileSpec("main.c")
+        )
+        frame = thread.GetFrameAtIndex(0)
+
+        self.assertEqual(frame.FindVariable("foo").get_expr_path(), "foo")
+        for i in range(2):
+            self.assertEqual(
+                frame.FindVariable("foo").GetChildAtIndex(i).get_expr_path(),
+                f"foo[{i}]",
+            )
+            for j in range(3):
+                self.assertEqual(
+                    frame.FindVariable("foo")
+                    .GetChildAtIndex(i)
+                    .GetChildAtIndex(j)
+                    .get_expr_path(),
+                    f"foo[{i}][{j}]",
+                )
+                for k in range(4):
+                    self.assertEqual(
+                        frame.FindVariable("foo")
+                        .GetChildAtIndex(i)
+                        .GetChildAtIndex(j)
+                        .GetChildAtIndex(k)
+                        .get_expr_path(),
+                        f"foo[{i}][{j}][{k}]",
+                    )
+        self.assertEqual(frame.FindVariable("bar").get_expr_path(), "bar")
+        for j in range(3):
+            self.assertEqual(
+                frame.FindVariable("bar").GetChildAtIndex(j).get_expr_path(), f"(*bar)[{j}]"
+            )
+            for k in range(4):
+                self.assertEqual(
+                    frame.FindVariable("bar")
+                    .GetChildAtIndex(j)
+                    .GetChildAtIndex(k)
+                    .get_expr_path(),
+                    f"(*bar)[{j}][{k}]",
+                )
diff --git a/lldb/test/API/python_api/value/get_expr_path/main.c b/lldb/test/API/python_api/value/get_expr_path/main.c
new file mode 100644
index 0000000000000..f6fcb11e36835
--- /dev/null
+++ b/lldb/test/API/python_api/value/get_expr_path/main.c
@@ -0,0 +1,5 @@
+int main() {
+  int foo[2][3][4];
+  int (*bar)[3][4] = foo;
+  return 0; // Break at this line
+}

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[jimingham]

This is more a meta-point, but get_expression_path's job is not to produce a "valid expression in the C programming language". It produces a valid expression in lldb's DIL (Data Inspection Language). That is the C-like lldb invention for inspecting data as produced by the ValueObject system (including the naming that Synthetic Child Providers allow) - which often isn't exactly what the language implementing the type would use.

The difference is that these expressions are expected to round-trip through "ValueObject::GetValueForExpressionPath". They are NOT guaranteed to round-trip through EvaluateExpression.

This is only a side-comment, however, because bar->[0] is not a valid expression in the DIL either:

>>> var = lldb.frame.FindVariable("bar")
>>> var
(int (*)[3][4]) bar = 0x000000016fdfe9c8
>>> expr = var.GetChildAtIndex(0).get_expr_path()
>>> expr
'bar->[0]'
>>> new_val = var.GetValueForExpressionPath("->[0]")
>>> new_val
No value
>>> new_val.GetError()
error: error: invalid value object

[jimingham]

But it is germane to the solution, however. Since the expression path in this case is just "[0][0]":

>>> var = lldb.frame.FindVariable("bar")
>>> elem = var.GetChildAtIndex(0)
>>> elem
(int[4]) [0] = ([0] = -1859049328, [1] = 1, [2] = -9611056, [3] = 1)
>>> new_val = var.GetValueForExpressionPath("[0][0]")
>>> new_val
(int[4]) [0] = ([0] = -1859049328, [1] = 1, [2] = -9611056, [3] = 1)

whereas:

>>> new_val = var.GetValueForExpressionPath("(*bar)[0]")
>>> new_val
No value
>>> 

[sedymrak]

@jimingham Thank you for your help. I have pushed a commit that tries to address the feedback you have given me so far. Hopefully, I haven't misunderstood you. I was not aware of the DIL concept.

[jimingham]

Might be worth putting a comment in the code you added to GetExpressionPath, saying that it is avoiding adding a spurious -> as an aid to people reading this code in the future.
Otherwise, LGTM.