[Debug-Info] [llvm-dwarfdump] Don't try to dump location list for attributes that don't have the loclist class.

[Esme]

Summary

As what we discussed in D105469, the overflow error occurs when we try to dump location list for those attributes that do not have the loclist class, like DW_AT_count and DW_AT_byte_size
After re-reviewed the entire list, I sorted those attributes into two parts, one for dumping location list and one for dumping the location expression.

[Esme]

ping

[probinson]

Overall the patch looks pretty reasonable, needs some polishing up.

llvm/lib/DebugInfo/DWARF/DWARFDie.cpp

96

Should verify the right kind of FORM here, as in:

assert((FormValue.isFormClass(DWARFFormValue::FC_Block) ||
        FormValue.isFormClass(DWARFFormValue::FC_Exprloc)) &&
       "bad FORM for location expression");

769

Missing DW_AT_bit_offset here (between byte_size and bit_size). I know this isn't really directly what you were doing, but if we're reviewing all the exprloc/loclist attributes, might as well fix both lists.

784

Missing DW_AT_data_location between associated and byte_stride.

llvm/test/tools/llvm-dwarfdump/ELF/formclass3.s

10 ↗

(On Diff #357171)

I think the following single source line should be sufficient:
unsigned char arr[0x100000];
This is a global definition (in C++ anyway), you don't need any statements to be referencing it. This will shorten the test code noticeably. We do try to minimize test cases, it helps readers to understand the goal of the test without wading through a lot of extra fluff.

14 ↗

(On Diff #357171)

If you simplify the source to have no executable instructions, then I *think* you can remove the -triple which will allow this test to be run in all environments. (A number of bots do not build all targets, so it's preferable to have tests be as generic as possible.)

[Esme]

Thank you Paul!
Address comments.

llvm/test/tools/llvm-dwarfdump/ELF/formclass3.s

14 ↗

(On Diff #357171)

Some targets, like AIX, have not yet supported llvm-mc. So I think we should still keep the -triple?

[probinson]

llvm/test/tools/llvm-dwarfdump/ELF/formclass3.s

7 ↗

(On Diff #360750)

It's more common to pipe the object file instead of writing it to the file system. The test runs a little faster that way, and across many thousands of tests these little things add up.

# RUN: llvm-mc ... -o - | llvm-dwarfdump -debug-info - | FileCheck %s

14 ↗

(On Diff #357171)

If you keep -triple then you need a REQUIRES to restrict the test to bots that include the correct target. A number of bots do not build all targets.

[dblaikie]

llvm/test/tools/llvm-dwarfdump/ELF/formclass3.s

14 ↗

(On Diff #357171)

If you move the test to the llvm/test/tools/llvm-dwarfdump/X86 directory you shouldn't need the REQUIRES - the whole directory has that implemented generically.

[Esme]

Address comments.

[probinson]

LGTM

llvm/include/llvm/DebugInfo/DWARF/DWARFAttribute.h

     return isValid();
   }
 
+  /// Identify DWARF attributes that may contain a pointer to a location list.
+  static bool mayHaveLocationList(dwarf::Attribute Attr);
+
   /// Identifies DWARF attributes that may contain a reference to a
   /// DWARF expression.
-  static bool mayHaveLocationDescription(dwarf::Attribute Attr);
+  static bool mayHaveLocationExpr(dwarf::Attribute Attr);
 };
 
 } // end namespace llvm

llvm/lib/DWARFLinker/DWARFLinker.cpp

   // buffer using cloneExpression(), otherwise copy the data directly.
   SmallVector<uint8_t, 32> Buffer;
   ArrayRef<uint8_t> Bytes = *Val.getAsBlock();
-  if (DWARFAttribute::mayHaveLocationDescription(AttrSpec.Attr) &&
+  if (DWARFAttribute::mayHaveLocationExpr(AttrSpec.Attr) &&
       (Val.isFormClass(DWARFFormValue::FC_Block) ||
        Val.isFormClass(DWARFFormValue::FC_Exprloc))) {
     DWARFUnit &OrigUnit = Unit.getOrigUnit();

llvm/lib/DebugInfo/DWARF/DWARFDie.cpp

   }
 }
 
-static void dumpLocation(raw_ostream &OS, const DWARFFormValue &FormValue,
+static void dumpLocationList(raw_ostream &OS, const DWARFFormValue &FormValue,
                              DWARFUnit *U, unsigned Indent,
                              DIDumpOptions DumpOpts) {
+  assert(FormValue.isFormClass(DWARFFormValue::FC_SectionOffset) &&
+         "bad FORM for location list");
   DWARFContext &Ctx = U->getContext();
   const MCRegisterInfo *MRI = Ctx.getRegisterInfo();
-  if (FormValue.isFormClass(DWARFFormValue::FC_Block) ||
-      FormValue.isFormClass(DWARFFormValue::FC_Exprloc)) {
-    ArrayRef<uint8_t> Expr = *FormValue.getAsBlock();
-    DataExtractor Data(StringRef((const char *)Expr.data(), Expr.size()),
-                       Ctx.isLittleEndian(), 0);
-    DWARFExpression(Data, U->getAddressByteSize(), U->getFormParams().Format)
-        .print(OS, DumpOpts, MRI, U);
-    return;
-  }
+  uint64_t Offset = *FormValue.getAsSectionOffset();
 
-  if (FormValue.isFormClass(DWARFFormValue::FC_SectionOffset)) {
-    uint64_t Offset = *FormValue.getAsSectionOffset();
-
-    if (FormValue.getForm() == DW_FORM_loclistx) {
-      FormValue.dump(OS, DumpOpts);
+  if (FormValue.getForm() == DW_FORM_loclistx) {
+    FormValue.dump(OS, DumpOpts);
 
-      if (auto LoclistOffset = U->getLoclistOffset(Offset))
-        Offset = *LoclistOffset;
-      else
-        return;
-    }
-    U->getLocationTable().dumpLocationList(&Offset, OS, U->getBaseAddress(),
-                                           MRI, Ctx.getDWARFObj(), U, DumpOpts,
-                                           Indent);
-    return;
   }
-
-  FormValue.dump(OS, DumpOpts);
+  U->getLocationTable().dumpLocationList(&Offset, OS, U->getBaseAddress(), MRI,
+                                         Ctx.getDWARFObj(), U, DumpOpts,
+                                         Indent);
+  return;
+}
+
+static void dumpLocationExpr(raw_ostream &OS, const DWARFFormValue &FormValue,
+                             DWARFUnit *U, unsigned Indent,
+                             DIDumpOptions DumpOpts) {
+  assert((FormValue.isFormClass(DWARFFormValue::FC_Block) ||
+          FormValue.isFormClass(DWARFFormValue::FC_Exprloc)) &&
+         "bad FORM for location expression");
+  DWARFContext &Ctx = U->getContext();
+  const MCRegisterInfo *MRI = Ctx.getRegisterInfo();
+  ArrayRef<uint8_t> Expr = *FormValue.getAsBlock();
+  DataExtractor Data(StringRef((const char *)Expr.data(), Expr.size()),
+                     Ctx.isLittleEndian(), 0);
+  DWARFExpression(Data, U->getAddressByteSize(), U->getFormParams().Format)
+      .print(OS, DumpOpts, MRI, U);
+  return;
 }
 
 /// Dump the name encoded in the type tag.

       else
         FormValue.dump(OS, DumpOpts);
     }
-  } else if (Form == dwarf::Form::DW_FORM_exprloc ||
-             DWARFAttribute::mayHaveLocationDescription(Attr))
-    dumpLocation(OS, FormValue, U, sizeof(BaseIndent) + Indent + 4, DumpOpts);
+  } else if (DWARFAttribute::mayHaveLocationList(Attr) &&
+             FormValue.isFormClass(DWARFFormValue::FC_SectionOffset))
+    dumpLocationList(OS, FormValue, U, sizeof(BaseIndent) + Indent + 4,
+                     DumpOpts);
+  else if (FormValue.isFormClass(DWARFFormValue::FC_Exprloc) ||
+           (DWARFAttribute::mayHaveLocationExpr(Attr) &&
+            FormValue.isFormClass(DWARFFormValue::FC_Block)))
+    dumpLocationExpr(OS, FormValue, U, sizeof(BaseIndent) + Indent + 4,
+                     DumpOpts);
   else
     FormValue.dump(OS, DumpOpts);
 

   return *this;
 }
 
-bool DWARFAttribute::mayHaveLocationDescription(dwarf::Attribute Attr) {
+bool DWARFAttribute::mayHaveLocationList(dwarf::Attribute Attr) {
+  switch(Attr) {
+  case DW_AT_location:
+  case DW_AT_string_length:
+  case DW_AT_return_addr:
+  case DW_AT_data_member_location:
+  case DW_AT_frame_base:
+  case DW_AT_static_link:
+  case DW_AT_segment:
+  case DW_AT_use_location:
+  case DW_AT_vtable_elem_location:
+    return true;
+  default:
+    return false;
+  }
+}
+
+bool DWARFAttribute::mayHaveLocationExpr(dwarf::Attribute Attr) {
   switch (Attr) {
   // From the DWARF v5 specification.
   case DW_AT_location:
   case DW_AT_byte_size:
+  case DW_AT_bit_offset:
   case DW_AT_bit_size:
   case DW_AT_string_length:
   case DW_AT_lower_bound:

   case DW_AT_vtable_elem_location:
   case DW_AT_allocated:
   case DW_AT_associated:
+  case DW_AT_data_location:
   case DW_AT_byte_stride:
   case DW_AT_rank:
   case DW_AT_call_value:

llvm/test/tools/llvm-dwarfdump/X86/formclass3.s

+# Source:
+#   unsigned char arr[0x100000];
+# Compile with:
+#   clang -O2 -gdwarf-3 -S 1.cpp -o 1.s --target=x86_64-pc-linux-gnu
+
+# RUN: llvm-mc -triple x86_64-pc-linux-gnu -o - -filetype obj < %s | \
+# RUN:   llvm-dwarfdump -debug-info - | FileCheck %s
+
+# CHECK: DW_AT_count (0x00100000)
+
+  .text
+  .file "1.cpp"
+  .file 1 "/llvm-project" "1.cpp"
+  .type arr,@object                     # @arr
+  .bss
+  .globl  arr
+  .p2align  4
+arr:
+  .zero 1048576
+  .size arr, 1048576
+
+  .section  .debug_abbrev,"",@progbits
+  .byte 1                               # Abbreviation Code
+  .byte 17                              # DW_TAG_compile_unit
+  .byte 1                               # DW_CHILDREN_yes
+  .byte 37                              # DW_AT_producer
+  .byte 14                              # DW_FORM_strp
+  .byte 19                              # DW_AT_language
+  .byte 5                               # DW_FORM_data2
+  .byte 3                               # DW_AT_name
+  .byte 14                              # DW_FORM_strp
+  .byte 16                              # DW_AT_stmt_list
+  .byte 6                               # DW_FORM_data4
+  .byte 27                              # DW_AT_comp_dir
+  .byte 14                              # DW_FORM_strp
+  .byte 0                               # EOM(1)
+  .byte 0                               # EOM(2)
+  .byte 2                               # Abbreviation Code
+  .byte 52                              # DW_TAG_variable
+  .byte 0                               # DW_CHILDREN_no
+  .byte 3                               # DW_AT_name
+  .byte 14                              # DW_FORM_strp
+  .byte 73                              # DW_AT_type
+  .byte 19                              # DW_FORM_ref4
+  .byte 63                              # DW_AT_external
+  .byte 12                              # DW_FORM_flag
+  .byte 58                              # DW_AT_decl_file
+  .byte 11                              # DW_FORM_data1
+  .byte 59                              # DW_AT_decl_line
+  .byte 11                              # DW_FORM_data1
+  .byte 2                               # DW_AT_location
+  .byte 10                              # DW_FORM_block1
+  .byte 0                               # EOM(1)
+  .byte 0                               # EOM(2)
+  .byte 3                               # Abbreviation Code
+  .byte 1                               # DW_TAG_array_type
+  .byte 1                               # DW_CHILDREN_yes
+  .byte 73                              # DW_AT_type
+  .byte 19                              # DW_FORM_ref4
+  .byte 0                               # EOM(1)
+  .byte 0                               # EOM(2)
+  .byte 4                               # Abbreviation Code
+  .byte 33                              # DW_TAG_subrange_type
+  .byte 0                               # DW_CHILDREN_no
+  .byte 73                              # DW_AT_type
+  .byte 19                              # DW_FORM_ref4
+  .byte 55                              # DW_AT_count
+  .byte 6                               # DW_FORM_data4
+  .byte 0                               # EOM(1)
+  .byte 0                               # EOM(2)
+  .byte 5                               # Abbreviation Code
+  .byte 36                              # DW_TAG_base_type
+  .byte 0                               # DW_CHILDREN_no
+  .byte 3                               # DW_AT_name
+  .byte 14                              # DW_FORM_strp
+  .byte 62                              # DW_AT_encoding
+  .byte 11                              # DW_FORM_data1
+  .byte 11                              # DW_AT_byte_size
+  .byte 11                              # DW_FORM_data1
+  .byte 0                               # EOM(1)
+  .byte 0                               # EOM(2)
+  .byte 6                               # Abbreviation Code
+  .byte 36                              # DW_TAG_base_type
+  .byte 0                               # DW_CHILDREN_no
+  .byte 3                               # DW_AT_name
+  .byte 14                              # DW_FORM_strp
+  .byte 11                              # DW_AT_byte_size
+  .byte 11                              # DW_FORM_data1
+  .byte 62                              # DW_AT_encoding
+  .byte 11                              # DW_FORM_data1
+  .byte 0                               # EOM(1)
+  .byte 0                               # EOM(2)
+  .byte 0                               # EOM(3)
+  .section  .debug_info,"",@progbits
+.Lcu_begin0:
+  .long .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit
+.Ldebug_info_start0:
+  .short  3                               # DWARF version number
+  .long .debug_abbrev                   # Offset Into Abbrev. Section
+  .byte 8                               # Address Size (in bytes)
+  .byte 1                               # Abbrev [1] 0xb:0x47 DW_TAG_compile_unit
+  .long .Linfo_string0                  # DW_AT_producer
+  .short  33                              # DW_AT_language
+  .long .Linfo_string1                  # DW_AT_name
+  .long .Lline_table_start0             # DW_AT_stmt_list
+  .long .Linfo_string2                  # DW_AT_comp_dir
+  .byte 2                               # Abbrev [2] 0x1e:0x16 DW_TAG_variable
+  .long .Linfo_string3                  # DW_AT_name
+  .long 52                              # DW_AT_type
+  .byte 1                               # DW_AT_external
+  .byte 1                               # DW_AT_decl_file
+  .byte 1                               # DW_AT_decl_line
+  .byte 9                               # DW_AT_location
+  .byte 3
+  .quad arr
+  .byte 3                               # Abbrev [3] 0x34:0xf DW_TAG_array_type
+  .long 67                              # DW_AT_type
+  .byte 4                               # Abbrev [4] 0x39:0x9 DW_TAG_subrange_type
+  .long 74                              # DW_AT_type
+  .long 1048576                         # DW_AT_count
+  .byte 0                               # End Of Children Mark
+  .byte 5                               # Abbrev [5] 0x43:0x7 DW_TAG_base_type
+  .long .Linfo_string4                  # DW_AT_name
+  .byte 8                               # DW_AT_encoding
+  .byte 1                               # DW_AT_byte_size
+  .byte 6                               # Abbrev [6] 0x4a:0x7 DW_TAG_base_type
+  .long .Linfo_string5                  # DW_AT_name
+  .byte 8                               # DW_AT_byte_size
+  .byte 7                               # DW_AT_encoding
+  .byte 0                               # End Of Children Mark
+.Ldebug_info_end0:
+  .section  .debug_str,"MS",@progbits,1
+.Linfo_string0:
+  .asciz  "clang version 13.0.0"          # string offset=0
+.Linfo_string1:
+  .asciz  "1.cpp"                         # string offset=21
+.Linfo_string2:
+  .asciz  "/llvm-project" # string offset=27
+.Linfo_string3:
+  .asciz  "arr"                           # string offset=61
+.Linfo_string4:
+  .asciz  "unsigned char"                 # string offset=65
+.Linfo_string5:
+  .asciz  "__ARRAY_SIZE_TYPE__"           # string offset=79
+  .ident  "clang version 13.0.0"
+  .section  ".note.GNU-stack","",@progbits
+  .addrsig
+  .section  .debug_line,"",@progbits
+.Lline_table_start0: