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()

[Hyeonggon Yoo <42.hyeyoo@gmail.com>]

On Mon, Jun 07, 2021 at 01:40:02PM +0200, Vlastimil Babka wrote:
> 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?
> 

Before replying, I should say that I'm not an expert on gcc.
I just tried some ways to fix the error, and it seemed to me that
gcc is doing something wrong.

I found the error was in kernel/bpf/local_storage.c
and I copied the bot's config in linux-next (20210607), and just entered all new config.
running 'make kernel/bpf/local_storage.o CC=gcc' can reproduce the
error.

the bot says it is error with parisc, but I was able to reproduce
it in my x86 machine.

I tested on gcc-9.3.0 and gcc-10.2.0 both makes an error,
and clang 10.0.1, clang 11.0.0 didn't make an error.

> > 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.

Yes, I knew that when I wrote the code. That totally doesn't make sense.
why __builtin_constant_p(size) was true in kmalloc_node,
and false in the evalaution (__builtin_constant_p(s) && true)?

but it actually solves the error.
At this point, I thought gcc was doing something wrong....

Well, I know we need more evidence to conclude gcc is wrong.
(Or we made wrong code that makes compiler confusing.)

I want to hear you and some other people's opinion.

> >  #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()?

As I said above - it doesn't make sense, but gcc is acting differently
on __inline and inline, in bpf_map_kmalloc_node function. just making
bpf_map_kmalloc_node always solves the error, but I have no clue WHY...

anyway, in summary:
- the diff I sent should not make sense, but it works for gcc.
- So I think gcc is doing something wrong (but more evidence needed)

I can be missing something. So If I said something wrong, or if you
can't reproduce the error, please tell me!

Thanks,
Hyeonggon Yoo

> > 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)
> >  {
> > 
>