[Debuginfo] (1/7) [DW_OP_implicit_pointer/second strategy] Support for DW_OP_LLVM_implicit_pointer

[alok]

Summary

This patch stems from D84112.

New dwarf operator DW_OP_LLVM_implicit_pointer is introduced (present only in LLVM IR)
This is needed because representation and specification (types of operands) of
it a bit different than actual dwarf expression DW_OP_implicit_pointer.
It will later be converted to DW_OP_implicit_pointer while creating actual DWARF info

[jmorse]

Hi @alok, would you be able to re-title the reviews in this stack to indicate what's in the patch itself? That'll make it easier to navigate.

llvm/include/llvm/BinaryFormat/Dwarf.h

127

Any particular reason for 0x100c instead of the sequentially next 0x1004?

llvm/test/DebugInfo/X86/LLVM_implicit_pointer.ll

3

Would you be able to add a file-comment along the lines of "round-trip test for implicit pointer metadata"? That'll make it easier for future readers to know what's being tested.

[alok]

Thanks for your reviewing this.

llvm/include/llvm/BinaryFormat/Dwarf.h

127

Thanks for pointing this out. It shall be corrected in next version of patch.

llvm/test/DebugInfo/X86/LLVM_implicit_pointer.ll

3

It shall be included in next version of patch.

[alok]

@jmorse Do you have any more comment on this ?

[jmorse]

No real additional comments on this or the 2nd patch -- for further progress though I think you need to stimulate some discussion in the llvm-dev "DW_OP_implicit_pointer design/implementation in general" thread, to convince David / Adrian that this patch series addresses their concerns from the earlier discussion.

[aprantl]

llvm/docs/LangRef.rst

5210

there's an article missing .. the variable?

5210

I'm also missing a Verifier check that enforces this constraint?

5211

instruction's

5212

I think we could expand this a bit.

• is it illegal in a DIGlobalVariableExpression?

Maybe include an example use?

[dblaikie]

llvm/lib/IR/DebugInfoMetadata.cpp

1043

Probably need to have test coverage for these invalid cases

