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

[Eric Christopher]

Can you add the DwarfDebug.cpp part of the change separately.
Otherwise it should be fine.

Thanks.

-eric

On Mon, Aug 12, 2013 at 6:54 AM, Carlo Kok <ck at remobjects.com> wrote:
> 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;
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>