[LLDB] Update DIL handling of array subscripting. #151605

cmtice · 2025-07-31T22:11:38Z

This updates the DIL code for handling array subscripting to more closely match and handle all the cases from the original 'frame var' implementation. Also updates the DIL array subscripting test. This particularly fixes some issues with handling synthetic children, objc pointers, and accessing specific bits within scalar data types.

llvmbot · 2025-07-31T22:12:08Z

@llvm/pr-subscribers-lldb

Author: None (cmtice)

Changes

This updates the DIL code for handling array subscripting to more closely match and handle all the cases from the original 'frame var' implementation. Also updates the DIL array subscripting test. This particularly fixes some issues with handling synthetic children, objc pointers, and accessing specific bits within scalar data types.

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

2 Files Affected:

(modified) lldb/source/ValueObject/DILEval.cpp (+126-29)
(modified) lldb/test/API/commands/frame/var-dil/basics/ArraySubscript/TestFrameVarDILArraySubscript.py (+9-7)

diff --git a/lldb/source/ValueObject/DILEval.cpp b/lldb/source/ValueObject/DILEval.cpp
index 6f28434c646cd..527fd46cab1cc 100644
--- a/lldb/source/ValueObject/DILEval.cpp
+++ b/lldb/source/ValueObject/DILEval.cpp
@@ -323,6 +323,7 @@ Interpreter::Visit(const MemberOfNode *node) {
       m_expr, errMsg, node->GetLocation(), node->GetFieldName().size());
 }
 
