On Sat, May 15, 2021 at 11:24:25PM +0200, Vlastimil Babka wrote:

On 5/15/21 11:09 PM, Hyeonggon Yoo wrote:

quoted

Hello Vlastimil, recently kbuild-all test bot reported compile error on
clang 10.0.1, with defconfig.

Hm yes, catching some compiler bug was something that was noted to be
possible to happen.

quoted

Nathan Chancellor wrote:

quoted

I think this happens because arch_prepare_optimized_kprobe() calls kzalloc()
with a size of MAX_OPTINSN_SIZE, which is

#define MAX_OPTINSN_SIZE                                \
      (((unsigned long)optprobe_template_end -        \
         (unsigned long)optprobe_template_entry) +     \
        MAX_OPTIMIZED_LENGTH + JMP32_INSN_SIZE)

quoted

and the optprobe_template_{end,entry} are not evaluated as constants.

I am not sure what the solution is. There seem to be a growing list of issues
with LLVM 10 that were fixed in LLVM 11, which might necessitate requiring
LLVM 11 and newer to build the kernel, given this affects a defconfig.
Cheers,
Nathan

I think it's because kmalloc compiles successfully when size is constant,
and kmalloc_index isn't. so I think compiler seems to be confused.

currently if size is non-constant, kmalloc calls dummy function __kmalloc,
which always returns NULL.

That's a misunderstanding. __kmalloc() is not a dummy function, you
probably found only the header declaration.

quoted

so what about changing kmalloc to do compile-time assertion too, and track
all callers that are calling kmalloc with non-constant argument.

kmalloc() is expected to be called with both constant and non-constant
size. __builtin_constant_p() is used to determine which implementation
to use. One based on kmalloc_index(), other on __kmalloc().

It appears clang 10.0.1 is mistakenly evaluating __builtin_constant_p()
as true. Probably something to do with LTO, because MAX_OPTINSN_SIZE
seems it could be a "link-time constant".

This happens with x86_64 defconfig so LTO is not involved.

However, the explanation makes sense, given that the LLVM change I
landed on changes the sparse conditional constant propagation pass,
which I believe can influence how LLVM handles __builtin_constant_p().

Maybe we could extend Marco Elver's followup patch that uses
BUILD_BUG_ON vs BUG() depending on size_is_constant parameter. It could
use BUG() also if the compiler is LLVM < 11 or something. What would be
the proper code for this condition?

This should work I think:

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 9d316aac0aba..1b653266f2aa 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h

@@ -413,7 +413,7 @@ static __always_inline unsigned int __kmalloc_index(size_t size,
 	if (size <=  16 * 1024 * 1024) return 24;
 	if (size <=  32 * 1024 * 1024) return 25;
 
-	if (size_is_constant)
+	if ((IS_ENABLED(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION > 110000) && size_is_constant)
 		BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()");
 	else
 		BUG();