[llvm] r187656 - Bugfix for making the DWARF debug strings and labels to code emitted as secrel32 instead of long opcodes (only for coff). This makes them debuggable with GDB (with fix for 64bits msvc)

[Carlo Kok]

Op 9-8-2013 00:27, Carlo Kok schreef:
>
>> If I may throw in my 2c:
>>
>> Carlo, I am afraid it may be not as simple as excluding those 3
>> DW_FORM's.
>> Please check out the equivalent GCC source here:
>> https://github.com/mirrors/gcc/blob/master/gcc/dwarf2out.c and search it
>> for calls to dw2_asm_output_offset (which eventually expands to
>> .secrel32).
>> You'll see that the decision of whether to use secrel32 depends not only
>> on DW_FORM, but also possibly on section into which information is
>> emitted (the 'for_eh' flag, meaning "for exception handling" I guess).
>
> I can check, but I'm fairly sure that the EH code doesn't use the same

I can't get it to emit those on Windows.

>>
>> At the very least, I would suggest to make the test for whether or not
>> to use secrel32 (in DIELabel::EmitValue) to be "opt-in", not "opt-out".
>>    According to DRAWF4 spec here <http://dwarfstd.org/doc/DWARF4.pdf>,
>> page 142, only DW_FORM_ref_addr, DW_FORM_sec_offset, DW_FORM_strp and
>> DW_OP_call_ref are supposed to be cross- debug section references, so I
>> would check for those.  Unfortunately, following this path you may find
>> out that certain DIE's currently use the wrong form of encoding, so that
>> may need to be fixed as well.  For example, I've found this to be the
>> case for the compile unit DIE (please see my patch attached to LLVM bug
>> 16249 <http://llvm.org/bugs/show_bug.cgi?id=16249>).
>
> I'll check tomorrow for how well the reversal of this works.
>
>> Also, I think that "IsAddress" parameter you've added to
>> EmitLabelPlusOffset(), has the wrong default - it should be "true" to
>> preserve the original behavior in case it's used to emit some custom
>> section other than "debug-info".  In fact, I would suggest to rename it
>> to "IsSectionRelative = false", for clarity.
>
> Sounds good.
>

Check attached patched.

-
Carlo Kok

-------------- next part --------------
Index: include/llvm/CodeGen/AsmPrinter.h
===================================================================
--- include/llvm/CodeGen/AsmPrinter.h	(revision 187993)
+++ include/llvm/CodeGen/AsmPrinter.h	(working copy)
@@ -359,13 +359,15 @@
     /// where the size in bytes of the directive is specified by Size and Label
     /// specifies the label.  This implicitly uses .set if it is available.
     void EmitLabelPlusOffset(const MCSymbol *Label, uint64_t Offset,
-                                   unsigned Size) const;
+                                   unsigned Size, 
+                                   bool IsSectionRelative = false) const;
 
     /// EmitLabelReference - Emit something like ".long Label"
     /// where the size in bytes of the directive is specified by Size and Label
     /// specifies the label.
-    void EmitLabelReference(const MCSymbol *Label, unsigned Size) const {
-      EmitLabelPlusOffset(Label, 0, Size);
+    void EmitLabelReference(const MCSymbol *Label, unsigned Size, 
+        bool IsSectionRelative = false) const {
+      EmitLabelPlusOffset(Label, 0, Size, IsSectionRelative);
     }
 
     //===------------------------------------------------------------------===//
Index: lib/CodeGen/AsmPrinter/AsmPrinter.cpp
===================================================================
--- lib/CodeGen/AsmPrinter/AsmPrinter.cpp	(revision 187993)
+++ lib/CodeGen/AsmPrinter/AsmPrinter.cpp	(working copy)
@@ -1415,9 +1415,9 @@
 /// where the size in bytes of the directive is specified by Size and Label
 /// specifies the label.  This implicitly uses .set if it is available.
 void AsmPrinter::EmitLabelPlusOffset(const MCSymbol *Label, uint64_t Offset,
-                                      unsigned Size)
+                                      unsigned Size, bool IsSectionRelative)
   const {
-  if (MAI->needsDwarfSectionOffsetDirective() && Size == 4) { // secrel32 ONLY works for 32bits.
+  if (MAI->needsDwarfSectionOffsetDirective() && IsSectionRelative) { 
     OutStreamer.EmitCOFFSecRel32(Label);
     return;
   }
Index: lib/CodeGen/AsmPrinter/DIE.cpp
===================================================================
--- lib/CodeGen/AsmPrinter/DIE.cpp	(revision 187993)
+++ lib/CodeGen/AsmPrinter/DIE.cpp	(working copy)
@@ -293,7 +293,8 @@
 /// EmitValue - Emit label value.
 ///
 void DIELabel::EmitValue(AsmPrinter *AP, uint16_t Form) const {
-  AP->EmitLabelReference(Label, SizeOf(AP, Form));
+  AP->EmitLabelReference(Label, SizeOf(AP, Form), Form == dwarf::DW_FORM_strp 
+    || Form == dwarf::DW_FORM_sec_offset || Form == dwarf::DW_OP_call_ref);
 }
 
 /// SizeOf - Determine size of label value in bytes.
Index: lib/CodeGen/AsmPrinter/DwarfDebug.cpp
===================================================================
--- lib/CodeGen/AsmPrinter/DwarfDebug.cpp	(revision 187993)
+++ lib/CodeGen/AsmPrinter/DwarfDebug.cpp	(working copy)
@@ -734,7 +734,7 @@
   // is not okay to use line_table_start here.
   if (!useSplitDwarf()) {
     if (Asm->MAI->doesDwarfUseRelocationsAcrossSections())
-      NewCU->addLabel(Die, dwarf::DW_AT_stmt_list, dwarf::DW_FORM_data4,
+      NewCU->addLabel(Die, dwarf::DW_AT_stmt_list, dwarf::DW_FORM_sec_offset,
                       UseTheFirstCU ?
                       Asm->GetTempSymbol("section_line") : LineTableStartSym);
     else if (UseTheFirstCU)