+
 llvm::Expected<lldb::ValueObjectSP>
 Interpreter::Visit(const ArraySubscriptNode *node) {
   auto lhs_or_err = Evaluate(node->GetBase());
@@ -330,40 +331,136 @@ Interpreter::Visit(const ArraySubscriptNode *node) {
     return lhs_or_err;
   lldb::ValueObjectSP base = *lhs_or_err;
 
-  // Check to see if 'base' has a synthetic value; if so, try using that.
+  StreamString var_expr_path_strm;
   uint64_t child_idx = node->GetIndex();
-  if (lldb::ValueObjectSP synthetic = base->GetSyntheticValue()) {
-    llvm::Expected<uint32_t> num_children =
-        synthetic->GetNumChildren(child_idx + 1);
-    if (!num_children)
-      return llvm::make_error<DILDiagnosticError>(
-          m_expr, toString(num_children.takeError()), node->GetLocation());
-    if (child_idx >= *num_children) {
-      std::string message = llvm::formatv(
-          "array index {0} is not valid for \"({1}) {2}\"", child_idx,
-          base->GetTypeName().AsCString("<invalid type>"),
-          base->GetName().AsCString());
-      return llvm::make_error<DILDiagnosticError>(m_expr, message,
+  lldb::ValueObjectSP child_valobj_sp;
+  bool is_incomplete_array = false;
+  CompilerType base_type = base->GetCompilerType().GetNonReferenceType();
+  base->GetExpressionPath(var_expr_path_strm);
+  if (base_type.IsPointerType()) {
+    bool is_objc_pointer = true;
+    if (base->GetCompilerType().GetMinimumLanguage() != lldb::eLanguageTypeObjC)
+      is_objc_pointer = false;
+    else if (!base_type.IsPointerType())
+      is_objc_pointer = false;
+
+    if (is_objc_pointer && !m_use_synthetic) {
+      std::string errMsg =
+          llvm::formatv("\"(({0}) {1}\" is an Objective-C pointer, and cannot "
+                        "be subscripted",
+                        base->GetTypeName().AsCString("<invalid type>"),
+                        base->GetName());
+      return llvm::make_error<DILDiagnosticError>(m_expr, errMsg,
+                                                  node->GetLocation());
+    } else if (is_objc_pointer) {
+      lldb::ValueObjectSP synthetic = base->GetSyntheticValue();
+      if (!synthetic || synthetic == base) {
+        std::string errMsg =
+            llvm::formatv("\"({0}) {1}\" is not an array type",
+                          base->GetTypeName().AsCString("<invalid type>"),
+                          base->GetName());
+        return llvm::make_error<DILDiagnosticError>(m_expr, errMsg,
+                                                    node->GetLocation());
+      } else if (static_cast<uint32_t>(child_idx) >=
+                 synthetic->GetNumChildrenIgnoringErrors()) {
+        std::string errMsg =
+            llvm::formatv("array index {0} is not valid for \"({1}) {2}\"",
+                          child_idx,
+                          base->GetTypeName().AsCString("<invalid type>"),
+                          var_expr_path_strm.GetData());
+        return llvm::make_error<DILDiagnosticError>(m_expr, errMsg,
+                                                    node->GetLocation());
+      } else {
+        child_valobj_sp = synthetic->GetChildAtIndex(child_idx);
+        if (!child_valobj_sp) {
+          std::string errMsg =
+              llvm::formatv("array index {0} is not valid for \"({1}) {2}\"",
+                            child_idx,
+                            base->GetTypeName().AsCString("<invalid type>"),
+                            var_expr_path_strm.GetData());
+          return llvm::make_error<DILDiagnosticError>(m_expr, errMsg,
+                                                      node->GetLocation());
+        }
+      }
+    } else { // it's not an objc pointer
+      child_valobj_sp = base->GetSyntheticArrayMember(child_idx, true);
+      if (!child_valobj_sp) {
+        std::string errMsg =
+            llvm::formatv("failed to use pointer as array for index {0} for "
+                          "\"({1}) {2}\"", child_idx,
+                          base->GetTypeName().AsCString("<invalid type>"),
+                          var_expr_path_strm.GetData());
+        if (base_type.IsPointerToVoid())
+          errMsg = "subscript of pointer to incomplete type 'void'";
+        return llvm::make_error<DILDiagnosticError>(m_expr, errMsg,
+                                                    node->GetLocation());
+      }
+    }
+  } else if (base_type.IsArrayType(
+      nullptr, nullptr, &is_incomplete_array)) {
+    child_valobj_sp = base->GetChildAtIndex(child_idx);
+    if (!child_valobj_sp && (is_incomplete_array || m_use_synthetic))
+      child_valobj_sp = base->GetSyntheticArrayMember(child_idx, true);
+    if (!child_valobj_sp) {
+      std::string errMsg =
+          llvm::formatv("array index {0} is not valid for \"({1}) {2}\"",
+                        child_idx,
+                        base->GetTypeName().AsCString("<invalid type>"),
+                        var_expr_path_strm.GetData());
+      return llvm::make_error<DILDiagnosticError>(m_expr, errMsg,
                                                   node->GetLocation());
     }
-    if (lldb::ValueObjectSP child_valobj_sp =
-            synthetic->GetChildAtIndex(child_idx))
-      return child_valobj_sp;
+  } else if (base_type.IsScalarType()) {
+    child_valobj_sp =
+        base->GetSyntheticBitFieldChild(child_idx, child_idx, true);
+    if (!child_valobj_sp) {
+      std::string errMsg =
+          llvm::formatv("bitfield range {0}-{1} is not valid for \"({2}) {3}\"",
+                        child_idx, child_idx,
+                        base->GetTypeName().AsCString("<invalid type>"),
+                        var_expr_path_strm.GetData());
+      return llvm::make_error<DILDiagnosticError>(m_expr, errMsg,
+                                                  node->GetLocation(), 1);
+    }
+  } else {
+    lldb::ValueObjectSP synthetic = base->GetSyntheticValue();
+    if (!m_use_synthetic || !synthetic || synthetic == base) {
+      std::string errMsg =
+          llvm::formatv("\"{0}\" is not an array type",
+                        base->GetTypeName().AsCString("<invalid type>"));
+      return llvm::make_error<DILDiagnosticError>(m_expr, errMsg,
+                                                  node->GetLocation(), 1);
+    } else if (static_cast<uint32_t>(child_idx) >=
+               synthetic->GetNumChildrenIgnoringErrors()) {
+      std::string errMsg =
+          llvm::formatv("array index {0} is not valid for \"({1}) {2}\"",
+                        child_idx,
+                        base->GetTypeName().AsCString("<invalid type>"),
+                        var_expr_path_strm.GetData());
+      return llvm::make_error<DILDiagnosticError>(m_expr, errMsg,
+                                                  node->GetLocation(), 1);
+    } else {
+      child_valobj_sp = synthetic->GetChildAtIndex(child_idx);
+      if (!child_valobj_sp) {
+        std::string errMsg =
+            llvm::formatv("array index {0} is not valid for \"({1}) {2}\"",
+                          child_idx,
+                          base->GetTypeName().AsCString("<invalid type>"),
+                          var_expr_path_strm.GetData());
+        return llvm::make_error<DILDiagnosticError>(m_expr, errMsg,
+                                                    node->GetLocation(), 1);
+      }
+    }
   }
 
-  auto base_type = base->GetCompilerType().GetNonReferenceType();
-  if (!base_type.IsPointerType() && !base_type.IsArrayType())
-    return llvm::make_error<DILDiagnosticError>(
-        m_expr, "subscripted value is not an array or pointer",
-        node->GetLocation());
-  if (base_type.IsPointerToVoid())
-    return llvm::make_error<DILDiagnosticError>(
-        m_expr, "subscript of pointer to incomplete type 'void'",
-        node->GetLocation());
-
-  if (base_type.IsArrayType()) {
-    if (lldb::ValueObjectSP child_valobj_sp = base->GetChildAtIndex(child_idx))
-      return child_valobj_sp;
+  if (child_valobj_sp) {
+    if (m_use_dynamic != lldb::eNoDynamicValues) {
+      lldb::ValueObjectSP dynamic_value_sp(
+          child_valobj_sp->GetDynamicValue(m_use_dynamic));
+      if (dynamic_value_sp)
+        child_valobj_sp = dynamic_value_sp;
+    }
+    return child_valobj_sp;
   }
 
   int64_t signed_child_idx = node->GetIndex();
diff --git a/lldb/test/API/commands/frame/var-dil/basics/ArraySubscript/TestFrameVarDILArraySubscript.py b/lldb/test/API/commands/frame/var-dil/basics/ArraySubscript/TestFrameVarDILArraySubscript.py
index 0f56057189395..769bed2acb02c 100644
--- a/lldb/test/API/commands/frame/var-dil/basics/ArraySubscript/TestFrameVarDILArraySubscript.py
+++ b/lldb/test/API/commands/frame/var-dil/basics/ArraySubscript/TestFrameVarDILArraySubscript.py
@@ -69,17 +69,19 @@ def test_subscript(self):
             substrs=["expected 'r_square', got: <'.'"],
         )
 
-        # Base should be a "pointer to T" and index should be of an integral type.
-        self.expect(
-            "frame var 'idx_1[0]'",
-            error=True,
-            substrs=["subscripted value is not an array or pointer"],
-        )
+
+        # Test accessing bits in scalar types.
+        self.expect_var_path("idx_1[0]", value="1")
+        self.expect_var_path("idx_1[1]", value="0")
+
+        # Bit acess not valid for a reference.
         self.expect(
             "frame var 'idx_1_ref[0]'",
             error=True,
-            substrs=["subscripted value is not an array or pointer"],
+            substrs=["bitfield range 0-0 is not valid"],
         )
+
+        # Base should be a "pointer to T" and index should be of an integral type.
         self.expect(
             "frame var 'int_arr[int_ptr]'",
             error=True,

github-actions · 2025-07-31T22:14:03Z

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

github-actions · 2025-07-31T22:14:03Z

✅ With the latest revision this PR passed the C/C++ code formatter.

kuilpd · 2025-08-01T14:39:41Z

lldb/source/ValueObject/DILEval.cpp

+    } else if (is_objc_pointer) {
+      lldb::ValueObjectSP synthetic = base->GetSyntheticValue();
+      if (!synthetic || synthetic == base) {
+        std::string errMsg =
+            llvm::formatv("\"({0}) {1}\" is not an array type",
+                          base->GetTypeName().AsCString("<invalid type>"),
+                          base->GetName());
+        return llvm::make_error<DILDiagnosticError>(m_expr, errMsg,
+                                                    node->GetLocation());
+      } else if (static_cast<uint32_t>(child_idx) >=
+                 synthetic->GetNumChildrenIgnoringErrors()) {
+        std::string errMsg =
+            llvm::formatv("array index {0} is not valid for \"({1}) {2}\"",
+                          child_idx,
+                          base->GetTypeName().AsCString("<invalid type>"),
+                          var_expr_path_strm.GetData());
+        return llvm::make_error<DILDiagnosticError>(m_expr, errMsg,
+                                                    node->GetLocation());
+      } else {
+        child_valobj_sp = synthetic->GetChildAtIndex(child_idx);
+        if (!child_valobj_sp) {
+          std::string errMsg =
+              llvm::formatv("array index {0} is not valid for \"({1}) {2}\"",
+                            child_idx,
+                            base->GetTypeName().AsCString("<invalid type>"),
+                            var_expr_path_strm.GetData());
+          return llvm::make_error<DILDiagnosticError>(m_expr, errMsg,
+                                                      node->GetLocation());
+        }
+      }


The code in lines 356-383 is basically the same as in lines 426-453, could we reuse this somehow?

kuilpd · 2025-08-01T14:40:06Z

lldb/test/API/commands/frame/var-dil/basics/ArraySubscript/TestFrameVarDILArraySubscript.py

+        self.expect_var_path("idx_1[0]", value="1")
+        self.expect_var_path("idx_1[1]", value="0")
+
+        # Bit acess not valid for a reference.


Suggested change

# Bit acess not valid for a reference.

# Bit access not valid for a reference.

labath

I see the bitfield handling is reflected in the test case, but I don't see anything about handling of synthetic children or objc pointers. For the objc case, I'm very tempted to not copy that bit of code over, since it: a) is untested (or you wouldn't have been able to start using the new implementation); b) doesn't fit into our concept of being language-agnostic.

It looks like that's mainly error handling code guarding a call to GetSyntheticArrayMember. That function eventually calls GetCompilerType().GetChildCompilerTypeAtIndex, so I think that special handling of ObjC should happen there.

That would also reduce the maze of the if statements, which is making the function hard to review.

labath · 2025-08-04T08:38:05Z

lldb/source/ValueObject/DILEval.cpp

+        return llvm::make_error<DILDiagnosticError>(m_expr, errMsg,
+                                                    node->GetLocation());
+      } else if (static_cast<uint32_t>(child_idx) >=
+                 synthetic->GetNumChildrenIgnoringErrors()) {


Suggested change

synthetic->GetNumChildrenIgnoringErrors()) {

synthetic->GetNumChildrenIgnoringErrors(child_idx+1)) {

.. potentially avoids materializing many children unnecessarily

labath · 2025-08-04T08:43:56Z

lldb/source/ValueObject/DILEval.cpp

+      is_objc_pointer = false;
+
+    if (is_objc_pointer && !m_use_synthetic) {
+      std::string errMsg =


labath · 2025-08-04T08:45:46Z

lldb/source/ValueObject/DILEval.cpp

+                        base->GetName());
+      return llvm::make_error<DILDiagnosticError>(m_expr, errMsg,
+                                                  node->GetLocation());
+    } else if (is_objc_pointer) {


https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
(everywhere in this function)

- Remove code testing for objc pointers - No else-after-return - Rename errMsg err_msg - Add child_idx+1 argument to calls to GetNumChildrenIgnoringErrors - Fix typo.

Michael137 · 2025-08-07T07:34:16Z

lldb/source/ValueObject/DILEval.cpp

+          "array index {0} is not valid for \"({1}) {2}\"", child_idx,
+          base->GetTypeName().AsCString("<invalid type>"),
+          var_expr_path_strm.GetData());
+      return llvm::make_error<DILDiagnosticError>(m_expr, err_msg,


You can std::move these err_msgs

Michael137 · 2025-08-07T07:36:19Z

lldb/source/ValueObject/DILEval.cpp

+      lldb::ValueObjectSP dynamic_value_sp(
+          child_valobj_sp->GetDynamicValue(m_use_dynamic));
+      if (dynamic_value_sp)
+        child_valobj_sp = dynamic_value_sp;


Suggested change

lldb::ValueObjectSP dynamic_value_sp(

child_valobj_sp->GetDynamicValue(m_use_dynamic));

if (dynamic_value_sp)

child_valobj_sp = dynamic_value_sp;

if (auto dynamic_sp = child_valobj_sp->GetDynamicValue(m_use_dynamic))

child_valobj_sp = std::move(dynamic_sp);

Add test case for GetSyntheticArrayMember.

cmtice · 2025-08-13T05:02:12Z

I've addressed all the review comments and added the test case. Please review again when you have a few minutes. Thanks!

labath · 2025-08-15T09:47:25Z

lldb/source/ValueObject/DILEval.cpp

+      return llvm::make_error<DILDiagnosticError>(m_expr, std::move(err_msg),
+                                                  node->GetLocation());
+    }
+  } else if (base_type.IsArrayType(nullptr, nullptr, &is_incomplete_array)) {


Maybe too fancy, but reduces the scope of the variable.

Suggested change

} else if (base_type.IsArrayType(nullptr, nullptr, &is_incomplete_array)) {

} else if (bool is_incomplete_array; base_type.IsArrayType(nullptr, nullptr, &is_incomplete_array)) {

This reverts commit 6d3ad9d. This was reverted because it broke the LLDB greendragon bot.

rastogishubham · 2025-08-15T16:18:35Z

Hi, it seems like this change broke greendragon

https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/31450/

It was reverted with cd0bf27

This attempts to fix the issues with the original PR (llvm#151605), updating the DIL code for handling array subscripting to more closely match and handle all the casees from the original 'frame var' implementation. The first PR did not include special-case code for objc pointers, which apparently caused a test failure on the green-dragon buildbot. Hopefully this PR, which includes the objc pointer special code, fixes that issue.

This attempts to fix the issues with the original PR (#151605), updating the DIL code for handling array subscripting to more closely match and handle all the casees from the original 'frame var' implementation. The first PR did not include special-case code for objc pointers, which apparently caused a test failure on the green-dragon buildbot. Hopefully this PR, which includes the objc pointer special code, fixes that issue.

cmtice requested a review from JDevlieghere as a code owner July 31, 2025 22:11

cmtice requested review from kuilpd and labath and removed request for JDevlieghere July 31, 2025 22:11

llvmbot added the lldb label Jul 31, 2025

cmtice requested a review from JDevlieghere July 31, 2025 22:12

cmtice requested review from asl and jimingham July 31, 2025 22:12

kuilpd reviewed Aug 1, 2025

View reviewed changes

labath reviewed Aug 4, 2025

View reviewed changes

cmtice added 2 commits August 6, 2025 22:58

Address reviewer comments:

0f706e8

- Remove code testing for objc pointers - No else-after-return - Rename errMsg err_msg - Add child_idx+1 argument to calls to GetNumChildrenIgnoringErrors - Fix typo.

Fix clang-format issues.

a394c68

Michael137 reviewed Aug 7, 2025

View reviewed changes

cmtice added 2 commits August 12, 2025 21:49

Add std::move for err_msg strings and dynamic value.

fe70937

Add test case for GetSyntheticArrayMember.

Fix clang format issues.

0f8331c

cmtice requested a review from werat August 14, 2025 15:04

labath approved these changes Aug 15, 2025

View reviewed changes

cmtice merged commit 6d3ad9d into llvm:main Aug 15, 2025
9 checks passed

rastogishubham added a commit that referenced this pull request Aug 15, 2025

Revert "[LLDB] Update DIL handling of array subscripting. (#151605)"

cd0bf27

This reverts commit 6d3ad9d. This was reverted because it broke the LLDB greendragon bot.

cmtice mentioned this pull request Aug 19, 2025

[LLDB] Re-land 'Update DIL handling of array subscripting' #154269

Merged

aemerson mentioned this pull request Aug 30, 2025

[NFC] [clangd] [Modules] remove dot in log #156207

Closed

	# Bit acess not valid for a reference.
	# Bit access not valid for a reference.

	synthetic->GetNumChildrenIgnoringErrors()) {
	synthetic->GetNumChildrenIgnoringErrors(child_idx+1)) {

	} else if (base_type.IsArrayType(nullptr, nullptr, &is_incomplete_array)) {
	} else if (bool is_incomplete_array; base_type.IsArrayType(nullptr, nullptr, &is_incomplete_array)) {

[LLDB] Update DIL handling of array subscripting. #151605

[LLDB] Update DIL handling of array subscripting. #151605

Uh oh!

Conversation

cmtice commented Jul 31, 2025

Uh oh!

llvmbot commented Jul 31, 2025

Uh oh!

github-actions bot commented Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

labath left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cmtice commented Aug 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rastogishubham commented Aug 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

github-actions bot commented Jul 31, 2025 •

edited

Loading

github-actions bot commented Jul 31, 2025 •

edited

Loading