Bug 375514 - valgrind_get_tls_addr() does not work in case of static TLS
Summary: valgrind_get_tls_addr() does not work in case of static TLS
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-24 17:55 UTC by Aleksandar Rikalo
Modified: 2017-03-14 17:21 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
GDB server - valgrind_get_tls_addr() fix for MIPS (1.88 KB, patch)
2017-01-24 17:55 UTC, Aleksandar Rikalo
Details
GDB server - valgrind_get_tls_addr() fix for MIPS v2 (2.31 KB, patch)
2017-01-27 11:35 UTC, Aleksandar Rikalo
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Aleksandar Rikalo 2017-01-24 17:55:51 UTC
Created attachment 103619 [details]
GDB server - valgrind_get_tls_addr() fix for MIPS

This is about valgrind_get_tls_addr() from m_gdbserver/target.c.
valgrind_get_tls_addr() fails on MIPS (and maybe on some other platforms) when given module uses static TLS. Proposed patch fixes problem for MIPS.
Changes are made according to td_thr_tlsbase() function from GLIBC/nptl_db/td_thr_tlsbase.c.
Related test is gdbserver_tests/hgtls.
Comment 1 Petar Jovanovic 2017-01-25 19:01:51 UTC
@Philippe
Are you ok with this change?
Comment 2 Philippe Waroquiers 2017-01-25 21:46:49 UTC
IIUC, on mips, the test hgtls fails to properly show the static thread variable:
static __thread int local;

I do not understand why we need this code on mips
but e.g. not on other platforms such as x86, amd64 or ppc.
Any idea ? If yes, it would be good to document why mips is special.


That being said, as this (mips only) code fixes the test hgtls, it looks good
to commit this.

Some small comments:
* As far as I can see, there is no (easy) way to modify auxprogs/getoff.c to
  also generate the offset of tls_offset.
  So, I would add a comment in the code, such as:
  /* To check the assumption, start a gdb on none/tests/tls and do:
      p &((struct link_map*)0x0)->l_tls_modid
      p &((struct link_map*)0x0)->l_tls_offset */

* the function valgrind_get_tls_addr has a lot of dlog calls to indicate
  which branch(es) it takes. It would be good to add a dlog (1, ...)
  inside the 'if' to trace this branch is being taken.

Thanks for the analysis and patch.
(please modify NEWS also, when committing).
Comment 3 Petar Jovanovic 2017-01-26 23:51:19 UTC
(In reply to Philippe Waroquiers from comment #2)
> IIUC, on mips, the test hgtls fails to properly show the static thread
> variable:
> static __thread int local;
> 
> I do not understand why we need this code on mips
> but e.g. not on other platforms such as x86, amd64 or ppc.
> Any idea ? If yes, it would be good to document why mips is special.
> 

Actually, I believe we will need this code on other architectures too.
IIUC, this issue may be more apparent with 2.22+ glibc and is dependent
on used TLS model. Aleksandar has marked platform dependent calculation
in the patch that needs to be done per arch.
I can see that gdbserver_tests/hgtls is failing for other architectures
too, maybe someone can test if this patch will help (and make necessary
changes in the aforementioned calculations).
Comment 4 Aleksandar Rikalo 2017-01-27 11:35:12 UTC
Created attachment 103665 [details]
GDB server - valgrind_get_tls_addr() fix for MIPS v2

Suggested comments and dlog(...) are added.
Comment 5 Petar Jovanovic 2017-01-30 19:37:04 UTC
(In reply to Aleksandar Rikalo from comment #4)
> Created attachment 103665 [details]
> GDB server - valgrind_get_tls_addr() fix for MIPS v2
> 
> Suggested comments and dlog(...) are added.

Patch committed in r16215.
Thanks.
Comment 6 Petar Jovanovic 2017-03-14 17:08:10 UTC
We should close this issue now.