2024-06-11 18:34:01

by Carlos Llamas

[permalink] [raw]
Subject: Re: [PATCH v2] selftests/vDSO: fix clang build errors and warnings

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.

>
> 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/[email protected]/
https://lore.kernel.org/all/[email protected]/

... I guess we somehow missed those?

>
> 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/[email protected]/

> 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?

>
> Signed-off-by: John Hubbard <[email protected]>
> ---

I would prefer to pick up the fixes from folks who sent out the patches
first but I'm fine either way.

Reviewed-by: Carlos Llamas <[email protected]>


2024-06-14 22:52:36

by John Hubbard

[permalink] [raw]
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/[email protected]/
> https://lore.kernel.org/all/[email protected]/
>
> ... 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/[email protected]/

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 <[email protected]>
>> ---
>
> 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 <[email protected]>

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


2024-06-14 22:56:33

by John Hubbard

[permalink] [raw]
Subject: Re: [PATCH v2] selftests/vDSO: fix clang build errors and warnings

On 6/14/24 3:51 PM, John Hubbard wrote:
> On 6/11/24 11:30 AM, Carlos Llamas wrote:
>> On Mon, May 27, 2024 at 02:16:22PM -0700, John Hubbard wrote:
...
>> Reviewed-by: Carlos Llamas <[email protected]>
>
> 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.
>

In fact, I have two more patches for vDSO (these are separate issues,
for the Makefile), and I just noticed that there are several accumulated
Reviewed-by and Tested-by tags on this patch here. So I think the way
forward is this:

I'll post a v3, with three patches for vDSO selftests, and the latest
tags. And let's see how that fares.

thanks,
--
John Hubbard
NVIDIA


2024-06-14 23:18:02

by Carlos Llamas

[permalink] [raw]
Subject: Re: [PATCH v2] selftests/vDSO: fix clang build errors and warnings

On Fri, Jun 14, 2024 at 03:56:01PM -0700, John Hubbard wrote:
> On 6/14/24 3:51 PM, John Hubbard wrote:
> > On 6/11/24 11:30 AM, Carlos Llamas wrote:
> > > On Mon, May 27, 2024 at 02:16:22PM -0700, John Hubbard wrote:
> ...
> > > Reviewed-by: Carlos Llamas <[email protected]>
> >
> > 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.
> >
>
> In fact, I have two more patches for vDSO (these are separate issues,
> for the Makefile), and I just noticed that there are several accumulated
> Reviewed-by and Tested-by tags on this patch here. So I think the way
> forward is this:
>
> I'll post a v3, with three patches for vDSO selftests, and the latest
> tags. And let's see how that fares.
>

Yeap, a resend would have been good. This time it might be best to bring
Andy and Thomas in the loop too.

Cc: Andy Lutomirski <[email protected]>
Cc: Thomas Gleixner <[email protected]>

> thanks,
> --
> John Hubbard
> NVIDIA
>