Thread (29 messages) 29 messages, 5 authors, 2021-05-19

Re: [PATCH v3] mm, slub: change run-time assertion in kmalloc_index() to compile-time

From: Vlastimil Babka <hidden>
Date: 2021-05-15 21:25:18
Also in: linux-next, lkml

On 5/15/21 11:09 PM, Hyeonggon Yoo wrote:
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.
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.
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".

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?
How do you think? If you think it is the solution, I'll do that work.
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help