[PATCH] D83958: [DebugInfo] Fix a misleading usage of DWARF forms with DIEExpr. NFC.

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin created this revision.
ikudrin added reviewers: dblaikie, echristo, aprantl, jhenderson, probinson.
ikudrin added projects: LLVM, debug-info.
Herald added a subscriber: hiraditya.

For now, `DIEExpr` is used only in two places:

1. in the debug info library unit test suite to emit a `DW_AT_str_offsets_base` attribute with the `DW_FORM_sec_offset` form, see `dwarfgen::DIE::addStrOffsetsBaseAttribute()`;
2. in `DwarfCompileUnit::addLocationAttribute()` to generate the location attribute for a TLS variable.

The later case used an incorrect DWARF form of `DW_FORM_udata`, which implies storing an uleb128 value, not a 4/8 byte constant. The generated result was as expected because `DIEExpr::SizeOf()` did not handle the used form, but returned the size of the code pointer by default.

The patch fixes the issue by using more appropriate DWARF forms for the problematic case and making `DIEExpr::SizeOf()` more straightforward.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83958

Files:
  llvm/lib/CodeGen/AsmPrinter/DIE.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp


Index: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
===================================================================
--- llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -260,7 +260,9 @@
                                      : dwarf::DW_OP_const8u);
             // 2) containing the (relocated) offset of the TLS variable
             //    within the module's TLS block.
-            addExpr(*Loc, dwarf::DW_FORM_udata,
+            addExpr(*Loc,
+                    PointerSize == 4 ? dwarf::DW_FORM_data4
+                                     : dwarf::DW_FORM_data8,
                     Asm->getObjFileLowering().getDebugThreadLocalSymbol(Sym));
           } else {
             addUInt(*Loc, dwarf::DW_FORM_data1, dwarf::DW_OP_GNU_const_index);
Index: llvm/lib/CodeGen/AsmPrinter/DIE.cpp
===================================================================
--- llvm/lib/CodeGen/AsmPrinter/DIE.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DIE.cpp
@@ -472,10 +472,17 @@
 /// SizeOf - Determine size of expression value in bytes.
 ///
 unsigned DIEExpr::SizeOf(const AsmPrinter *AP, dwarf::Form Form) const {
-  if (Form == dwarf::DW_FORM_data4) return 4;
-  if (Form == dwarf::DW_FORM_sec_offset) return 4;
-  if (Form == dwarf::DW_FORM_strp) return 4;
-  return AP->getPointerSize();
+  switch (Form) {
+  case dwarf::DW_FORM_data4:
+    return 4;
+  case dwarf::DW_FORM_data8:
+    return 8;
+  case dwarf::DW_FORM_sec_offset:
+    // FIXME: add support for DWARF64
+    return 4;
+  default:
+    llvm_unreachable("DIE Value form not supported yet");
+  }
 }
 
 LLVM_DUMP_METHOD


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D83958.278502.patch
Type: text/x-patch
Size: 1640 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200716/fea5cbf7/attachment.bin>