<div dir="ltr"><div id="yiv8212420959"><div id="yui_3_7_2_38_1375999649160_67"><div id="yui_3_7_2_38_1375999649160_66" style="color:rgb(0,0,0);background-color:rgb(255,255,255);font-family:arial,helvetica,sans-serif;font-size:12pt">
<div id="yiv8212420959yui_3_7_2_38_1375999014954_39">Carlo Kok <ck@...> writes:<br><br>> <br>> Op 2-8-2013 23:30, David Blaikie schreef:<br>> ><br>> > Do you know if this is a limitation of the format, or just of the<br>
> > debugger's ability to read? I've hit bugs before where only the 32 or<br>> > 64 bit case have been implemented because that's all that was needed.<br>> > If there is a sensible generic solution to 32 and 64 bit and it's just<br>
> > GDB (or the linker) being silly & special casing, then it'd be nice to<br>> > document that we're working around a bug in another tool (& file a bug<br>> > for that tool).<br>> ><br>
> > If you can use a .quad address, I'd expect a .long address if it<br>> > needed to be 32 bits.<br>> ><br>> <br>> You are
 right. GDB just didn't show the failure after this. as Vadim <br>> noted in the last comment here<br>> <a href="http://llvm.org/bugs/show_bug.cgi?id=16249">http://llvm.org/bugs/show_bug.cgi?id=16249</a><br>> <br>
> It did load it, but didn't set a proper breakpoint when using "start" (I <br>> used run). I spent some more time researching this, what really needed <br>> to happen was that for dwarf addresses it shouldn't use secrel, for all <br>
> others it should. Attached is a new patch that does this (had to <br>> introduce a new parameter as the dwarf type wasn't available at that level).<br>> <br>> There were a few forms used for addresses that I could find:<br>
> dwarf::DW_FORM_addr<br>> dwarf::DW_FORM_udata<br>> dwarf::DW_FORM_sdata<br>> <br>> these all pointed to text/data segments (in which it shouldn't use <br>> secrel). Attached is an incremental patch tested on 32 and 64bits mingw, <br>
> if this patch is oke I'll commit, gdb now properly steps when
 using <br>> start too (on both mingw 32 and 64):<br>> <br>> clang -target x86_64-pc-mingw32 -g test2.c -o test2.exe<br>> gdb test2.exe<br>> ...<br>> This GDB was configured as "x86_64-w64-mingw32".<br>
> (gdb) start<br>> Temporary breakpoint 1 at 0x401529: file test2.c, line 8.<br>> Starting program: C:\Projects\nco\cfe64\bin\Debug\test2.exe<br>> [New Thread 38532.0xa3c4]<br>> <br>> Temporary breakpoint 1, main () at test2.c:8<br>
> 8               foobar(17);<br>> (gdb) step<br>> foobar (x=17) at test2.c:3<br>> 3           return x+42;<br>> (gdb) step<br>> main () at test2.c:9<br>> 9               return 0;<br>> (gdb) step<br>
> <br>> --<br>> Carlo Kok<br>> <br>> <br>> Index:
 include/llvm/CodeGen/AsmPrinter.h<br>> ===================================================================<br>> --- include/llvm/CodeGen/AsmPrinter.h    (revision 187993)<br>> +++ include/llvm/CodeGen/AsmPrinter.h    (working copy)<br>
>  <at>  <at>  -359,13 +359,13  <at>  <at> <br>>      /// where the size in bytes of the directive is specified by Size and Label<br>>      /// specifies the label.  This implicitly uses .set if it is available.<br>
>      void EmitLabelPlusOffset(const MCSymbol *Label, uint64_t Offset,<br>> -                                   unsigned Size) const;<br>>
 +                                   unsigned Size, bool IsAddress = false) const;<br>> <br>>      /// EmitLabelReference - Emit something like ".long Label"<br>>      /// where the size in bytes of the directive is specified by Size and Label<br>
>      /// specifies the label.<br>> -    void EmitLabelReference(const MCSymbol *Label, unsigned Size) const {<br>> -      EmitLabelPlusOffset(Label, 0, Size);<br>> +    void EmitLabelReference(const MCSymbol *Label, unsigned Size, bool IsAddress = false) const {<br>
> +      EmitLabelPlusOffset(Label, 0, Size, IsAddress);<br>>      }<br>>
 <br>>      //===------------------------------------------------------------------===//<br>> Index: lib/CodeGen/AsmPrinter/AsmPrinter.cpp<br>> ===================================================================<br>
