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

[Adrian Prantl]

I filed PR18423 to keep track of this.
http://llvm.org/bugs/show_bug.cgi?id=18423

-- adrian


On Jan 7, 2014, at 20:03, dblaikie at gmail.com wrote:

> 
> 
> On Tue Jan 07 2014 at 5:20:54 PM, Adrian Prantl <aprantl at apple.com> 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?
> 
> Just tried it both with the GDB 7.5 test suite and the version of GDB 7.6 I have locally - neither version supports the DW_OP_form_tls_address.
> 
> 7.6:
> Unhandled dwarf expression opcode 0x9b
> 
> 
> 
> So this might be a non-starter for now. Someone can file a GDB bug, though.
> 
>  
> 
> 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);