Add support for generating debug-info for structured bindings of structs and arrays

[shafik]

Summary

Currently we are not emitting debug-info for all cases of structured bindings a C++17 feature which allows us to bind names to subobjects in an initializer.

A structured binding is represented by a DecompositionDecl AST node and the binding are represented by a BindingDecl. It looks the original implementation
only covered the tuple like case which be represented by a DeclRefExpr which contains a VarDecl.

If the binding is to a subobject of the struct the binding will contain a MemberExpr and in the case of arrays it will contain an ArraySubscriptExpr.
This PR adds support emitting debug-info for the MemberExpr and ArraySubscriptExpr cases as well as llvm and lldb tests for these cases as well as the tuple case.

[aprantl]

I think this looks pretty good! I have a few questions inside.

clang/lib/CodeGen/CGDebugInfo.cpp

4663

13 seems to be unnecessarily high. Shouldn't 1 be enough for the expected single DW_OP_deref?

4696

this looks good!

4709

Can C++ arrays ever have a non-zero stride? (perhaps due to element alignment?)

4712

Should there be an else { assert("unhandled binding expressions"); } here or are there other expressions that just don't need special handling?

clang/test/CodeGenCXX/debug-info-structured-binding.cpp

4

We should check what F is, too, right?

lldb/test/API/lang/cpp/structured-binding/main.cpp

70

Nice!

[aprantl]

clang/lib/CodeGen/CGDebugInfo.cpp

4643

do you need a

if (DebugKind > codegenoptions::LimitedDebugInfo)
 return

here?

[shafik]

clang/lib/CodeGen/CGDebugInfo.cpp

4663

Good catch, I used the VarDecl version of EmitDeclare as an example and I copied it from there.

I will also be pushing the DW_OP_plus_uconst and the value later on as well. So maybe 3 is a the right value.

4709

Elements must be contiguous and start at 0 see dcl.array/p6:

An object of type “array of N U” consists of a contiguously allocated non-empty set of N subobjects of type U, known as the elements of the array, and numbered 0 to N-1.

The size of the type should include any padding.

4712

We should only have three cases:

• struct
• array
• tuple-like

We exit early for the tuple-like case at the top b/c we handle that elsewhere.

So maybe asserting we never end up with a not handled case may make sense.

[shafik]

Addressed comments on SmallVector size and fixed test.

clang/lib/CodeGen/CGDebugInfo.cpp

4643

That kind of check is only used in limited places, why would it apply here as let's say opposed to the EmitDeclare for VarDecl case?

clang/test/CodeGenCXX/debug-info-structured-binding.cpp

4

Actually I should have used a different match, that was a mistake.

[aprantl]

clang/lib/CodeGen/CGDebugInfo.cpp

4643

I think the answer is that Ty would be null then.

clang/lib/CodeGen/CGDebugInfo.h

private:
   /// Emit call to llvm.dbg.declare for a variable declaration.
   /// Returns a pointer to the DILocalVariable associated with the
   /// llvm.dbg.declare, or nullptr otherwise.
   llvm::DILocalVariable *EmitDeclare(const VarDecl *decl, llvm::Value *AI,
                                      llvm::Optional<unsigned> ArgNo,
                                      CGBuilderTy &Builder,
                                      const bool UsePointerValue = false);
 
+  /// Emit call to llvm.dbg.declare for a binding declaration.
+  /// Returns a pointer to the DILocalVariable associated with the
+  /// llvm.dbg.declare, or nullptr otherwise.
+  llvm::DILocalVariable *EmitDeclare(const BindingDecl *decl, llvm::Value *AI,
+                                     llvm::Optional<unsigned> ArgNo,
+                                     CGBuilderTy &Builder,
+                                     const bool UsePointerValue = false);
+
   struct BlockByRefType {
     /// The wrapper struct used inside the __block_literal struct.
     llvm::DIType *BlockByRefWrapper;
     /// The type as it appears in the source code.
     llvm::DIType *WrappedType;
   };
 
   std::string GetName(const Decl*, bool Qualified = false) const;

clang/lib/CodeGen/CGDebugInfo.cpp

llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const VarDecl *VD,
   DBuilder.insertDeclare(Storage, D, DBuilder.createExpression(Expr),
                          llvm::DILocation::get(CGM.getLLVMContext(), Line,
                                                Column, Scope, CurInlinedAt),
                          Builder.GetInsertBlock());
 
   return D;
 }
 
+llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const BindingDecl *BD,
+                                                llvm::Value *Storage,
+                                                llvm::Optional<unsigned> ArgNo,
+                                                CGBuilderTy &Builder,
+                                                const bool UsePointerValue) {
+  assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
+  assert(!LexicalBlockStack.empty() && "Region stack mismatch, stack empty!");
+  if (BD->hasAttr<NoDebugAttr>())
+    return nullptr;
+
+  // Skip the tuple like case, we don't handle that here
+  if (isa<DeclRefExpr>(BD->getBinding()))
+    return nullptr;
+
+  llvm::DIFile *Unit = getOrCreateFile(BD->getLocation());
+  llvm::DIType *Ty = getOrCreateType(BD->getType(), Unit);
+
+  // If there is no debug info for this type then do not emit debug info
+  // for this variable.
+  if (!Ty)
+    return nullptr;
+
+  auto Align = getDeclAlignIfRequired(BD, CGM.getContext());
+  unsigned AddressSpace = CGM.getContext().getTargetAddressSpace(BD->getType());
+
+  SmallVector<uint64_t, 3> Expr;
+  AppendAddressSpaceXDeref(AddressSpace, Expr);
+
+  // Clang stores the sret pointer provided by the caller in a static alloca.
+  // Use DW_OP_deref to tell the debugger to load the pointer and treat it as
+  // the address of the variable.
+  if (UsePointerValue) {
+    assert(!llvm::is_contained(Expr, llvm::dwarf::DW_OP_deref) &&
+           "Debug info already contains DW_OP_deref.");
+    Expr.push_back(llvm::dwarf::DW_OP_deref);
+  }
+
+  unsigned Line = getLineNumber(BD->getLocation());
+  unsigned Column = getColumnNumber(BD->getLocation());
+  StringRef Name = BD->getName();
+  auto *Scope = cast<llvm::DIScope>(LexicalBlockStack.back());
+  // Create the descriptor for the variable.
+  llvm::DILocalVariable *D = DBuilder.createAutoVariable(
+      Scope, Name, Unit, Line, Ty, CGM.getLangOpts().Optimize,
+      llvm::DINode::FlagZero, Align);
+
+  if (const MemberExpr *ME = dyn_cast<MemberExpr>(BD->getBinding())) {
+    if (const FieldDecl *FD = dyn_cast<FieldDecl>(ME->getMemberDecl())) {
+      const unsigned fieldIndex = FD->getFieldIndex();
+      const clang::CXXRecordDecl *parent =
+          (const CXXRecordDecl *)FD->getParent();
+      const ASTRecordLayout &layout =
+          CGM.getContext().getASTRecordLayout(parent);
+      const uint64_t fieldOffset = layout.getFieldOffset(fieldIndex);
+
+      if (fieldOffset != 0) {
+        Expr.push_back(llvm::dwarf::DW_OP_plus_uconst);
+        Expr.push_back(
+            CGM.getContext().toCharUnitsFromBits(fieldOffset).getQuantity());
+      }
+    }
+  } else if (const ArraySubscriptExpr *ASE =
+                 dyn_cast<ArraySubscriptExpr>(BD->getBinding())) {
+    if (const IntegerLiteral *IL = dyn_cast<IntegerLiteral>(ASE->getIdx())) {
+      const uint64_t value = IL->getValue().getZExtValue();
+      const uint64_t typeSize = CGM.getContext().getTypeSize(BD->getType());
+
+      if (value != 0) {
+        Expr.push_back(llvm::dwarf::DW_OP_plus_uconst);
+        Expr.push_back(CGM.getContext()
+                           .toCharUnitsFromBits(value * typeSize)
+                           .getQuantity());
+      }
+    }
+  }
+
+  // Insert an llvm.dbg.declare into the current block.
+  DBuilder.insertDeclare(Storage, D, DBuilder.createExpression(Expr),
+                         llvm::DILocation::get(CGM.getLLVMContext(), Line,
+                                               Column, Scope, CurInlinedAt),
+                         Builder.GetInsertBlock());
+
+  return D;
+}
+
 llvm::DILocalVariable *
 CGDebugInfo::EmitDeclareOfAutoVariable(const VarDecl *VD, llvm::Value *Storage,
                                        CGBuilderTy &Builder,
                                        const bool UsePointerValue) {
   assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
+
+  if (auto *DD = dyn_cast<DecompositionDecl>(VD))
+    for (auto *B : DD->bindings()) {
+      EmitDeclare(B, Storage, llvm::None, Builder,
+                  VD->getType()->isReferenceType());
+    }
+
   return EmitDeclare(VD, Storage, llvm::None, Builder, UsePointerValue);
 }
 
 void CGDebugInfo::EmitLabel(const LabelDecl *D, CGBuilderTy &Builder) {
   assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
   assert(!LexicalBlockStack.empty() && "Region stack mismatch, stack empty!");
 
   if (D->hasAttr<NoDebugAttr>())

clang/test/CodeGenCXX/debug-info-structured-binding.cpp

+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone -triple %itanium_abi_triple %s -o - | FileCheck %s
+
+// CHECK: call void @llvm.dbg.declare(metadata %struct.A* %{{[0-9]+}}, metadata !{{[0-9]+}}, metadata !DIExpression())
+// CHECK: call void @llvm.dbg.declare(metadata %struct.A* %{{[0-9]+}}, metadata !{{[0-9]+}}, metadata !DIExpression(DW_OP_plus_uconst, {{[0-9]+}}))
+// CHECK: call void @llvm.dbg.declare(metadata %struct.A* %{{[0-9]+}}, metadata !{{[0-9]+}}, metadata !DIExpression())
+// CHECK: call void @llvm.dbg.declare(metadata %struct.A** %{{[0-9]+}}, metadata !{{[0-9]+}}, metadata !DIExpression(DW_OP_deref))
+// CHECK: call void @llvm.dbg.declare(metadata %struct.A** %{{[0-9]+}}, metadata !{{[0-9]+}}, metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst, {{[0-9]+}}))
+// CHECK: call void @llvm.dbg.declare(metadata %struct.A** %{{[0-9]+}}, metadata !{{[0-9]+}}, metadata !DIExpression())
+struct A {
+  int x;
+  int y;
+};
+
+int f() {
+  A a{10, 20};
+  auto [x1, y1] = a;
+  auto &[x2, y2] = a;
+  return x1 + y1 + x2 + y2;
+}

lldb/test/API/lang/cpp/structured-binding/Makefile

+CXX_SOURCES := main.cpp
+CXXFLAGS_EXTRAS := -std=c++17
+
+include Makefile.rules

lldb/test/API/lang/cpp/structured-binding/TestStructuredBinding.py

+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestStructuredBinding(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    @skipIf(compiler="clang", compiler_version=['<', '14.0'])
+    def test(self):
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.cpp"))
+
+        self.expect_expr("a1", result_type="A",
+            result_children=[ValueCheck(name="x", type="int"),
+                             ValueCheck(name="y", type="int")])
+        self.expect_expr("b1", result_type="char", result_value="'a'")
+        self.expect_expr("c1", result_type="char", result_value="'b'")
+        self.expect_expr("d1", result_type="short", result_value="50")
+        self.expect_expr("e1", result_type="int", result_value="60")
+        self.expect_expr("f1", result_type="char", result_value="'c'")
+
+        self.expect_expr("a2", result_type="A",
+            result_children=[ValueCheck(name="x", type="int"),
+                             ValueCheck(name="y", type="int")])
+        self.expect_expr("b2", result_type="char", result_value="'a'")
+        self.expect_expr("c2", result_type="char", result_value="'b'")
+        self.expect_expr("d2", result_type="short", result_value="50")
+        self.expect_expr("e2", result_type="int", result_value="60")
+        self.expect_expr("f2", result_type="char", result_value="'c'")
+
+        self.expect_expr("a3", result_type="A",
+            result_children=[ValueCheck(name="x", type="int"),
+                             ValueCheck(name="y", type="int")])
+        self.expect_expr("b3", result_type="char", result_value="'a'")
+        self.expect_expr("c3", result_type="char", result_value="'b'")
+        self.expect_expr("d3", result_type="short", result_value="50")
+        self.expect_expr("e3", result_type="int", result_value="60")
+        self.expect_expr("f3", result_type="char", result_value="'c'")
+
+        self.expect_expr("carr_ref1", result_type="char", result_value="'a'")
+        self.expect_expr("carr_ref2", result_type="char", result_value="'b'")
+        self.expect_expr("carr_ref3", result_type="char", result_value="'c'")
+
+        self.expect_expr("sarr_ref1", result_type="short", result_value="11")
+        self.expect_expr("sarr_ref2", result_type="short", result_value="12")
+        self.expect_expr("sarr_ref3", result_type="short", result_value="13")
+
+        self.expect_expr("iarr_ref1", result_type="int", result_value="22")
+        self.expect_expr("iarr_ref2", result_type="int", result_value="33")
+        self.expect_expr("iarr_ref3", result_type="int", result_value="44")
+
+        self.expect_expr("carr_rref1", result_type="char", result_value="'a'")
+        self.expect_expr("carr_rref2", result_type="char", result_value="'b'")
+        self.expect_expr("carr_rref3", result_type="char", result_value="'c'")
+
+        self.expect_expr("sarr_rref1", result_type="short", result_value="11")
+        self.expect_expr("sarr_rref2", result_type="short", result_value="12")
+        self.expect_expr("sarr_rref3", result_type="short", result_value="13")
+
+        self.expect_expr("iarr_rref1", result_type="int", result_value="22")
+        self.expect_expr("iarr_rref2", result_type="int", result_value="33")
+        self.expect_expr("iarr_rref3", result_type="int", result_value="44")
+
+        self.expect_expr("carr_copy1", result_type="char", result_value="'a'")
+        self.expect_expr("carr_copy2", result_type="char", result_value="'b'")
+        self.expect_expr("carr_copy3", result_type="char", result_value="'c'")
+
+        self.expect_expr("sarr_copy1", result_type="short", result_value="11")
+        self.expect_expr("sarr_copy2", result_type="short", result_value="12")
+        self.expect_expr("sarr_copy3", result_type="short", result_value="13")
+
+        self.expect_expr("iarr_copy1", result_type="int", result_value="22")
+        self.expect_expr("iarr_copy2", result_type="int", result_value="33")
+        self.expect_expr("iarr_copy3", result_type="int", result_value="44")
+
+        self.expect_expr("tx1", result_type="float", result_value="4")
+        self.expect_expr("ty1", result_type="char", result_value="'z'")
+        self.expect_expr("tz1", result_type="int", result_value="10")
+
+        self.expect_expr("tx2", result_type="float", result_value="4")
+        self.expect_expr("ty2", result_type="char", result_value="'z'")
+        self.expect_expr("tz2", result_type="int", result_value="10")

lldb/test/API/lang/cpp/structured-binding/main.cpp

+// Structured binding in C++ can bind identifiers to subobjects of an object.
+//
+// There are three cases we need to test:
+// 1) arrays
+// 2) tuples like objects
+// 3) non-static data members
+//
+// They can also bind by copy, reference or rvalue reference.
+
+#include <tuple>
+
+struct A {
+  int x;
+  int y;
+};
+
+// We want to cover a mix of types and also different sizes to make sure we
+// hande the offsets correctly.
+struct MixedTypesAndSizesStruct {
+  A a;
+  char b1;
+  char b2;
+  short b3;
+  int b4;
+  char b5;
+};
+
+int main() {
+  MixedTypesAndSizesStruct b{{20, 30}, 'a', 'b', 50, 60, 'c'};
+
+  auto [a1, b1, c1, d1, e1, f1] = b;
+  auto &[a2, b2, c2, d2, e2, f2] = b;
+  auto &&[a3, b3, c3, d3, e3, f3] =
+      MixedTypesAndSizesStruct{{20, 30}, 'a', 'b', 50, 60, 'c'};
+
+  // Array with different sized types
+  char carr[]{'a', 'b', 'c'};
+  short sarr[]{11, 12, 13};
+  int iarr[]{22, 33, 44};
+
+  auto [carr_copy1, carr_copy2, carr_copy3] = carr;
+  auto [sarr_copy1, sarr_copy2, sarr_copy3] = sarr;
+  auto [iarr_copy1, iarr_copy2, iarr_copy3] = iarr;
+
+  auto &[carr_ref1, carr_ref2, carr_ref3] = carr;
+  auto &[sarr_ref1, sarr_ref2, sarr_ref3] = sarr;
+  auto &[iarr_ref1, iarr_ref2, iarr_ref3] = iarr;
+
+  auto &&[carr_rref1, carr_rref2, carr_rref3] = carr;
+  auto &&[sarr_rref1, sarr_rref2, sarr_rref3] = sarr;
+  auto &&[iarr_rref1, iarr_rref2, iarr_rref3] = iarr;
+
+  float x{4.0};
+  char y{'z'};
+  int z{10};
+
+  std::tuple<float, char, int> tpl(x, y, z);
+  auto [tx1, ty1, tz1] = tpl;
+  auto &[tx2, ty2, tz2] = tpl;
+
+  return a1.x + b1 + c1 + d1 + e1 + f1 + a2.y + b2 + c2 + d2 + e2 + f2 + a3.x +
+         b3 + c3 + d3 + e3 + f3 + carr_copy1 + carr_copy2 + carr_copy3 +
+         sarr_copy1 + sarr_copy2 + sarr_copy3 + iarr_copy1 + iarr_copy2 +
+         iarr_copy3 + carr_ref1 + carr_ref2 + carr_ref3 + sarr_ref1 +
+         sarr_ref2 + sarr_ref3 + iarr_ref1 + iarr_ref2 + iarr_ref3 +
+         carr_rref1 + carr_rref2 + carr_rref3 + sarr_rref1 + sarr_rref2 +
+         sarr_rref3 + iarr_rref1 + iarr_rref2 + iarr_rref3 + tx1 + ty1 + tz1 +
+         tx2 + ty2 + tz2; // break here
+}