* [Bug] Failing kunit test on ARCH=arm and LLVM=1 @ 2024-06-07 12:37 Christian Schrefl 2024-06-07 14:33 ` Nathan Chancellor 0 siblings, 1 reply; 3+ messages in thread From: Christian Schrefl @ 2024-06-07 12:37 UTC (permalink / raw) To: llvm, linux-kernel; +Cc: kunit-dev, Andrew Morton Greetings, when trying to port Rust to ARM I noticed that the DEFINE_FLEX_test kunit test in lib/overflow_kunit.c:1188 fails when combining LLVM=1 and ARCH=arm. I have reproduced this on v6.10-rc1 and next-20240606. Here is the clang/llvm version I'm using: clang version 18.1.6 (Fedora 18.1.6-3.fc40) Target: x86_64-redhat-linux-gnu Thread model: posix InstalledDir: /usr/bin Configuration file: /etc/clang/x86_64-redhat-linux-gnu-clang.cfg I have not looked closer at the failure so I'm unsure if this is a problem with LLVM or if the test case is to speciffic. Let me know if I should open a issue at https://github.com/ClangBuiltLinux/linux/issues instead. Cheers, Christian ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Bug] Failing kunit test on ARCH=arm and LLVM=1 2024-06-07 12:37 [Bug] Failing kunit test on ARCH=arm and LLVM=1 Christian Schrefl @ 2024-06-07 14:33 ` Nathan Chancellor 2024-06-10 18:24 ` Kees Cook 0 siblings, 1 reply; 3+ messages in thread From: Nathan Chancellor @ 2024-06-07 14:33 UTC (permalink / raw) To: Christian Schrefl; +Cc: llvm, linux-kernel, kunit-dev, Andrew Morton, Kees Cook Hi Christian, On Fri, Jun 07, 2024 at 02:37:19PM +0200, Christian Schrefl wrote: > Greetings, > > when trying to port Rust to ARM I noticed that the DEFINE_FLEX_test > kunit test in lib/overflow_kunit.c:1188 fails when combining LLVM=1 > and ARCH=arm. > > I have reproduced this on v6.10-rc1 and next-20240606. > > Here is the clang/llvm version I'm using: > clang version 18.1.6 (Fedora 18.1.6-3.fc40) > Target: x86_64-redhat-linux-gnu > Thread model: posix > InstalledDir: /usr/bin > Configuration file: /etc/clang/x86_64-redhat-linux-gnu-clang.cfg > > I have not looked closer at the failure so I'm unsure if this is a > problem with LLVM or if the test case is to speciffic. Thanks a lot for the report! I can reproduce this with tip of tree LLVM as well. $ echo 'CONFIG_KUNIT=y CONFIG_OVERFLOW_KUNIT_TEST=y CONFIG_RUNTIME_KERNEL_TESTING_MENU=y' >kernel/configs/repro.config $ make -skj"$(nproc)" ARCH=arm LLVM=1 {def,repro.}config zImage $ boot-qemu.py -a arm -k . ... [ 0.000000] Linux version 6.10.0-rc2-00235-g8a92980606e3 (nathan@thelio-3990X) (ClangBuiltLinux clang version 19.0.0git (https://github.com/llvm/llvm-project e635520be888335dd59874038d33e60cca3a7143), ClangBuiltLinux LLD 19.0.0) #1 SMP Fri Jun 7 06:12:02 MST 2024 ... [ 1.832472] # castable_to_type_test: 75 castable_to_type() tests finished [ 1.833483] ok 21 castable_to_type_test [ 1.834122] # DEFINE_FLEX_test: EXPECTATION FAILED at lib/overflow_kunit.c:1188 [ 1.834122] Expected __builtin_dynamic_object_size(two, 0) == sizeof(struct foo) + sizeof(s16) + sizeof(s16), but [ 1.834122] __builtin_dynamic_object_size(two, 0) == 8 (0x8) [ 1.834122] sizeof(struct foo) + sizeof(s16) + sizeof(s16) == 12 (0xc) [ 1.834746] not ok 22 DEFINE_FLEX_test ... I don't see the same failure with GCC 13.2.0. This test fails when building for arm64 and x86_64 as well, so it does not appear to be architecture specific. I think I see what is going on here. Looking at the documentation for DEFINE_RAW_FLEX(), it states "Define an on-stack instance of structure with a trailing flexible array member, when it does not have a __counted_by annotation." but commit d8e45f2929b9 ("overflow: Change DEFINE_FLEX to take __counted_by member") defined 'struct foo' with __counted_by on it. __counted_by informs __builtin_dynamic_object_size() about the size of the flexible array. With DEFINE_FLEX_RAW, the counter is zero, so the size of array in 'struct foo' is zero, meaning this test is incorrect when built with a compiler that supports __counted_by, which is just Clang 18+ right now (it should land in GCC 15 if I understand correctly). I see two potential solutions that work for me. One would be to stop using DEFINE_RAW_FLEX() and match the other uses (but I assume testing DEFINE_RAW_FLEX() was intentional): diff --git a/lib/overflow_kunit.c b/lib/overflow_kunit.c index 4ef31b0bb74d..883670adf0cc 100644 --- a/lib/overflow_kunit.c +++ b/lib/overflow_kunit.c @@ -1180,7 +1180,7 @@ struct foo { static void DEFINE_FLEX_test(struct kunit *test) { - DEFINE_RAW_FLEX(struct foo, two, array, 2); + DEFINE_FLEX(struct foo, two, array, counter, 2); DEFINE_FLEX(struct foo, eight, array, counter, 8); DEFINE_FLEX(struct foo, empty, array, counter, 0); The other would be making the size of the array conditional on not having __counted_by support (which is admittedly ugly): diff --git a/lib/overflow_kunit.c b/lib/overflow_kunit.c index 4ef31b0bb74d..7eed0890e25f 100644 --- a/lib/overflow_kunit.c +++ b/lib/overflow_kunit.c @@ -1185,7 +1185,11 @@ static void DEFINE_FLEX_test(struct kunit *test) DEFINE_FLEX(struct foo, empty, array, counter, 0); KUNIT_EXPECT_EQ(test, __struct_size(two), - sizeof(struct foo) + sizeof(s16) + sizeof(s16)); + sizeof(struct foo) +#if !__has_attribute(__counted_by__) + + sizeof(s16) + sizeof(s16) +#endif + ); KUNIT_EXPECT_EQ(test, __struct_size(eight), 24); KUNIT_EXPECT_EQ(test, __struct_size(empty), sizeof(struct foo)); } Kees, am I missing anything here? Cheers, Nathan ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Bug] Failing kunit test on ARCH=arm and LLVM=1 2024-06-07 14:33 ` Nathan Chancellor @ 2024-06-10 18:24 ` Kees Cook 0 siblings, 0 replies; 3+ messages in thread From: Kees Cook @ 2024-06-10 18:24 UTC (permalink / raw) To: Nathan Chancellor Cc: Christian Schrefl, llvm, linux-kernel, kunit-dev, Andrew Morton On Fri, Jun 07, 2024 at 07:33:29AM -0700, Nathan Chancellor wrote: > Hi Christian, > > On Fri, Jun 07, 2024 at 02:37:19PM +0200, Christian Schrefl wrote: > > Greetings, > > > > when trying to port Rust to ARM I noticed that the DEFINE_FLEX_test > > kunit test in lib/overflow_kunit.c:1188 fails when combining LLVM=1 > > and ARCH=arm. > > > > I have reproduced this on v6.10-rc1 and next-20240606. > > > > Here is the clang/llvm version I'm using: > > clang version 18.1.6 (Fedora 18.1.6-3.fc40) > > Target: x86_64-redhat-linux-gnu > > Thread model: posix > > InstalledDir: /usr/bin > > Configuration file: /etc/clang/x86_64-redhat-linux-gnu-clang.cfg > > > > I have not looked closer at the failure so I'm unsure if this is a > > problem with LLVM or if the test case is to speciffic. > > Thanks a lot for the report! I can reproduce this with tip of tree LLVM > as well. > > $ echo 'CONFIG_KUNIT=y > CONFIG_OVERFLOW_KUNIT_TEST=y > CONFIG_RUNTIME_KERNEL_TESTING_MENU=y' >kernel/configs/repro.config > > $ make -skj"$(nproc)" ARCH=arm LLVM=1 {def,repro.}config zImage > > $ boot-qemu.py -a arm -k . > ... > [ 0.000000] Linux version 6.10.0-rc2-00235-g8a92980606e3 (nathan@thelio-3990X) (ClangBuiltLinux clang version 19.0.0git (https://github.com/llvm/llvm-project e635520be888335dd59874038d33e60cca3a7143), ClangBuiltLinux LLD 19.0.0) #1 SMP Fri Jun 7 06:12:02 MST 2024 > ... > [ 1.832472] # castable_to_type_test: 75 castable_to_type() tests finished > [ 1.833483] ok 21 castable_to_type_test > [ 1.834122] # DEFINE_FLEX_test: EXPECTATION FAILED at lib/overflow_kunit.c:1188 > [ 1.834122] Expected __builtin_dynamic_object_size(two, 0) == sizeof(struct foo) + sizeof(s16) + sizeof(s16), but > [ 1.834122] __builtin_dynamic_object_size(two, 0) == 8 (0x8) > [ 1.834122] sizeof(struct foo) + sizeof(s16) + sizeof(s16) == 12 (0xc) > [ 1.834746] not ok 22 DEFINE_FLEX_test > ... > > I don't see the same failure with GCC 13.2.0. This test fails when > building for arm64 and x86_64 as well, so it does not appear to be > architecture specific. > > I think I see what is going on here. Looking at the documentation for > DEFINE_RAW_FLEX(), it states "Define an on-stack instance of structure > with a trailing flexible array member, when it does not have a > __counted_by annotation." but commit d8e45f2929b9 ("overflow: Change > DEFINE_FLEX to take __counted_by member") defined 'struct foo' with > __counted_by on it. __counted_by informs __builtin_dynamic_object_size() > about the size of the flexible array. With DEFINE_FLEX_RAW, the counter > is zero, so the size of array in 'struct foo' is zero, meaning this test > is incorrect when built with a compiler that supports __counted_by, > which is just Clang 18+ right now (it should land in GCC 15 if I > understand correctly). > > I see two potential solutions that work for me. > > One would be to stop using DEFINE_RAW_FLEX() and match the other uses > (but I assume testing DEFINE_RAW_FLEX() was intentional): > > diff --git a/lib/overflow_kunit.c b/lib/overflow_kunit.c > index 4ef31b0bb74d..883670adf0cc 100644 > --- a/lib/overflow_kunit.c > +++ b/lib/overflow_kunit.c > @@ -1180,7 +1180,7 @@ struct foo { > > static void DEFINE_FLEX_test(struct kunit *test) > { > - DEFINE_RAW_FLEX(struct foo, two, array, 2); > + DEFINE_FLEX(struct foo, two, array, counter, 2); > DEFINE_FLEX(struct foo, eight, array, counter, 8); > DEFINE_FLEX(struct foo, empty, array, counter, 0); > > > The other would be making the size of the array conditional on not > having __counted_by support (which is admittedly ugly): > > diff --git a/lib/overflow_kunit.c b/lib/overflow_kunit.c > index 4ef31b0bb74d..7eed0890e25f 100644 > --- a/lib/overflow_kunit.c > +++ b/lib/overflow_kunit.c > @@ -1185,7 +1185,11 @@ static void DEFINE_FLEX_test(struct kunit *test) > DEFINE_FLEX(struct foo, empty, array, counter, 0); > > KUNIT_EXPECT_EQ(test, __struct_size(two), > - sizeof(struct foo) + sizeof(s16) + sizeof(s16)); > + sizeof(struct foo) > +#if !__has_attribute(__counted_by__) > + + sizeof(s16) + sizeof(s16) > +#endif > + ); > KUNIT_EXPECT_EQ(test, __struct_size(eight), 24); > KUNIT_EXPECT_EQ(test, __struct_size(empty), sizeof(struct foo)); > } > > Kees, am I missing anything here? Thanks for analyzing this! I've sent a patch for this now. It's similar to what you've suggested here, but I wanted to break out the non-counted_by usage as well, which is how DEFINE_RAW_FLEX() is supposed to be used, but it's good to capture the expected behavior of RAW with counted_by too. -- Kees Cook ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-06-10 18:24 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-06-07 12:37 [Bug] Failing kunit test on ARCH=arm and LLVM=1 Christian Schrefl 2024-06-07 14:33 ` Nathan Chancellor 2024-06-10 18:24 ` Kees Cook
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).