(& I'm not quite clear on the reasoning for where this can appear (why only at the beginning? I would've naively guessed maybe only at the end? (top of the stack) but haven't thought it through in any great detail, for sure - making sure it generalizes over multiple lost indirection would be great (I guess given the DWARF format, it can't generalize over parts of an object lacking indirection, but maybe it'd be nice if the IR format supported it (eg: "struct x { int y; int *z; }" if 'z' no longer has pointable storage - can we still describe what it points to, while also describing the value of 'y'?))

[alok]

llvm/lib/IR/DebugInfoMetadata.cpp

1043

Thanks for your comment. I shall add the invalid test cases.

Transformation of these operators are
DW_OP_LLVM_explicit_pointer (Generated by optimization phases), which is later converted to DW_OP_LLVM_implicit_pointer and which is finally converted to DW_OP_implicit_pointer.
DW_OP_LLVM_explicit_pointer is not bound by condition of appearing only at start. these are later converted to implicit_pointer versions to conform to DWARF standard.
I shall look into the use-case explained by you.

[alok]

Updated to re-base and incorporate comment from @dblaikie .

[aprantl]

llvm/docs/LangRef.rst

5212

I think this could really benefit from an example. Also, it is odd to refer to a DILocalVariable in the documentation of a DIExpression operand, since DIExpressions can be pointed to by dbg.values (which may bind them to one or more DILocalVariables) but they can also appear in other places. I understand that the DWARF definition of DW_OP_implicit_pointer makes some of this strangeness necessary, but we should do at least our best in documenting how this is supposed to be used.

[alok]

llvm/docs/LangRef.rst

5212

Thanks for your comment. I shall work on this.

[alok]

Updated comments from @aprantl and @dblaikie (on other review).

[alok]

In D84113#2390725, @alok wrote:

Updated comments from @aprantl and @dblaikie (on other review).

Ping !

[dblaikie]

Seems generally good to me - but @aprantl and/or @JDevlieghere is probably the keeper of DWARF expression tracking stuff, so they ought to take a look/sign off.

Out of interest, can this be composed? eg:

struct x { int i; int *j; };
void f1(int p) {
  x y  = {5, &p};
  f(); // opaque function call - breaking here it'd be good to still be able to describe y.j as pointing to the value stored in 'p', even though that value shouldn't be in memory anywhere (shuold just be in a register)
  return *y.j;
}

[jmorse]

FTR, this LGTM

llvm/docs/LangRef.rst

5210

[alok]

Updated for comment from @jmorse

[aprantl]

This is needed because representation and specification (types of operands) of

it a bit different than actual dwarf expression DW_OP_implicit_pointer.

Can you add this to the documention to explain what/how this is different from the DWARF op?

Otherwise this LGTM

llvm/docs/LangRef.rst

- ``DW_OP_LLVM_entry_value, N`` can only appear at the beginning of a
   parameter.
 - ``DW_OP_breg`` (or ``DW_OP_bregx``) represents a content on the provided
   signed offset of the specified register. The opcode is only generated by the
   ``AsmPrinter`` pass to describe call site parameter value which requires an
   expression over two registers.
 - ``DW_OP_push_object_address`` pushes the address of the object which can then
   serve as a descriptor in subsequent calculation. This opcode can be used to
   calculate bounds of fortran allocatable array which has array descriptors.
+- ``DW_OP_LLVM_implicit_pointer, N`` can only appear at the
+  beginning of a ``DIExpression``, and it specifies the value of variable
+  represented by the ``DILocalVariable`` referred to by the instructions
+  value/address operand at offset N.
 
 DWARF specifies three kinds of simple location descriptions: Register, memory,
 and implicit location descriptions.  Note that a location description is
 defined over certain ranges of a program, i.e the location of a variable may
 change over the course of the program. Register and memory location
 descriptions describe the *concrete location* of a source variable (in the
 sense that a debugger might modify its value), whereas *implicit locations*
 describe merely the actual *value* of a source variable which might not exist

llvm/include/llvm/BinaryFormat/Dwarf.h

#include "llvm/BinaryFormat/Dwarf.def"
   DW_FORM_lo_user = 0x1f00, ///< Not specified by DWARF.
 };
 
 enum LocationAtom {
 #define HANDLE_DW_OP(ID, NAME, VERSION, VENDOR) DW_OP_##NAME = ID,
 #include "llvm/BinaryFormat/Dwarf.def"
   DW_OP_lo_user = 0xe0,
   DW_OP_hi_user = 0xff,
-  DW_OP_LLVM_fragment = 0x1000,    ///< Only used in LLVM metadata.
-  DW_OP_LLVM_convert = 0x1001,     ///< Only used in LLVM metadata.
-  DW_OP_LLVM_tag_offset = 0x1002,  ///< Only used in LLVM metadata.
-  DW_OP_LLVM_entry_value = 0x1003, ///< Only used in LLVM metadata.
+  DW_OP_LLVM_fragment = 0x1000,         ///< Only used in LLVM metadata.
+  DW_OP_LLVM_convert = 0x1001,          ///< Only used in LLVM metadata.
+  DW_OP_LLVM_tag_offset = 0x1002,       ///< Only used in LLVM metadata.
+  DW_OP_LLVM_entry_value = 0x1003,      ///< Only used in LLVM metadata.
+  DW_OP_LLVM_implicit_pointer = 0x1004, ///< Only used in LLVM metadata.
 };
 
 enum TypeKind : uint8_t {
 #define HANDLE_DW_ATE(ID, NAME, VERSION, VENDOR) DW_ATE_##NAME = ID,
 #include "llvm/BinaryFormat/Dwarf.def"
   DW_ATE_lo_user = 0x80,
   DW_ATE_hi_user = 0xff
 };

llvm/lib/BinaryFormat/Dwarf.cpp

#include "llvm/BinaryFormat/Dwarf.def"
   case DW_OP_LLVM_convert:
     return "DW_OP_LLVM_convert";
   case DW_OP_LLVM_fragment:
     return "DW_OP_LLVM_fragment";
   case DW_OP_LLVM_tag_offset:
     return "DW_OP_LLVM_tag_offset";
   case DW_OP_LLVM_entry_value:
     return "DW_OP_LLVM_entry_value";
