[llvm] r192118 - Formally added an explicit enum for DWARF TLS support. No functionality change.

[Richard Mitton]

I've no idea why we use the GCC extension.

I believe this patch will probably break LLDB, I don't think it supports 
the DWARF3 TLS form.

Richard Mitton
richard at codersnotes.com

On 01/07/2014 05:19 PM, Adrian Prantl wrote:
> On Jan 7, 2014, at 17:12, dblaikie at gmail.com wrote:
>
>> On Tue Jan 07 2014 at 3:25:31 PM, Adrian Prantl <aprantl at apple.com> wrote:
>>
>> On Jan 7, 2014, at 15:23, Eric Christopher <echristo at gmail.com> wrote:
>>
>>> On Tue, Jan 7, 2014 at 3:09 PM, Adrian Prantl <aprantl at apple.com> wrote:
>>>> Hi Richard,
>>>>
>>>> [Apologies for replying to age-old commits]
>>> Heh. I've got 1 from June or something I still need to look at :)
>>>
>>>> I’m wondering why we are using the custom DW_OP_GNU_push_tls_address when there is a DW_OP_form_tls_address since DWARF 3. Are there historical or technical reasons why we do this? Could we change it to use the DWARF version?
>>>>
>>> We can. My guess is that there are pre-dwarf3 readers that understand
>>> the OP_GNU one and not the other, but I'm not very sympathetic. Could
>>> just pick it as < dwarf3 use the OP_GNU one, otherwise use the
>>> standard one.
>> So the two are functionally equivalent?
>>
>> I’m inclined to implement a patch that switches to the DWARF version for >=3 then. Would that break gdb on your end?
>>
>> Given that I implemented this functionality, I can probably speak with some authority... - I just implemented this based on what GCC does, and the fact that that passes the GDB 7.5 test suite. I didn't consult the DWARF standard, nor specifically justify implementing it this way for backwards compatibility.
>>
>> So, yeah, I'm happy for myself or anyone else to try doing this the standard-supported way, see if it passes the GDB 7.5 test suite/bot, and if it does, move on. If anyone else has any particular need for backcompat, they can speak up when things break. But given that no one noticed except me when running the GDB 7.5 suite... I'm not sure anyone does.
> Sounds great! Can you check whether this patch breaks the GDB testsuite?
>
> thanks,
> adrian
>
> diff --git a/lib/CodeGen/AsmPrinter/DwarfUnit.cpp b/lib/CodeGen/AsmPrinter/Dwarf
> index 04a2573..bfe1524 100644
> --- a/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> +++ b/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> @@ -1595,7 +1595,9 @@ void DwarfCompileUnit::createGlobalVariableDIE(DIGlobalVar
>           addUInt(Block, dwarf::DW_FORM_udata, DU->getAddrPoolIndex(Expr));
>         }
>         // 3) followed by a custom OP to make the debugger do a TLS lookup.
> -      addUInt(Block, dwarf::DW_FORM_data1, dwarf::DW_OP_GNU_push_tls_address);
> +      addUInt(Block, dwarf::DW_FORM_data1,
> +              DD->getDwarfVersion() <= 2 ? dwarf::DW_OP_GNU_push_tls_address
> +                                         : dwarf::DW_OP_form_tls_address);
>       } else {
>         DD->addArangeLabel(SymbolCU(this, Sym));
>         addOpAddress(Block, Sym);
>
>