linux-kernel - Re: [PATCH v2] selftests/vDSO: fix clang build errors and warnings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2461f002-fdd4-4bf5-b797-827069cafc4b@nvidia.com>
Date: Fri, 14 Jun 2024 15:51:56 -0700
From: John Hubbard <jhubbard@...dia.com>
To: Carlos Llamas <cmllamas@...gle.com>
CC: Shuah Khan <shuah@...nel.org>, Mark Brown <broonie@...nel.org>, "Vincenzo
 Frascino" <vincenzo.frascino@....com>, Colin Ian King
	<colin.i.king@...il.com>, Valentin Obst <kernel@...entinobst.de>,
	<linux-kselftest@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>,
	<llvm@...ts.linux.dev>
Subject: Re: [PATCH v2] selftests/vDSO: fix clang build errors and warnings

On 6/11/24 11:30 AM, Carlos Llamas wrote:
> On Mon, May 27, 2024 at 02:16:22PM -0700, John Hubbard wrote:
>> When building with clang, via:
>>
>>      make LLVM=1 -C tools/testing/selftests
>>
>> ...there are several warnings, and an error. This fixes all of those and
>> allows these tests to run and pass.
> 
> It might be best to split the 4 _different_ fixes into separate patches.

If necessary, that can be done. It's sufficiently difficult to attract
attention for selftests (see below) that I'm reluctant to make it even
harder to get it all fixed, though.

> 
>>
>> 1. Fix linker error (undefined reference to memcpy) by providing a local
>>     version of memcpy.
>>
>> 2. clang complains about using this form:
>>
>>      if (g = h & 0xf0000000)
>>
>> ...so factor out the assignment into a separate step.
> 
> There has been multiple attempts to fix this. I can see these two:
> https://lore.kernel.org/all/20211206102931.1433871-1-anders.roxell@linaro.org/
> https://lore.kernel.org/all/20240501002150.1370861-1-edliaw@google.com/
> 
> ... I guess we somehow missed those?

I guess so. :)

> 
>>
>> 3. The code is passing a signed const char* to elf_hash(), which expects
>>     a const unsigned char *. There are several callers, so fix this at
>>     the source by allowing the function to accept a signed argument, and
>>     then converting to unsigned operations, once inside the function.
>>
> 
> There is also a v4 fix for this item that was sent out here:
> https://lore.kernel.org/all/20240506181951.1804451-1-edliaw@google.com/

No idea why these fixes are not getting picked up.

> 
>> 4. clang doesn't have __attribute__((externally_visible)) and generates
>>     a warning to that effect. Fortunately, gcc 12 and gcc 13 do not seem
>>     to require that attribute in order to build, run and pass tests here,
>>     so remove it.
>>
>> [1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c49f@valentinobst.de/
> 
> What is this about? Left over from v1 maybe?

Maybe.

> 
>>
>> Signed-off-by: John Hubbard <jhubbard@...dia.com>
>> ---
> 
> I would prefer to pick up the fixes from folks who sent out the patches
> first but I'm fine either way.

I'm perfectly fine with dropping this and letting the other patches go
in, but *something* needs to go in.

> 
> Reviewed-by: Carlos Llamas <cmllamas@...gle.com>

Thanks for the review! I have no earthly idea what will happen next.
I'd like to hear at least something from the maintainers about their
intentions here.

thanks,
-- 
John Hubbard
NVIDIA