+  case DW_OP_LLVM_implicit_pointer:
+    return "DW_OP_LLVM_implicit_pointer";
   }
 }
 
 unsigned llvm::dwarf::getOperationEncoding(StringRef OperationEncodingString) {
   return StringSwitch<unsigned>(OperationEncodingString)
 #define HANDLE_DW_OP(ID, NAME, VERSION, VENDOR)                                \
   .Case("DW_OP_" #NAME, DW_OP_##NAME)
 #include "llvm/BinaryFormat/Dwarf.def"
       .Case("DW_OP_LLVM_convert", DW_OP_LLVM_convert)
       .Case("DW_OP_LLVM_fragment", DW_OP_LLVM_fragment)
       .Case("DW_OP_LLVM_tag_offset", DW_OP_LLVM_tag_offset)
       .Case("DW_OP_LLVM_entry_value", DW_OP_LLVM_entry_value)
+      .Case("DW_OP_LLVM_implicit_pointer", DW_OP_LLVM_implicit_pointer)
       .Default(0);
 }
 
 unsigned llvm::dwarf::OperationVersion(dwarf::LocationAtom Op) {
   switch (Op) {
   default:
     return 0;
 #define HANDLE_DW_OP(ID, NAME, VERSION, VENDOR)                                \

llvm/lib/IR/AsmWriter.cpp

if (N->isValid()) {
     for (auto I = N->expr_op_begin(), E = N->expr_op_end(); I != E; ++I) {
       auto OpStr = dwarf::OperationEncodingString(I->getOp());
       assert(!OpStr.empty() && "Expected valid opcode");
 
       Out << FS << OpStr;
       if (I->getOp() == dwarf::DW_OP_LLVM_convert) {
         Out << FS << I->getArg(0);
         Out << FS << dwarf::AttributeEncodingString(I->getArg(1));
+      } else if (I->getOp() == dwarf::DW_OP_LLVM_implicit_pointer) {
+        Out << FS << I->getArg(0);
       } else {
         for (unsigned A = 0, AE = I->getNumArgs(); A != AE; ++A)
           Out << FS << I->getArg(A);
       }
     }
   } else {
     for (const auto &I : N->getElements())
       Out << FS << I;

llvm/lib/IR/DebugInfoMetadata.cpp

case dwarf::DW_OP_bregx:
     return 3;
   case dwarf::DW_OP_constu:
   case dwarf::DW_OP_consts:
   case dwarf::DW_OP_deref_size:
   case dwarf::DW_OP_plus_uconst:
   case dwarf::DW_OP_LLVM_tag_offset:
   case dwarf::DW_OP_LLVM_entry_value:
   case dwarf::DW_OP_regx:
+  case dwarf::DW_OP_LLVM_implicit_pointer:
     return 2;
   default:
     return 1;
   }
 }
 
 bool DIExpression::isValid() const {
   for (auto I = expr_op_begin(), E = expr_op_end(); I != E; ++I) {
case dwarf::DW_OP_LLVM_entry_value: {
       // An entry value operator must appear at the beginning and the number of
       // operations it cover can currently only be 1, because we support only
       // entry values of a simple register location. One reason for this is that
       // we currently can't calculate the size of the resulting DWARF block for
       // other expressions.
       return I->get() == expr_op_begin()->get() && I->getArg(0) == 1 &&
              getNumElements() == 2;
     }
+    case dwarf::DW_OP_LLVM_implicit_pointer: {
+      // A DW_OP_LLVM_implicit_pointer operator must appear at the beginning
+      // and it has one argument.
+      return (I == expr_op_begin()) && (getNumElements() == 2);
+    }
     case dwarf::DW_OP_LLVM_convert:
     case dwarf::DW_OP_LLVM_tag_offset:
     case dwarf::DW_OP_constu:
     case dwarf::DW_OP_plus_uconst:
     case dwarf::DW_OP_plus:
     case dwarf::DW_OP_minus:
     case dwarf::DW_OP_mul:
     case dwarf::DW_OP_div:

llvm/test/DebugInfo/X86/LLVM_implicit_pointer.ll

+; Round trip test for DW_OP_LLVM_implicit_pointer metadata 
+
+; RUN: llvm-as < %s | llvm-dis | FileCheck %s
+
+; CHECK: !DIExpression(DW_OP_LLVM_implicit_pointer, 0)
+
+;---------------------------
+;static const char *b = "opq";
+;volatile int v;
+;int main() {
+;  int var1 = 4;
+;  int *ptr1;
+;
+;  v++;
+;  ptr1 = &var1;
+;  v++;
+;
+;  return *ptr1 - 5;
+;}
+;---------------------------
+
+; ModuleID = 'LLVM_implicit_pointer.c'
+source_filename = "LLVM_implicit_pointer.c"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@v = common dso_local global i32 0, align 4, !dbg !0
+; Function Attrs: nofree norecurse nounwind uwtable
+define dso_local i32 @main() local_unnamed_addr !dbg !12 {
+entry:
+  call void @llvm.dbg.value(metadata i32 4, metadata !16, metadata !DIExpression()), !dbg !19
+  %0 = load volatile i32, i32* @v, align 4, !dbg !20, !tbaa !21
+  %inc = add nsw i32 %0, 1, !dbg !20
+  store volatile i32 %inc, i32* @v, align 4, !dbg !20, !tbaa !21
+
+  call void @llvm.dbg.value(metadata i32 4, metadata !17, metadata !DIExpression(DW_OP_LLVM_implicit_pointer, 0)), !dbg !19
+
+  %1 = load volatile i32, i32* @v, align 4, !dbg !25, !tbaa !21
+  %inc1 = add nsw i32 %1, 1, !dbg !25
+  store volatile i32 %inc1, i32* @v, align 4, !dbg !25, !tbaa !21
+  ret i32 -1, !dbg !26
+}
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!8, !9, !10}
+!llvm.ident = !{!11}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "v", scope: !2, file: !3, line: 2, type: !6, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 10.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, splitDebugInlining: false, nameTableKind: None)
+!3 = !DIFile(filename: "LLVM_implicit_pointer.c", directory: "/dir", checksumkind: CSK_MD5, checksum: "218aaa8dc9f04b056b56d944d06383dd")
+!4 = !{}
+!5 = !{!0}
+!6 = !DIDerivedType(tag: DW_TAG_volatile_type, baseType: !7)
+!7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!8 = !{i32 7, !"Dwarf Version", i32 5}
+!9 = !{i32 2, !"Debug Info Version", i32 3}
+!10 = !{i32 1, !"wchar_size", i32 4}
+!11 = !{!"clang version 10.0.0"}
+!12 = distinct !DISubprogram(name: "main", scope: !3, file: !3, line: 3, type: !13, scopeLine: 3, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !15)
+!13 = !DISubroutineType(types: !14)
+!14 = !{!7}
+!15 = !{!16, !17}
+!16 = !DILocalVariable(name: "var1", scope: !12, file: !3, line: 4, type: !7)
+!17 = !DILocalVariable(name: "ptr1", scope: !12, file: !3, line: 5, type: !18)
+!18 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !7, size: 64)
+!19 = !DILocation(line: 0, scope: !12)
+!20 = !DILocation(line: 7, column: 4, scope: !12)
+!21 = !{!22, !22, i64 0}
+!22 = !{!"int", !23, i64 0}
+!23 = !{!"omnipotent char", !24, i64 0}
+!24 = !{!"Simple C/C++ TBAA"}
+!25 = !DILocation(line: 9, column: 4, scope: !12)
+!26 = !DILocation(line: 11, column: 3, scope: !12)

llvm/test/DebugInfo/X86/invalid-noarg-LLVM_implicit_pointer.ll

+; RUN: not llvm-as < %s -disable-output 2>&1 | FileCheck %s
+
+!named = !{!0}
+; CHECK: assembly parsed, but does not verify as correct!
+; CHECK: invalid expression
+; CHECK: !DIExpression(4100)
+!0 = !DIExpression(DW_OP_LLVM_implicit_pointer)

llvm/test/DebugInfo/X86/invalid-non-start-LLVM_implicit_pointer.ll

+; RUN: not llvm-as < %s -disable-output 2>&1 | FileCheck %s
+
+!named = !{!0}
+; CHECK: assembly parsed, but does not verify as correct!
+; CHECK: invalid expression
+; CHECK: !DIExpression(6, 4100, 0)
+!0 = !DIExpression(DW_OP_deref, DW_OP_LLVM_implicit_pointer, 0)