Re: [linux-next:master 7012/7430] include/linux/compiler_types.h:328:38: error: call to '__compiletime_assert_183' declared with attribute error: unexpected size in kmalloc_index()

[Vlastimil Babka <vbabka@suse.cz>]

On 6/6/21 1:08 PM, Hyeonggon Yoo wrote:
> On Sat, Jun 05, 2021 at 02:10:46PM +0800, kernel test robot wrote:
>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
>> head:   ccc252d2e818f6a479441119ad453c3ce7c7c461
>> commit: a7ba988ff9de37f0961b4bf96d17aca73d0d2e25 [7012/7430] mm, slub: change run-time assertion in kmalloc_index() to compile-time
>> config: parisc-randconfig-r014-20210604 (attached as .config)
>> compiler: hppa-linux-gcc (GCC) 9.3.0
>> reproduce (this is a W=1 build):
>>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>         chmod +x ~/bin/make.cross
>>         # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=a7ba988ff9de37f0961b4bf96d17aca73d0d2e25
>>         git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
>>         git fetch --no-tags linux-next master
>>         git checkout a7ba988ff9de37f0961b4bf96d17aca73d0d2e25
>>         # save the attached .config to linux build tree
>>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc 
>> 
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <lkp@intel.com>
>> 
>> All errors (new ones prefixed by >>):
>>    In file included from <command-line>:
>>    In function 'kmalloc_index',
>>        inlined from 'kmalloc_node' at include/linux/slab.h:572:20,
>>        inlined from 'bpf_map_kmalloc_node.isra.0.part.0' at include/linux/bpf.h:1319:9:
>> >> include/linux/compiler_types.h:328:38: error: call to '__compiletime_assert_183' declared with attribute error: unexpected size in kmalloc_index()
>>      328 |  _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
>>          |                                      ^
>>    include/linux/compiler_types.h:309:4: note: in definition of macro '__compiletime_assert'
>>      309 |    prefix ## suffix();    \
>>          |    ^~~~~~
>>    include/linux/compiler_types.h:328:2: note: in expansion of macro '_compiletime_assert'
>>      328 |  _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
>>          |  ^~~~~~~~~~~~~~~~~~~
>>    include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
>>       39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
>>          |                                     ^~~~~~~~~~~~~~~~~~
>>    include/linux/slab.h:389:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
>>      389 |  BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()");
>>          |  ^~~~~~~~~~~~~~~~
> 
> Reproduce with attached config, and read the code.
> It has no problem in clang (clang-10/clang-11). it is problem with gcc.

But what exactly is the gcc problem here?
Did you have to reproduce it with specific gcc version and/or architecture?

> I found two ways to solve the error (maybe there would be better
> solution)  Any thoughts or opinions?
> 
> 
> First ways is to change condition of kmalloc_index macro.
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 70e46db766ca..be2c900cba4b 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -397,7 +397,7 @@ static __always_inline unsigned int __kmalloc_index(size_t size,
>         /* Will never be reached. Needed because the compiler may complain */
>         return -1;
>  }
> -#define kmalloc_index(s) __kmalloc_index(s, true)
> +#define kmalloc_index(s) __kmalloc_index(s, __builtin_constant_p(s) && true)

I wonder how this extra guard can possibly matter?

bpf_map_kmalloc_node()
  kmalloc_node()
     if (__builtin_constant_p(size) ...)
        unsigned int i = kmalloc_index(size);

We shouldn't be even reaching kmalloc_index() unless __builtin_constant_p(size)
is already true.

>  #endif /* !CONFIG_SLOB */
>  
>  void *__kmalloc(size_t size, gfp_t flags) __assume_kmalloc_alignment __malloc;
> 
> 
> Second way is making bpf_map_kmalloc_node always inline.

If bpf_map_kmalloc_node() is not being compiled as inline, how can it possibly
evaluate __builtin_constant_p(size) as true when processing the inlined
kmalloc_node()?

> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 02b02cb29ce2..09379d705349 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1312,7 +1312,7 @@ void *bpf_map_kzalloc(const struct bpf_map *map, size_t size, gfp_t flags);
>  void __percpu *bpf_map_alloc_percpu(const struct bpf_map *map, size_t size,
>                                     size_t align, gfp_t flags);
>  #else
> -static inline void *
> +static __always_inline void *
>  bpf_map_kmalloc_node(const struct bpf_map *map, size_t size, gfp_t flags,
>                      int node)
>  {
>