[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1886e4a4-7c19-4a0f-9689-9edda970d508@linuxfoundation.org>
Date: Tue, 3 Sep 2024 15:24:44 -0600
From: Shuah Khan <skhan@...uxfoundation.org>
To: Fangrui Song <maskray@...gle.com>
Cc: Xi Ruoyao <xry111@...111.site>, Shuah Khan <shuah@...nel.org>,
linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org,
Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH] selftests/vDSO: support DT_GNU_HASH
On 8/27/24 07:37, Fangrui Song wrote:
> On Tue, Aug 27, 2024 at 10:12 PM Shuah Khan <skhan@...uxfoundation.org> wrote:
>>
>> On 8/26/24 00:07, Xi Ruoyao wrote:
>>> On Wed, 2024-08-14 at 20:26 -0700, Fangrui Song wrote:
>>>> glibc added support for DT_GNU_HASH in 2006 and DT_HASH has been
>>>> obsoleted for more than one decade in many Linux distributions.
>>>>
>>>> Many vDSOs support DT_GNU_HASH. This patch adds selftests support.
>>>>
>>>> Signed-off-by: Fangrui Song <maskray@...gle.com>
>>>> ---
>>>
>>> Ping.
>>>
>>> Some context: I'd change LoongArch vDSO to use the toolchain default
>>> instead of forcing DT_HASH (note that LoongArch is launched decades
>>> after all major distros switched to DT_GNU_HASH), but without the
>>> selftest support we'll lose test coverage.
>>>
>>> And now ARM64 has already lost test coverage after commit 48f6430505c0.
>>>
>>
>> I am seeing several checkpatch errors - please fix them and send me v2.
>>
>> thanks,
>> -- Shuah
>>
>
> The applicable change is:
>
> --- i/tools/testing/selftests/vDSO/parse_vdso.c
> +++ w/tools/testing/selftests/vDSO/parse_vdso.c
> @@ -177,7 +177,7 @@ void vdso_init_from_sysinfo_ehdr(uintptr_t base)
> if (vdso_info.gnu_hash) {
> vdso_info.nbucket = vdso_info.gnu_hash[0];
> /* The bucket array is located after the header (4
> uint32) and the bloom
> - filter (size_t array of gnu_hash[2] elements). */
> + * filter (size_t array of gnu_hash[2] elements). */
> vdso_info.bucket = vdso_info.gnu_hash + 4 +
> sizeof(size_t) / 4 * vdso_info.gnu_hash[2];
> } else {
>
>
> Other checkpatch.pl output is not actionable. `ELF(Sym) *sym` instead
> of `ELF(Sym) * sym` has the correct spacing (used in this file and
> elsewhere ElfW in the code base).
>
>
Okay. Send v2 with the actionable change.
thanks,
-- Shuah
Powered by blists - more mailing lists