> --- lib/CodeGen/AsmPrinter/AsmPrinter.cpp    (revision 187993)<br>> +++ lib/CodeGen/AsmPrinter/AsmPrinter.cpp    (working copy)<br>>  <at>  <at>  -1415,9 +1415,9  <at>  <at> <br>>  /// where the size in bytes of the directive is specified by Size and Label<br>
>  /// specifies the label.  This implicitly uses .set if it is available.<br>>  void AsmPrinter::EmitLabelPlusOffset(const MCSymbol *Label, uint64_t Offset,<br>>
 -                                      unsigned Size)<br>> +                                      unsigned Size, bool IsAddress)<br>>    const {<br>> -  if (MAI->needsDwarfSectionOffsetDirective() && Size == 4) { // secrel32 ONLY works for 32bits.<br>
> +  if (MAI->needsDwarfSectionOffsetDirective() && !IsAddress) { // secrel32 ONLY works for 32bits.<br>>      OutStreamer.EmitCOFFSecRel32(Label);<br>>      return;<br>>    }<br>> Index:
 lib/CodeGen/AsmPrinter/DIE.cpp<br>> ===================================================================<br>> --- lib/CodeGen/AsmPrinter/DIE.cpp    (revision 187993)<br>> +++ lib/CodeGen/AsmPrinter/DIE.cpp    (working copy)<br>
>  <at>  <at>  -293,7 +293,8  <at>  <at> <br>>  /// EmitValue - Emit label value.<br>>  ///<br>>  void DIELabel::EmitValue(AsmPrinter *AP, uint16_t Form) const {<br>> -  AP->EmitLabelReference(Label, SizeOf(AP, Form));<br>
> +  AP->EmitLabelReference(Label, SizeOf(AP, Form), Form == dwarf::DW_FORM_addr <br>> +    || Form == dwarf::DW_FORM_udata || Form == dwarf::DW_FORM_sdata);<br>>  }<br>> <br>>  /// SizeOf - Determine size of label value in bytes.<br>
> <br>> <br>> _______________________________________________<br>> llvm-commits mailing
 list<br>> llvm-commits@...<br>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>> <br><br>If I may throw in my 2c:<br><br>Carlo, I am afraid it may be not as simple as excluding those 3 DW_FORM's.  <br>
Please
 check out the equivalent GCC source here: 
<a href="https://github.com/mirrors/gcc/blob/master/gcc/dwarf2out.c">https://github.com/mirrors/gcc/blob/master/gcc/dwarf2out.c</a> and search it
 for calls to dw2_asm_output_offset (which eventually expands to 
.secrel32).  <br id="yiv8212420959yui_3_7_2_38_1375999014954_115">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).</div><div class="" id="yiv8212420959yui_3_7_2_38_1375999014954_83" style="color:rgb(0,0,0);font-size:16px;font-family:arial,helvetica,sans-serif;background-color:transparent;font-style:normal">
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?..<br><br>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 <a rel="nofollow" target="_blank" href="http://dwarfstd.org/doc/DWARF4.pdf">here</a>,
 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 <a rel="nofollow" target="_blank" href="http://llvm.org/bugs/show_bug.cgi?id=16249">LLVM bug 16249</a>).<br>
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.<br><br>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.<br><br>Vadim<br></div></div></div></div></div>