[PATCH v4 2/2] x86: Update _dl_tlsdesc_dynamic to preserve caller-saved registers

[Adhemerval Zanella Netto]

On 16/02/24 09:37, H.J. Lu wrote:
> On Fri, Feb 16, 2024 at 4:20 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>> On Fri, Feb 16, 2024 at 4:18 AM Florian Weimer <fweimer@redhat.com> wrote:
>>>
>>> * H. J. Lu:
>>>
>>>> On Thu, Feb 15, 2024 at 10:23 PM Florian Weimer <fweimer@redhat.com> wrote:
>>>>>
>>>>> * Adhemerval Zanella Netto:
>>>>>
>>>>>>> diff --git a/elf/malloc-for-test.map b/elf/malloc-for-test.map
>>>>>>> new file mode 100644
>>>>>>> index 0000000000..8437cf4346
>>>>>>> --- /dev/null
>>>>>>> +++ b/elf/malloc-for-test.map
>>>>>>> @@ -0,0 +1,6 @@
>>>>>>> +GLIBC_2.0 {
>>>>>>
>>>>>> You need to use the correct version to override the malloc:
>>>>>>
>>>>>> $ gdb --args tst-gnu2-tls2 --direct
>>>>>> [...]
>>>>>> (gdb) b apply_tls
>>>>>> (gdb) r
>>>>>> Thread 2 "tst-gnu2-tls2" hit Breakpoint 1, apply_tls (p=0x7ffff7bfee80) at tst-gnu2-tls2mod1.c:25
>>>>>> 25 {
>>>>>> (gdb) b malloc
>>>>>> Breakpoint 2 at 0x7ffff7ca8ad0: malloc. (3 locations)
>>>>>> (gdb) c
>>>>>> Continuing.
>>>>>
>>>>> Why do we need to set a symbol version here?  I think this can be
>>>>> removed.
>>>>
>>>> malloc-for-test.so provides a fake malloc to clobber caller-save
>>>> registers.  Since malloc in ld.so has a symbol version,
>>>>
>>>>   struct r_found_version version;
>>>>   version.name = symbol_version_string (libc, GLIBC_2_0);
>>>>   version.hidden = 0;
>>>>   version.hash = _dl_elf_hash (version.name);
>>>>   version.filename = NULL;
>>>>
>>>>   void *new_calloc = lookup_malloc_symbol (main_map, "calloc", &version);
>>>>   void *new_free = lookup_malloc_symbol (main_map, "free", &version);
>>>>   void *new_malloc = lookup_malloc_symbol (main_map, "malloc", &version);
>>>>   void *new_realloc = lookup_malloc_symbol (main_map, "realloc", &version);
>>>>
>>>> malloc in malloc-for-test.so must have the same version.  Otherwise,
>>>> it won't be used in the test.
>>>
>>> I thought that unversioned symbols interpose all versioned symbols.  Has
>>> this changed in the dynamic linker?
>>
>> Only for this case.
>>
> 
> I was wrong.  Version isn't needed.  Will fix it.
> 

Indeed, but I think we also improve the register check testing since just clobbering
on malloc does not really provide much coverage.