[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)

Vadim vadimcn at gmail.com
Thu Aug 8 15:11:54 PDT 2013


Carlo Kok <ck at ...> writes:

>
> Op 2-8-2013 23:30, David Blaikie schreef:
> >
> > Do you know if this is a limitation of the format, or just of the
> > debugger's ability to read? I've hit bugs before where only the 32 or
> > 64 bit case have been implemented because that's all that was needed.
> > If there is a sensible generic solution to 32 and 64 bit and it's just
> > GDB (or the linker) being silly & special casing, then it'd be nice to
> > document that we're working around a bug in another tool (& file a bug
> > for that tool).
> >
> > If you can use a .quad address, I'd expect a .long address if it
> > needed to be 32 bits.
> >
>
> You are right. GDB just didn't show the failure after this. as Vadim
> noted in the last comment here
> http://llvm.org/bugs/show_bug.cgi?id=16249
>
> It did load it, but didn't set a proper breakpoint when using "start" (I
> used run). I spent some more time researching this, what really needed
> to happen was that for dwarf addresses it shouldn't use secrel, for all
> others it should. Attached is a new patch that does this (had to
> introduce a new parameter as the dwarf type wasn't available at that
level).
>
> There were a few forms used for addresses that I could find:
> dwarf::DW_FORM_addr
> dwarf::DW_FORM_udata
> dwarf::DW_FORM_sdata
>
> these all pointed to text/data segments (in which it shouldn't use
> secrel). Attached is an incremental patch tested on 32 and 64bits mingw,
> if this patch is oke I'll commit, gdb now properly steps when using
> start too (on both mingw 32 and 64):
>
> clang -target x86_64-pc-mingw32 -g test2.c -o test2.exe
> gdb test2.exe
> ...
> This GDB was configured as "x86_64-w64-mingw32".
> (gdb) start
> Temporary breakpoint 1 at 0x401529: file test2.c, line 8.
> Starting program: C:\Projects\nco\cfe64\bin\Debug\test2.exe
> [New Thread 38532.0xa3c4]
>
> 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;
> (gdb) step
>
> --
> Carlo Kok
>
>
> Index: include/llvm/CodeGen/AsmPrinter.h
> ===================================================================
> --- include/llvm/CodeGen/AsmPrinter.h    (revision 187993)
> +++ include/llvm/CodeGen/AsmPrinter.h    (working copy)
>  <at>  <at>  -359,13 +359,13  <at>  <at>
>      /// 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 IsAddress =
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
IsAddress = false) const {
> +      EmitLabelPlusOffset(Label, 0, Size, IsAddress);
>      }
>
>
//===------------------------------------------------------------------===//
> Index: lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> ===================================================================
> --- lib/CodeGen/AsmPrinter/AsmPrinter.cpp    (revision 187993)
> +++ lib/CodeGen/AsmPrinter/AsmPrinter.cpp    (working copy)
>  <at>  <at>  -1415,9 +1415,9  <at>  <at>
>  /// 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 IsAddress)
>    const {
> -  if (MAI->needsDwarfSectionOffsetDirective() && Size == 4) { //
secrel32 ONLY works for 32bits.
> +  if (MAI->needsDwarfSectionOffsetDirective() && !IsAddress) { //
secrel32 ONLY works for 32bits.
>      OutStreamer.EmitCOFFSecRel32(Label);
>      return;
>    }
> Index: lib/CodeGen/AsmPrinter/DIE.cpp
> ===================================================================
> --- lib/CodeGen/AsmPrinter/DIE.cpp    (revision 187993)
> +++ lib/CodeGen/AsmPrinter/DIE.cpp    (working copy)
>  <at>  <at>  -293,7 +293,8  <at>  <at>
>  /// 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_addr
> +    || Form == dwarf::DW_FORM_udata || Form == dwarf::DW_FORM_sdata);
>  }
>
>  /// SizeOf - Determine size of label value in bytes.
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at ...
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>

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.

Finally, I want to apologize in advance if this appears non-constructive.
I would like to have fixed this myself, but this area is complex and
under-documented, especially on the subject of secrel32 relocations (which
seem to be PE-ELF specific), and I felt that there would be a very low
chance of me getting this right without studying this stuff at length
first.  I was hoping that an area expert, who alreadsy knows how this is
supposed to work, would have much easier time.  Or at least to chime in
with suggestions which way they'd like to see this fixed.

Vadim
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130808/8e65c885/attachment.html>


More information about the llvm-commits mailing list