[lldb] Fix DWARF-5 DW_FORM_implicit_const (used by GCC)

[jankratochvil]

lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.cpp

30

This const_cast is not nice. For its removal one can do either of:

• Just pass the DWARFUnit *cu additional parameter like there was before (but it is a duplication of data)
• de-const DWARFUnit * in a lot of code around: https://people.redhat.com/jkratoch/lldb-deconst.patch

[jankratochvil]

Update after rGfe7333174f54.

[labath]

Could you simplify the test case? It sounds like all you need is a single global variable (int foo = 47;), without any functions or that stuff. I guess we should also delete test/Shell/Breakpoint/implicit_const_form_support.test, as it's not very useful (the numbers it checks come from the line table, not from the implicit constants). Other than that, this seems fine.

lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.cpp

30

this is not the first const_cast of DWARFUnits, so, while definitely not ideal, I'm not particularly worried about it.

lldb/test/Shell/SymbolFile/DWARF/dwarf5-implicit-const.s

4

I hope these are not necessary, since you don't run the code here.

[jankratochvil]

In D98197#2613149, @labath wrote:

Could you simplify the test case?

done

I guess we should also delete test/Shell/Breakpoint/implicit_const_form_support.test, as it's not very useful (the numbers it checks come from the line table, not from the implicit constants).

I have not deleted it myself; I find non-racy simple tests to be always good as they may catch some completely unrelated regression in the future.

lldb/test/Shell/SymbolFile/DWARF/dwarf5-implicit-const.s

4

OK, removed:

-# UNSUPPORTED: system-darwin, system-windows
-# REQUIRES: x86

We will see after all the bots run again.

[labath]

In D98197#2613227, @jankratochvil wrote:

I guess we should also delete test/Shell/Breakpoint/implicit_const_form_support.test, as it's not very useful (the numbers it checks come from the line table, not from the implicit constants).

I have not deleted it myself; I find non-racy simple tests to be always good as they may catch some completely unrelated regression in the future.

Fine, I'll do it afterwards. :) If it was at least testing what it claims it's testing, I might keep it. However, judging by the need to re-fix implicit const support, it's not doing a good job at even that.

[labath]

lldb/test/Shell/SymbolFile/DWARF/dwarf5-implicit-const.s

4

Sorry, I only meant the system-*** thingies. The REQUIRES: x86 thingy was correct, as it will cause the test to be skipped if someone completely disables x86 support (via LLVM_TARGETS_TO_BUILD).

That said, no bot actually builds lldb in that way, so I'm sure we have a lot of tests which are missing this clause.

lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp

while (data.ValidOffset(*offset_ptr)) {
 
     if (!attr || !form)
       return llvm::make_error<llvm::object::GenericBinaryError>(
           "malformed abbreviation declaration attribute");
 
     DWARFFormValue::ValueType val;
 
     if (form == DW_FORM_implicit_const)
-      val.value.sval = data.GetULEB128(offset_ptr);
+      val.value.sval = data.GetSLEB128(offset_ptr);
 
     m_attributes.push_back(DWARFAttribute(attr, form, val));
   }
 
   return llvm::make_error<llvm::object::GenericBinaryError>(
       "abbreviation declaration attribute list not terminated with a null "
       "entry");
 }

lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h

 class DWARFAttribute {
 public:
   DWARFAttribute(dw_attr_t attr, dw_form_t form,
                  DWARFFormValue::ValueType value)
       : m_attr(attr), m_form(form), m_value(value) {}
 
   dw_attr_t get_attr() const { return m_attr; }
   dw_form_t get_form() const { return m_form; }
+  DWARFFormValue::ValueType get_value() const { return m_value; }
   void get(dw_attr_t &attr, dw_form_t &form,
            DWARFFormValue::ValueType &val) const {
     attr = m_attr;
     form = m_form;
     val = m_value;
   }
   typedef std::vector<DWARFAttribute> collection;
   typedef collection::iterator iterator;
   typedef collection::const_iterator const_iterator;
 
 protected:
   dw_attr_t m_attr;
   dw_form_t m_form;
   DWARFFormValue::ValueType m_value;
 };
 
 class DWARFAttributes {
 public:
   DWARFAttributes();
   ~DWARFAttributes();
 
-  void Append(DWARFUnit *cu, dw_offset_t attr_die_offset, dw_attr_t attr,
-              dw_form_t form);
+  void Append(const DWARFFormValue &form_value, dw_offset_t attr_die_offset,
+              dw_attr_t attr);
   DWARFUnit *CompileUnitAtIndex(uint32_t i) const { return m_infos[i].cu; }
   dw_offset_t DIEOffsetAtIndex(uint32_t i) const {
     return m_infos[i].die_offset;
   }
   dw_attr_t AttributeAtIndex(uint32_t i) const {
     return m_infos[i].attr.get_attr();
   }
   dw_attr_t FormAtIndex(uint32_t i) const { return m_infos[i].attr.get_form(); }
+  DWARFFormValue::ValueType ValueAtIndex(uint32_t i) const {
+    return m_infos[i].attr.get_value();
+  }
   bool ExtractFormValueAtIndex(uint32_t i, DWARFFormValue &form_value) const;
   DWARFDIE FormValueAsReferenceAtIndex(uint32_t i) const;
   DWARFDIE FormValueAsReference(dw_attr_t attr) const;
   uint32_t FindAttributeIndex(dw_attr_t attr) const;
   void Clear() { m_infos.clear(); }
   size_t Size() const { return m_infos.size(); }
 
 protected:

lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.cpp

uint32_t DWARFAttributes::FindAttributeIndex(dw_attr_t attr) const {
   collection::const_iterator pos;
   for (pos = beg; pos != end; ++pos) {
     if (pos->attr.get_attr() == attr)
       return std::distance(beg, pos);
   }
   return UINT32_MAX;
 }
 
-void DWARFAttributes::Append(DWARFUnit *cu, dw_offset_t attr_die_offset,
-                             dw_attr_t attr, dw_form_t form) {
-  AttributeValue attr_value = {
-      cu, attr_die_offset, {attr, form, DWARFFormValue::ValueType()}};
+void DWARFAttributes::Append(const DWARFFormValue &form_value,
+                             dw_offset_t attr_die_offset, dw_attr_t attr) {
+  AttributeValue attr_value = {const_cast<DWARFUnit *>(form_value.GetUnit()),
+                               attr_die_offset,
+                               {attr, form_value.Form(), form_value.Value()}};
   m_infos.push_back(attr_value);
 }
 
 bool DWARFAttributes::ExtractFormValueAtIndex(
     uint32_t i, DWARFFormValue &form_value) const {
   const DWARFUnit *cu = CompileUnitAtIndex(i);
   form_value.SetUnit(cu);
   form_value.SetForm(FormAtIndex(i));
+  if (form_value.Form() == DW_FORM_implicit_const) {
+    form_value.SetValue(ValueAtIndex(i));
+    return true;
+  }
   lldb::offset_t offset = DIEOffsetAtIndex(i);
   return form_value.ExtractValue(cu->GetData(), &offset);
 }
 
 DWARFDIE
 DWARFAttributes::FormValueAsReference(dw_attr_t attr) const {
   const uint32_t attr_idx = FindAttributeIndex(attr);
   if (attr_idx != UINT32_MAX)

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp

for (uint32_t i = 0; i < num_attributes; ++i) {
         if (curr_depth > 0) {
           // This attribute doesn't make sense when combined with the DIE that
           // references this DIE. We know a DIE is referencing this DIE because
           // curr_depth is not zero
           break;
         }
         LLVM_FALLTHROUGH;
       default:
-        attributes.Append(cu, offset, attr, form);
+        attributes.Append(form_value, offset, attr);
         break;
       }
 
       if (recurse == Recurse::yes &&
           ((attr == DW_AT_specification) || (attr == DW_AT_abstract_origin))) {
         if (form_value.ExtractValue(data, &offset)) {
           DWARFDIE spec_die = form_value.Reference();
           if (spec_die)

lldb/test/Shell/SymbolFile/DWARF/dwarf5-implicit-const.s

+# Test handling of DWARF5 DW_FORM_implicit_const as used by GCC.
+
+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-pc-linux %s
+# RUN: %lldb %t -o "expression -T -- variable_implicit_const" \
+# RUN:   -o exit | FileCheck %s
+
+# Failing case was:
+# error: need to add support for DW_TAG_base_type 'int' encoded with DW_ATE = 0x5, bit_size = 0
+# CHECK: (int) $0 = 0
+
+  .bss
+  .globl  variable_implicit_const
+  .type variable_implicit_const, @object
+  .size variable_implicit_const, 4
+variable_implicit_const:
+  .long 0
+.Lvariable_implicit_const_end:
+  .section  .debug_info,"",@progbits
+.Ldebug_info0:
+  .long .Ldebug_info0_end - .Ldebug_info0_start # Length of Compilation Unit Info
+.Ldebug_info0_start:
+  .value  0x5 # DWARF version number
+  .byte 0x1 # DW_UT_compile
+  .byte 0x8 # Pointer Size (in bytes)
+  .long .Ldebug_abbrev0 # Offset Into Abbrev. Section
+  .uleb128 0x1  # (DIE DW_TAG_compile_unit)
+      # DW_AT_producer
+  .string "GNU C17 11.0.0 20210210 (Red Hat 11.0.0-0) -mtune=generic -march=x86-64 -g"
+  .byte 0x1d  # DW_AT_language
+      # DW_AT_name
+  .string "var4.c"
+  .uleb128 0x2  # (DIE DW_TAG_variable)
+      # DW_AT_name
+  .string "variable_implicit_const"
+  .long .Ltype_int - .Ldebug_info0  # DW_AT_type
+      # DW_AT_external
+  .uleb128 0x9  # DW_AT_location
+  .byte 0x3 # DW_OP_addr
+  .quad variable_implicit_const
+.Ltype_int:
+  .uleb128 0x3  # (DIE DW_TAG_base_type)
+      # DW_AT_byte_size
+  .byte 0x5 # DW_AT_encoding
+  .ascii "int\0"  # DW_AT_name
+  .byte 0 # end of children of DIE DW_TAG_compile_unit
+.Ldebug_info0_end:
+  .section  .debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+  .uleb128 0x1  # (abbrev code)
+  .uleb128 0x11 # (TAG: DW_TAG_compile_unit)
+  .byte 0x1 # DW_children_yes
+  .uleb128 0x25 # (DW_AT_producer)
+  .uleb128 0x8  # (DW_FORM_string)
+  .uleb128 0x13 # (DW_AT_language)
+  .uleb128 0xb  # (DW_FORM_data1)
+  .uleb128 0x3  # (DW_AT_name)
+  .uleb128 0x8  # (DW_FORM_string)
+  .byte 0
+  .byte 0
+  .uleb128 0x2  # (abbrev code)
+  .uleb128 0x34 # (TAG: DW_TAG_variable)
+  .byte 0 # DW_children_no
+  .uleb128 0x3  # (DW_AT_name)
+  .uleb128 0x8  # (DW_FORM_string)
+  .uleb128 0x49 # (DW_AT_type)
+  .uleb128 0x13 # (DW_FORM_ref4)
+  .uleb128 0x3f # (DW_AT_external)
+  .uleb128 0x19 # (DW_FORM_flag_present)
+  .uleb128 0x2  # (DW_AT_location)
+  .uleb128 0x18 # (DW_FORM_exprloc)
+  .byte 0
+  .byte 0
+  .uleb128 0x3  # (abbrev code)
+  .uleb128 0x24 # (TAG: DW_TAG_base_type)
+  .byte 0 # DW_children_no
+  .uleb128 0xb  # (DW_AT_byte_size)
+  .uleb128 0x21 # (DW_FORM_implicit_const)
+  .sleb128 .Lvariable_implicit_const_end - variable_implicit_const
+  .uleb128 0x3e # (DW_AT_encoding)
+  .uleb128 0xb  # (DW_FORM_data1)
+  .uleb128 0x3  # (DW_AT_name)
+  .uleb128 0x8  # (DW_FORM_string)
+  .byte 0
+  .byte 0
+  .byte 0