[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:11, Vadim schreef:
> Carlo Kok <ck at ...> writes:
>
> 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).
> Now, it appears to me that LLVM relies on the assembler to emit stack
> unwinding tables for exception handling, because I don't see that stuff
> in .s files), so this issue may be moot...  On the other hand, what
> happens when LLVM emits binary code directly?  Does code still pass
> through AsmPrinter?   Does LLVM use DIE* classes when generating unwind
> tables?..
>
> 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>).
> 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.
>

Attached is the latest patch. Essentially it was what Vadin spoke about. 
When it's one of those 4 it emits it as a COFF secrel32 (only on coff), 
else it emits it as a regular label (.long/.quad depending on the 
address size)

This improves on my earlier patch and now does line info properly too 
(both 32 and 64bits):


Temporary breakpoint 1, main () at test2.c:8
8               foobar(17);
(gdb) step
foobar (x=17) at test2.c:3
3           return x+42;
(gdb) step
main () at test2.c:9
9               return 0;
-------------- 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,9 @@
 /// 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
+    || Form == dwarf::DW_FORM_ref_addr);
 }
 
 /// 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)