[Lldb-commits] [PATCH] Added support for reading thread-local storage variables, as defined using the __thread modifier.

Greg Clayton gclayton at apple.com
Wed Oct 16 11:19:58 PDT 2013 

On Oct 16, 2013, at 10:46 AM, Richard Mitton <richard at codersnotes.com> wrote:

> Is there any situation in which a DWARF expression can exist outside of a module? Surely they're only ever created from within .debug_info sections? I agree that the check should only happen if TLS is used, so I'll fix that.

Yes, but mostly for hand crafted expressions. I have been thinking that for conditional breakpoints it would be great if we could take an expression like "i==3" and turn that into a DWARF expression "DW_OP_reg22, DW_OP_constu(3), DW_OP_eq" and then ship that down to a remote GDB server and have it do the computation when breakpoints get hit to avoid the GDB remote packet traffic to speed things up a bit. This is future stuff, but something I have my eye on. You are correct in that all current DWARF expressions have a module context.

> 
> The management of thread local storage is indeed implicitly tied directly to the dynamic loader. Each module has it's own TLS section in the ELF, and so requires the linker (or dynamic linker therefore for shared modules) to arbitrate this.

Sounds good. Hopefully other platforms do the same kind of thing.
> 
> On systems that only support TLS via a get/set call (e.g. using the TlsAlloc method on Windows, pthread_get_specific on OSX), these are not variables and so the debugger is not concerned with them.

I believe we have the old manual code for pthread_get_specific()/pthread_set_specific(), but we did recently add support for the __thread keyword and I am not sure how we implemented it.

> 
> I agree with the rest of the comments and I'll update the patch to fix them.

Great! It will be a great patch to get in, I am looking forward to adding support for this on Darwin.

Greg

