__mulodi4 from u32_u32__int_overflow_test in lib/overflow_kunit.c

[nathanchance]

Reported by CI with LLVM 11 (newer versions do not show this):

https://builds.tuxbuild.com/2EqpS04tCKn154FmyeYYpl7WJJ2/build.log

$ make -skj"$(nproc)" ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- KCONFIG_ALLCONFIG=<(echo CONFIG_WERROR=n) LLVM=1 LLVM_IAS=0 allmodconfig all
...
ERROR: modpost: "__mulodi4" [lib/overflow_kunit.ko] undefined!
...

"Caused" by commit d219d2a9a92e ("overflow: Allow mixed type arguments") (cc @kees). A simplified reproducer on top of allnoconfig:

$ make -skj"$(nproc)" ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- LLVM=1 LLVM_IAS=0 allnoconfig

$ scripts/config -e KUNIT -e RUNTIME_TESTING_MENU -e OVERFLOW_KUNIT_TEST

$ make -skj"$(nproc)" ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- LLVM=1 LLVM_IAS=0 olddefconfig all
ld.lld: error: undefined symbol: __mulodi4
>>> referenced by overflow_kunit.c
>>>               lib/overflow_kunit.o:(u32_u32__int_overflow_test) in archive vmlinux.a
>>> referenced by overflow_kunit.c
>>>               lib/overflow_kunit.o:(u32_u32__int_overflow_test) in archive vmlinux.a

My reverse bisect of LLVM shows commit 3203143f1356 ("CodeGen: Improve generated IR for __builtin_mul_overflow(uint, uint, int)") resolved this, which appears to be this exact case. I wonder if it is worth bumping the minimum version of LLVM to 12.0.0 over this?

[nickdesaulniers]

Can we simply omit the test for clang < 12?

[nathanchance]

Sure, that would fix this immediate issue. However, it is still going to be possible for someone to call these overflow macros in a driver or core kernel code with the same types as the test (that was the original motivation for the change), which would cause this error at that point.

[nickdesaulniers]

Maybe something like:

diff --git a/lib/overflow_kunit.c b/lib/overflow_kunit.c
index f385ca652b74..9240533e0283 100644
--- a/lib/overflow_kunit.c
+++ b/lib/overflow_kunit.c
@@ -716,12 +716,22 @@ static struct kunit_case overflow_test_cases[] = {
        KUNIT_CASE(u32_u32__u32_overflow_test),
        KUNIT_CASE(s32_s32__s32_overflow_test),
 /* Clang 13 and earlier generate unwanted libcalls on 32-bit. */
-#if BITS_PER_LONG == 64
+#if defined(CONFIG_CC_IS_GCC) || \
+  (defined(CONFIG_CC_IS_CLANG) && \
+    (__clang_major__ > 13 || BITS_PER_LONG == 64))
        KUNIT_CASE(u64_u64__u64_overflow_test),
        KUNIT_CASE(s64_s64__s64_overflow_test),
 #endif
-       KUNIT_CASE(u32_u32__u8_overflow_test),
+/* Clang 11 and earlier generate unwanted libcalls for signed output, unsigned
+ * input.
+ * https://github.com/llvm/llvm-project/commit/3203143f1356a4e4e3ada231156fc6da6e1a9f9d
+ * */
+#if defined(CONFIG_CC_IS_GCC) || \
+  (defined(CONFIG_CC_IS_CLANG) && \
+    (__clang_major__ > 11 || BITS_PER_LONG == 64))
        KUNIT_CASE(u32_u32__int_overflow_test),
+#endif
+       KUNIT_CASE(u32_u32__u8_overflow_test),
        KUNIT_CASE(u8_u8__int_overflow_test),
        KUNIT_CASE(int_int__u8_overflow_test),
        KUNIT_CASE(shift_sane_test),

[nathanchance]

This is now in mainline:

https://github.com/ClangBuiltLinux/continuous-integration2/actions/runs/3179910972

I will try to test the above diff.

[kees]

Ah! Shall I take the above patch for it?

[nickdesaulniers]

I'll send a formal patch today.

[nickdesaulniers]

v2: https://lore.kernel.org/llvm/20221006171751.3444575-1-ndesaulniers@google.com/

[nickdesaulniers]

Accepted: https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?id=2d4df670c64c

And @kees has a follow up: https://lore.kernel.org/llvm/20221006230017.1833458-1-keescook@chromium.org/

[nathanchance]

Are those changes scheduled for 6.1 or 6.2? CI is still failing with this on mainline so it is needed there. I can just apply it to the relevant branches in CI for the time being if necessary.

[kees]

Hi! I originally wanted to get them out of the door for -next, and if they were happy, get them sent to Linus for 6.1 shortly.

[nathanchance]

Thanks Kees!

https://git.kernel.org/linus/0e5b9f25b27a7a92880f88f5dba3edf726ec5f61