On Tue, 09 Apr 2024 07:03:45 PDT (-0700), adhemerval.zanella@linaro.org wrote: > > > On 09/04/24 05:30, Szabolcs Nagy wrote: >> The 04/08/2024 13:57, Adhemerval Zanella Netto wrote: >>> On 08/04/24 04:26, Szabolcs Nagy wrote: >>>> The 04/07/2024 16:29, Cristian Rodríguez wrote: >>>>> On Fri, Apr 5, 2024 at 11:59 AM Szabolcs Nagy wrote: >>>>>> The 04/05/2024 09:35, Adhemerval Zanella wrote: >>>>>>> Use the hidden alias instead. >>>>>>> >>>>>>> Checked on aarch64-linux-gnu. >>>>>> >>>>>> does this change behaviour in case __tls_get_addr is interposed? >>>>> >>>>> Wut ? is that really supported.. I mean.. isn't that symbol prefix >>>>> reserved for the implementation and any assumption about it is either >>>>> ID or UB? >>>> >>>> a behaviour can change even if it's not supported. >>>> i did not try to imply that it should be supported. >>>> >>>> i know sanitizers interpose __tls_get_addr, because >>>> https://sourceware.org/bugzilla/show_bug.cgi?id=16291 >>>> i don't know if that hack works at all now for tlsdesc >>>> (where the ld.so calls __tls_get_addr, not user code) >>>> >>>> my question was if we investigated this issue since it >>>> is useful to document then in the commit msg (or news >>>> entry if this affects users) >>> >>> This change 'breaks' the sanitizer trick to get the dynamic TLS, with >>> this patch I now see: >>> >>> MemorySanitizer-AARCH64 :: dtls_test.c >>> SanitizerCommon-asan-aarch64-Linux :: Linux/resize_tls_dynamic.cpp >>> SanitizerCommon-msan-aarch64-Linux :: Linux/resize_tls_dynamic.cpp >>> SanitizerCommon-tsan-aarch64-Linux :: Linux/resize_tls_dynamic.cpp >>> >>> And it does not fail on x86 only because it uses -mtls=gnu as default >>> (the same tests fail on x86 with -mtls=gnu2). >>> >>> Now that GCC and distributions are aiming to use GNU2/DESC as the >>> default TLS, this hack will also break on x86. So the question is >>> whether we revert 050f7298e1ecc39887c329037575ccd972071255 and >>> document that __tls_get_addr should be interposable, or move with this >>> change and try to come up with a possible solution for BZ#16291. >>> >>> I bringing this because we will have another two ABIs with tlsdesc >>> support (loongarch and riscv). >> >> adding some sanitizer committers to cc. >> >> tl;dr: in the next glibc release tlsdesc will not call >> __tls_get_addr in an interposable way in the dynamic tls >> allocation case, unless somebody screems that this is needed. >> (affects targets that may default to tlsdesc, but note that >> the dynamic case only triggers with tlsdesc when a lot of >> dlopened tls is used, otherwise static tls area is used) > > Just a note that this already true for x86 with -mtls=gnu2 since > 2.21. And now that distro are aiming to make it default, this issues > will happen more often. > >> >> i think it is also possible that we will use custom malloc >> in ld.so which may be just as big change for the sanitizers. >> (this can make tls access signal safe) >> >> i'm not against the change, but if we plan to add several >> interposable hooks as in >> https://sourceware.org/glibc/wiki/ThreadPropertiesAPI >> then we might as well keep __tls_get_addr PLT for now. >> > > I don't have a strong opinion, but what I really want is to have > consistency over the architectures. Meaning that if we want to keep > the __tls_get_addr PLT for sanitizer/runtime hooks, it would be good > to revert the x86 change. > > It also means to document it properly somewhere and make the new > RISC-V and loongarch follow the same guidelines. I also don't have a strong opinion on whether __tls_get_addr should be interposable, but I'm happy to try and make the RISC-V port match arm64/x86. I guess we're kind of safe for now as we don't have TLSDESC merged, though I think we were getting pretty close there so we should probbaly decide before we accidentally commit to an ABI. > I will take a look again on the ThreadPropertiesAPI, since it is has > been more and more a demanding issue.