> 
> Richard Mitton
> richard at codersnotes.com
> 
> On 10/16/2013 10:29 AM, Greg Clayton wrote:
>> A few comments:
>> 1 - A module should not be required in DWARFExpression. The current code will fail to evaluate if the DWARF expression has no module. If your changed from DWARFExpression::Evaluate():
>> 
>>     ModuleSP module_sp = m_module_wp.lock();
>>     if (!module_sp.get())
>>     {
>>         if (error_ptr)
>>             error_ptr->SetErrorString("Module was unloaded.");
>>         return false;
>>     }
>> 
>> The check would be better made as:
>> 
>>     if (m_module_wp.expired())
>>     {
>>         if (error_ptr)
>>             error_ptr->SetErrorString("Module was unloaded.");
>>         return false;
>>     }
>> 
>> 2 - We should only fail to evaluate the expression for a DW_OP_XXX that _requires_ the module. The only thing that currently requires the module is DW_OP_GNU_push_tls_address. So we should only check the module in the DW_OP_GNU_push_tls_address case.
>> 
>> 3 - You will need to modify the DWARFExpression contractor:
>> 
>> DWARFExpression::DWARFExpression(lldb::ModuleSP module_sp, const DataExtractor& data, lldb::offset_t data_offset, lldb::offset_t data_length) :
>>     m_module_wp(module_sp),
>>     m_data(data, data_offset, data_length),
>>     m_reg_kind (eRegisterKindDWARF),
>>     m_loclist_slide(LLDB_INVALID_ADDRESS)
>> {
>> }
>> 
>> If "module_sp" is empty, it will throw an exception. So this needs to be:
>> 
>> DWARFExpression::DWARFExpression(lldb::ModuleSP module_sp, const DataExtractor& data, lldb::offset_t data_offset, lldb::offset_t data_length) :
>>     m_module_wp(),
>>     m_data(data, data_offset, data_length),
>>     m_reg_kind (eRegisterKindDWARF),
>>     m_loclist_slide(LLDB_INVALID_ADDRESS)
>> {
>>     if (module_sp)
>> 	m_module_wp = module_sp;
>> }
>> 
>> We shouldn't require a module.
>> 
>> 4 - We should be asking the lldb_private::Thread object itself for the thread local data, not the process on the thread's behalf. There can still be functions in lldb_private::Process if required, but they should be protected and only lldb_private::Thread should have access.
>> 
>> 5 - Does the functionality of determining the thread local storage really depend on the DYLD plug-in? Is it specific to the dynamic loader, or specific to the system itself? I know MacOSX can create processes that don't use dyld and they might have thread local storage. I will have to look into how we do this on MacOSX to be able to more intelligently speak to this.
>> 
>> 6 - The changes in source/Plugins/Process/Utility/RegisterContextLLDB.cpp will crash unless you fix #1
>> 
>> 
>> On Oct 15, 2013, at 3:39 PM, Richard Mitton <richard at codersnotes.com> wrote:
>> 
>>> Added support for reading thread-local storage variables, as defined using the __thread modifier. I think this should be fine as a first pass, but I'm throwing this out there for review.
>>> 
>>> To make this work this patch extends LLDB to:
>>> 
>>> - Explicitly track the link_map address for each module. This is effectively the module handle, not sure why it wasn't already being stored off anywhere. As an extension later, it would be nice if someone were to add support for printing this as part of the modules list.
>>> 
>>> - Allow reading the per-thread data pointer via ptrace. I have added support for Linux here. I'll be happy to add support for FreeBSD once this is reviewed. OS X does not appear to have __thread variables, so maybe we don't need it there. Windows support should eventually be workable along the same lines.
>>> 
>>> - Make DWARF expressions track which module they originated from.
>>> 
>>> - Add support for the DW_OP_GNU_push_tls_address DWARF opcode, as generated by gcc and recent versions of clang. Earlier versions of clang (such as 3.2, which is default on Ubuntu right now) do not generate TLS debug info correctly so can not be supported here.
>>> 
>>> - Understand the format of the pthread DTV block. This is where it gets tricky. We have three basic options here:
>>> 
>>>  1) Call "dlinfo" or "__tls_get_addr" on the inferior and ask it directly. However this won't work on core dumps, and generally speaking it's not a good idea for the debugger to call functions itself, as it has the potential to not work depending on the state of the target.
>>> 
>>>  2) Use libthread_db. This is what GDB does. However this option requires having a version of libthread_db on the host cross-compiled for each potential target. This places a large burden on the user, and would make it very hard to cross-debug from Windows to Linux, for example. Trying to build a library intended exclusively for one OS on a different one is not pleasant. GDB sidesteps the problem and asks the user to figure it out.
>>> 
>>>  3) Parse the DTV structure ourselves. On initial inspection this seems to be a bad option, as the DTV structure (the format used by the runtime to manage TLS data) is not in fact a kernel data structure, it is implemented entirely in useerland in libc. Therefore the layout of it's fields are version and OS dependent, and are not standardized.
>>> 
>>>  However, it turns out not to be such a problem. All OSes use basically the same algorithm (a per-module lookup table) as detailed in Ulrich Drepper's TLS ELF ABI document, so we can easily write code to decode it ourselves. The only question therefore is the exact field layouts required. Happily, the implementors of libpthread expose the structure of the DTV via metadata exported as symbols from the .so itself, designed exactly for this kind of thing. So this patch simply reads that metadata in, and re-implements libthread_db's algorithm itself. We thereby get cross-platform TLS lookup without either requiring third-party libraries, while still being independent of the version of libpthread being used.
>>> 
>>> Test case included.
>>> 
>>> 
>>> http://llvm-reviews.chandlerc.com/D1944
>>> 
>>> Files:
>>>  include/lldb/Expression/DWARFExpression.h
>>>  include/lldb/Target/DynamicLoader.h
>>>  include/lldb/Target/Process.h
>>>  source/Expression/DWARFExpression.cpp
>>>  source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
>>>  source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
>>>  source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
>>>  source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
>>>  source/Plugins/Process/Linux/ProcessMonitor.cpp
>>>  source/Plugins/Process/Linux/ProcessMonitor.h
>>>  source/Plugins/Process/POSIX/ProcessPOSIX.cpp
>>>  source/Plugins/Process/POSIX/ProcessPOSIX.h
>>>  source/Plugins/Process/Utility/RegisterContextLLDB.cpp
>>>  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
>>>  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
>>>  source/Target/Process.cpp
>>>  test/lang/c/tls_globals/Makefile
>>>  test/lang/c/tls_globals/TestTlsGlobals.py
>>>  test/lang/c/tls_globals/a.c
>>>  test/lang/c/tls_globals/main.c
>>> <D1944.1.patch>_______________________________________________
>>> lldb-commits mailing list
>>> lldb-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>

More information about the lldb-commits mailing list