[DebugInfo] Fix misleading using of DWARF forms with DIELabel. NFCI.

[ikudrin]

Summary

DIELabel can emit only 32- or 64-bit values, while it was created in some places with DW_FORM_udata, which implies emitting uleb128. Nevertheless, these places also expected to emit U32 or U64, but just used a misleading DWARF form. The patch updates those places to use more appropriate DWARF forms and restricts DIELabel::SizeOf() to accept only forms that are actually used in the LLVM codebase.

See also: D83958.

[aardappel]

Ok, so we had incorrectly specified DW_FORM_udata for the label, but AddLabel had previously caused it to be emitted as an int32 anyway? So this doesn't change anything about the DWARF output?

@dschuff

[probinson]

It looks like you have been tweaking a number of related SizeOf() methods. Is it practical to have just one? In a parent class, or even as a free function (as the size computation depends only on form/format, and is not related to the class at all).

[ikudrin]

In D84094#2162466, @probinson wrote:

It looks like you have been tweaking a number of related SizeOf() methods. Is it practical to have just one? In a parent class, or even as a free function (as the size computation depends only on form/format, and is not related to the class at all).

The idea is that SizeOf() should be in sync with emitValue(). If a value class accepts a DWARF form, i.e. SizeOf() does not break on llvm_unreachable(), it should emit the value in the specified way, corresponding to that form.

[ikudrin]

In D84094#2162298, @aardappel wrote:

Ok, so we had incorrectly specified DW_FORM_udata for the label, but AddLabel had previously caused it to be emitted as an int32 anyway? So this doesn't change anything about the DWARF output?

Exactly.

llvm/lib/CodeGen/AsmPrinter/DIE.cpp

 
 //===----------------------------------------------------------------------===//
 // DIELabel Implementation
 //===----------------------------------------------------------------------===//
 
 /// EmitValue - Emit label value.
 ///
 void DIELabel::emitValue(const AsmPrinter *AP, dwarf::Form Form) const {
-  AP->emitLabelReference(
-      Label, SizeOf(AP, Form),
-      Form == dwarf::DW_FORM_strp || Form == dwarf::DW_FORM_sec_offset ||
-          Form == dwarf::DW_FORM_ref_addr || Form == dwarf::DW_FORM_data4);
+  bool IsSectionRelative = Form != dwarf::DW_FORM_addr;
+  AP->emitLabelReference(Label, SizeOf(AP, Form), IsSectionRelative);
 }
 
 /// SizeOf - Determine size of label value in bytes.
 ///
 unsigned DIELabel::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;
+  switch (Form) {
+  case dwarf::DW_FORM_data4:
+    return 4;
+  case dwarf::DW_FORM_sec_offset:
+  case dwarf::DW_FORM_strp:
+    // FIXME: add support for DWARF64
+    return 4;
+  case dwarf::DW_FORM_addr:
     return AP->MAI->getCodePointerSize();
+  default:
+    llvm_unreachable("DIE Value form not supported yet");
+  }
 }
 
 LLVM_DUMP_METHOD
 void DIELabel::print(raw_ostream &O) const { O << "Lbl: " << Label->getName(); }
 
 //===----------------------------------------------------------------------===//
 // DIEBaseTypeRef Implementation
 //===----------------------------------------------------------------------===//

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

case TargetFrameLowering::DwarfFrameBase::WasmFrameBase: {
         SPSym->setGlobalType(wasm::WasmGlobalType{
             uint8_t(Asm->getSubtargetInfo().getTargetTriple().getArch() ==
                             Triple::wasm64
                         ? wasm::WASM_TYPE_I64
                         : wasm::WASM_TYPE_I32),
             true});
         DIELoc *Loc = new (DIEValueAllocator) DIELoc;
         addUInt(*Loc, dwarf::DW_FORM_data1, dwarf::DW_OP_WASM_location);
-        addSInt(*Loc, dwarf::DW_FORM_sdata, FrameBase.Location.WasmLoc.Kind);
-        addLabel(*Loc, dwarf::DW_FORM_udata, SPSym);
+        addSInt(*Loc, dwarf::DW_FORM_sdata, TI_GLOBAL_RELOC);
+        addLabel(*Loc, dwarf::DW_FORM_data4, SPSym);
         DD->addArangeLabel(SymbolCU(this, SPSym));
         addUInt(*Loc, dwarf::DW_FORM_data1, dwarf::DW_OP_stack_value);
         addBlock(*SPDie, dwarf::DW_AT_frame_base, Loc);
       } else {
         DIELoc *Loc = new (DIEValueAllocator) DIELoc;
         DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
         DIExpressionCursor Cursor({});
         DwarfExpr.addWasmLocation(FrameBase.Location.WasmLoc.Kind,

llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp

void DwarfUnit::addOpAddress(DIELoc &Die, const MCSymbol *Sym) {
   if (DD->useSplitDwarf()) {
     addUInt(Die, dwarf::DW_FORM_data1, dwarf::DW_OP_GNU_addr_index);
     addUInt(Die, dwarf::DW_FORM_GNU_addr_index,
             DD->getAddressPool().getIndex(Sym));
     return;
   }
 
   addUInt(Die, dwarf::DW_FORM_data1, dwarf::DW_OP_addr);
-  addLabel(Die, dwarf::DW_FORM_udata, Sym);
+  addLabel(Die, dwarf::DW_FORM_addr, Sym);
 }
 
 void DwarfUnit::addLabelDelta(DIE &Die, dwarf::Attribute Attribute,
                               const MCSymbol *Hi, const MCSymbol *Lo) {
   Die.addValue(DIEValueAllocator, Attribute, dwarf::DW_FORM_data4,
                new (DIEValueAllocator) DIEDelta(Hi, Lo));
 }