linux-kernel - Re: [PATCH v2] selftests/user_events: silence a clang warning: address of packed member

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] [day] [month] [year] [list]

Date: Tue, 28 May 2024 13:54:07 -0700
From: John Hubbard <jhubbard@...dia.com>
To: Nathan Chancellor <nathan@...nel.org>
CC: Shuah Khan <shuah@...nel.org>, Beau Belgrave <beaub@...ux.microsoft.com>,
	Steven Rostedt <rostedt@...dmis.org>, Mark Brown <broonie@...nel.org>,
	"Naresh Kamboju" <naresh.kamboju@...aro.org>, sunliming
	<sunliming@...inos.cn>, "Masami Hiramatsu" <mhiramat@...nel.org>, 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/user_events: silence a clang warning:
 address of packed member

On 5/28/24 1:35 PM, Nathan Chancellor wrote:
> On Tue, May 28, 2024 at 01:28:33PM -0700, Nathan Chancellor wrote:
..
>>> diff --git a/tools/testing/selftests/user_events/Makefile b/tools/testing/selftests/user_events/Makefile
>>> index 10fcd0066203..617e94344711 100644
>>> --- a/tools/testing/selftests/user_events/Makefile
>>> +++ b/tools/testing/selftests/user_events/Makefile
>>> @@ -1,5 +1,10 @@
>>>   # SPDX-License-Identifier: GPL-2.0
>>>   CFLAGS += -Wl,-no-as-needed -Wall $(KHDR_INCLUDES)
>>> +
>>> +ifneq ($(LLVM),)
>>
>> Perhaps it would be better if this were
>>
>>    ifeq ($(CC),clang)
>>
>> as that would catch both CC=clang and LLVM=1 users? I haven't tested
>> this though.

That exact fix wouldn't quite work for the LLVM=1 case, because there,
CC is set to a horrible long mess, like this:

     CC = clang --target=x86_64-pc-linux-gnu -fintegrated-as

..but I see what you mean. It's not covering the CC=clang case.

> 
> Hmmm, now that I am actually looking at tools/testing/selftests/lib.mk,
> it seems like CC is only set to clang when $(LLVM) is set, so keeping it
> the way it is now is probably best. Sorry about that, I am not too
> familiar with the tools build system.
> 

That's true, and the patch here was admittedly only attempting to fix
the LLVM=1 case. However, I recall someone is building these kselftests
via "make CC=clang", which caught me by surprise, so I suppose that
should be fixed too. Maybe all of these checks (there are a few other
patches, but none merged yet) should be effectively:

     "if LLVM==1 or CC==clang, then apply the clang fix(es)"
	

>> Additionally, I think it would be good to mention that
>> -Wno-address-of-packed-member is GCC's default, whereas Clang enables
>> -Waddress-of-packed-member by default.

OK, I'll add that to a short comment nearby.

>>
>>> +    CFLAGS += -Wno-address-of-packed-member
>>> +endif
>>> +
>>>   LDLIBS += -lrt -lpthread -lm
>>>   
>>>   TEST_GEN_PROGS = ftrace_test dyn_test perf_test abi_test
>>>
>>> base-commit: 2bfcfd584ff5ccc8bb7acde19b42570414bf880b
>>> -- 
>>> 2.45.1
>>>
>>>
>>

thanks,
-- 
John Hubbard
NVIDIA