Re: [PATCH 01/15] aarch64: free tlsdesc data on dlclose [BZ #27403]

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 06/04/2021 10:43, Szabolcs Nagy wrote:
> The 04/01/2021 09:57, Adhemerval Zanella wrote:
>> On 15/02/2021 08:56, Szabolcs Nagy via Libc-alpha wrote:
>>> DL_UNMAP_IS_SPECIAL and DL_UNMAP were not defined. The definitions are
>>> now copied from arm, since the same is needed on aarch64. The cleanup
>>> of tlsdesc data is handled by the custom _dl_unmap.
>>>
>>> Fixes bug 27403.
>>> ---
>>>  sysdeps/aarch64/dl-lookupcfg.h | 27 +++++++++++++++++++++++++++
>>>  1 file changed, 27 insertions(+)
>>>  create mode 100644 sysdeps/aarch64/dl-lookupcfg.h
>>>
>>> diff --git a/sysdeps/aarch64/dl-lookupcfg.h b/sysdeps/aarch64/dl-lookupcfg.h
>>> new file mode 100644
>>> index 0000000000..c1ae676ae1
>>> --- /dev/null
>>> +++ b/sysdeps/aarch64/dl-lookupcfg.h
>>> @@ -0,0 +1,27 @@
>>> +/* Configuration of lookup functions.
>>> +   Copyright (C) 2006-2021 Free Software Foundation, Inc.
>>> +   This file is part of the GNU C Library.
>>> +
>>> +   The GNU C Library is free software; you can redistribute it and/or
>>> +   modify it under the terms of the GNU Lesser General Public
>>> +   License as published by the Free Software Foundation; either
>>> +   version 2.1 of the License, or (at your option) any later version.
>>> +
>>> +   The GNU C Library is distributed in the hope that it will be useful,
>>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> +   Lesser General Public License for more details.
>>> +
>>> +   You should have received a copy of the GNU Lesser General Public
>>> +   License along with the GNU C Library.  If not, see
>>> +   <https://www.gnu.org/licenses/>.  */
>>> +
>>> +#define DL_UNMAP_IS_SPECIAL
>>> +
>>> +#include_next <dl-lookupcfg.h>
>>> +
>>> +struct link_map;
>>> +
>>> +extern void _dl_unmap (struct link_map *map);
>>> +
>>> +#define DL_UNMAP(map) _dl_unmap (map)
>>>
>>
>> The fix looks ok to me (aarch64 supports tlsdesc, but it not
>> calling htab_delete) but _dl_unmap is replicated over aarch64, 
>> arm, i386, and x86_64 (the architectures that supports tlsdesc).
>> I think it would be simpler to add a define on linkmap.h to 
>> indicate the ABI supports tsldesc and consolidate the _dl_unmap 
>> code that call htab_delete on _dl_unmap_segments.
>>
>> It would allow to remove all the tlsdesc.c implementations (and
>> dl-lookupcfg.h completely on some architectures) and simplify
>> the required code if any other architectures decides to support
>> tlsdesc. 
> 
> i agree.
> 
> the last few patches allow even more refactoring
> (by removing lazy tlsdesc code paths from x86)
> 
> i think consolidation should be separate from the bug
> fixes, so i plan to commit this patch as is.

Right, but would work on the possible refactor? This should be that
complicate and would prevent other architectures to incur in the
same aarch64 mistake.