| Message ID | 20240527214704.300444-1-jhubbard@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] selftests/user_events: silence a clang warning: address of packed member | expand |
Hi John, On Mon, May 27, 2024 at 02:47:04PM -0700, John Hubbard wrote: > When building with clang, via: > > make LLVM=1 -C tools/testing/selftest > > ...clang warns about "taking address of packed member 'write_index' ". > This is not particularly helpful, because the test code really wants to > write to exactly this location, and if it ends up being unaligned, then > the test won't work (and may segfault, depending on the CPU type). > > If that ever comes up, it will be obvious and can be fixed. But all it's > doing now is prevent a clean clang build, so disable the warning. > > Signed-off-by: John Hubbard <jhubbard@nvidia.com> > --- > > Changes since the first version: > > 1) Rebased onto Linux 6.10-rc1 > > > thanks, > John Hubbard > > > tools/testing/selftests/user_events/Makefile | 5 +++++ > 1 file changed, 5 insertions(+) > > 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. 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. > + 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 > >
On Tue, May 28, 2024 at 01:28:33PM -0700, Nathan Chancellor wrote: > Hi John, > > On Mon, May 27, 2024 at 02:47:04PM -0700, John Hubbard wrote: > > When building with clang, via: > > > > make LLVM=1 -C tools/testing/selftest > > > > ...clang warns about "taking address of packed member 'write_index' ". > > This is not particularly helpful, because the test code really wants to > > write to exactly this location, and if it ends up being unaligned, then > > the test won't work (and may segfault, depending on the CPU type). > > > > If that ever comes up, it will be obvious and can be fixed. But all it's > > doing now is prevent a clean clang build, so disable the warning. > > > > Signed-off-by: John Hubbard <jhubbard@nvidia.com> > > --- > > > > Changes since the first version: > > > > 1) Rebased onto Linux 6.10-rc1 > > > > > > thanks, > > John Hubbard > > > > > > tools/testing/selftests/user_events/Makefile | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > 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. 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. > 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. > > > + 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 > > > > >
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,
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),) + CFLAGS += -Wno-address-of-packed-member +endif + LDLIBS += -lrt -lpthread -lm TEST_GEN_PROGS = ftrace_test dyn_test perf_test abi_test
When building with clang, via: make LLVM=1 -C tools/testing/selftest ...clang warns about "taking address of packed member 'write_index' ". This is not particularly helpful, because the test code really wants to write to exactly this location, and if it ends up being unaligned, then the test won't work (and may segfault, depending on the CPU type). If that ever comes up, it will be obvious and can be fixed. But all it's doing now is prevent a clean clang build, so disable the warning. Signed-off-by: John Hubbard <jhubbard@nvidia.com> --- Changes since the first version: 1) Rebased onto Linux 6.10-rc1 thanks, John Hubbard tools/testing/selftests/user_events/Makefile | 5 +++++ 1 file changed, 5 insertions(+) base-commit: 2bfcfd584ff5ccc8bb7acde19b42570414bf880b