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

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

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4208 bytes --]

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()");
         |  ^~~~~~~~~~~~~~~~


vim +/__compiletime_assert_183 +328 include/linux/compiler_types.h

eb5c2d4b45e3d2 Will Deacon 2020-07-21  314  
eb5c2d4b45e3d2 Will Deacon 2020-07-21  315  #define _compiletime_assert(condition, msg, prefix, suffix) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21  316  	__compiletime_assert(condition, msg, prefix, suffix)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  317  
eb5c2d4b45e3d2 Will Deacon 2020-07-21  318  /**
eb5c2d4b45e3d2 Will Deacon 2020-07-21  319   * compiletime_assert - break build and emit msg if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  320   * @condition: a compile-time constant condition to check
eb5c2d4b45e3d2 Will Deacon 2020-07-21  321   * @msg:       a message to emit if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  322   *
eb5c2d4b45e3d2 Will Deacon 2020-07-21  323   * In tradition of POSIX assert, this macro will break the build if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  324   * supplied condition is *false*, emitting the supplied error message if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  325   * compiler has support to do so.
eb5c2d4b45e3d2 Will Deacon 2020-07-21  326   */
eb5c2d4b45e3d2 Will Deacon 2020-07-21  327  #define compiletime_assert(condition, msg) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 @328  	_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  329  

:::::: The code at line 328 was first introduced by commit
:::::: eb5c2d4b45e3d2d5d052ea6b8f1463976b1020d5 compiler.h: Move compiletime_assert() macros into compiler_types.h

:::::: TO: Will Deacon <will@kernel.org>
:::::: CC: Will Deacon <will@kernel.org>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 21363 bytes --]

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

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4279 bytes --]

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()");
         |  ^~~~~~~~~~~~~~~~


vim +/__compiletime_assert_183 +328 include/linux/compiler_types.h

eb5c2d4b45e3d2 Will Deacon 2020-07-21  314  
eb5c2d4b45e3d2 Will Deacon 2020-07-21  315  #define _compiletime_assert(condition, msg, prefix, suffix) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21  316  	__compiletime_assert(condition, msg, prefix, suffix)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  317  
eb5c2d4b45e3d2 Will Deacon 2020-07-21  318  /**
eb5c2d4b45e3d2 Will Deacon 2020-07-21  319   * compiletime_assert - break build and emit msg if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  320   * @condition: a compile-time constant condition to check
eb5c2d4b45e3d2 Will Deacon 2020-07-21  321   * @msg:       a message to emit if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  322   *
eb5c2d4b45e3d2 Will Deacon 2020-07-21  323   * In tradition of POSIX assert, this macro will break the build if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  324   * supplied condition is *false*, emitting the supplied error message if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  325   * compiler has support to do so.
eb5c2d4b45e3d2 Will Deacon 2020-07-21  326   */
eb5c2d4b45e3d2 Will Deacon 2020-07-21  327  #define compiletime_assert(condition, msg) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 @328  	_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  329  

:::::: The code at line 328 was first introduced by commit
:::::: eb5c2d4b45e3d2d5d052ea6b8f1463976b1020d5 compiler.h: Move compiletime_assert() macros into compiler_types.h

:::::: TO: Will Deacon <will@kernel.org>
:::::: CC: Will Deacon <will@kernel.org>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 21363 bytes --]

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]

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.

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

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

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]

[-- Attachment #1: Type: text/plain, Size: 4145 bytes --]

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.

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

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

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]

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

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]

[-- Attachment #1: Type: text/plain, Size: 4890 bytes --]

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

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]

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

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]

[-- Attachment #1: Type: text/plain, Size: 6729 bytes --]

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

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]

On 6/7/21 2:25 PM, Hyeonggon Yoo wrote:
> 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'm involving my gcc colleagues, will report results...

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]

[-- Attachment #1: Type: text/plain, Size: 3443 bytes --]

On 6/7/21 2:25 PM, Hyeonggon Yoo wrote:
> 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'm involving my gcc colleagues, will report results...

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]

On Mon, Jun 07, 2021 at 05:27:27PM +0200, Vlastimil Babka wrote:
> On 6/7/21 2:25 PM, Hyeonggon Yoo wrote:
> > 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'm involving my gcc colleagues, will report results...

Thank you so much!

Hyeonggon

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]

[-- Attachment #1: Type: text/plain, Size: 3656 bytes --]

On Mon, Jun 07, 2021 at 05:27:27PM +0200, Vlastimil Babka wrote:
> On 6/7/21 2:25 PM, Hyeonggon Yoo wrote:
> > 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'm involving my gcc colleagues, will report results...

Thank you so much!

Hyeonggon

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]

On 6/7/21 5:49 PM, Hyeonggon Yoo wrote:
> On Mon, Jun 07, 2021 at 05:27:27PM +0200, Vlastimil Babka wrote:
>> On 6/7/21 2:25 PM, Hyeonggon Yoo wrote:
>> > 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:
>> >> 
>> >> 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'm involving my gcc colleagues, will report results...

Well, it seems the bot's .config included CONFIG_PROFILE_ALL_BRANCHES as the
main factor to trigger the problem. And (according to my colleagues) it might
add too many instrumented if conditions to sustain the builtin-constant
tracking, which is not unlimited, or interact with optimizations in some other
corner case way.

I guess we could add IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES) to the list of
conditions that excludes using BUILD_BUG_ON_MSG().

> Thank you so much!
> 
> Hyeonggon
> 

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]

[-- Attachment #1: Type: text/plain, Size: 1256 bytes --]

On 6/7/21 5:49 PM, Hyeonggon Yoo wrote:
> On Mon, Jun 07, 2021 at 05:27:27PM +0200, Vlastimil Babka wrote:
>> On 6/7/21 2:25 PM, Hyeonggon Yoo wrote:
>> > 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:
>> >> 
>> >> 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'm involving my gcc colleagues, will report results...

Well, it seems the bot's .config included CONFIG_PROFILE_ALL_BRANCHES as the
main factor to trigger the problem. And (according to my colleagues) it might
add too many instrumented if conditions to sustain the builtin-constant
tracking, which is not unlimited, or interact with optimizations in some other
corner case way.

I guess we could add IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES) to the list of
conditions that excludes using BUILD_BUG_ON_MSG().

> Thank you so much!
> 
> Hyeonggon
> 

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]

On Tue, Jun 08, 2021 at 09:57:18AM +0200, Vlastimil Babka wrote:
> On 6/7/21 5:49 PM, Hyeonggon Yoo wrote:
> > On Mon, Jun 07, 2021 at 05:27:27PM +0200, Vlastimil Babka wrote:
> >> On 6/7/21 2:25 PM, Hyeonggon Yoo wrote:
> >> > 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:
> >> >> 
> >> >> 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'm involving my gcc colleagues, will report results...
> 
> Well, it seems the bot's .config included CONFIG_PROFILE_ALL_BRANCHES as the
> main factor to trigger the problem. And (according to my colleagues)

Wow, AWESOME! How did your colleague find it? that was a big hint for me.
when CONFIG_PROFILE_ALL_BRANCHES is not set, it doesn't make an error.

> it might add too many instrumented if conditions to sustain the builtin-constant
> tracking, which is not unlimited, or interact with optimizations in some other
> corner case way.

> I guess we could add IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES) to the list of
> conditions that excludes using BUILD_BUG_ON_MSG().

Well I wanted to understand how CONFIG_PROFILE_ALL_BRANCHES excludes
BUILD_BUG_ON This is gcc's preprocessor output.

So let's start from what CONFIG_PROFILE_ALL_BRANCHES does.

#ifdef CONFIG_PROFILE_ALL_BRANCHES
/*
 * "Define 'is'", Bill Clinton
 * "Define 'if'", Steven Rostedt
 */
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )

#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))

#define __trace_if_value(cond) ({               \
      static struct ftrace_branch_data          \
            __aligned(4)                        \
            __section("_ftrace_branch")         \
            __if_trace = {                      \
                  .func = __func__,       \
                  .file = __FILE__,       \
                  .line = __LINE__,       \
            };                            \
      (cond) ?                            \
            (__if_trace.miss_hit[1]++,1) :            \
            (__if_trace.miss_hit[0]++,0);       \
})

#endif /* CONFIG_PROFILE_ALL_BRANCHES */

it seems it records non-constant condition's hit or miss.
if cond is constant, do nothing. else, records its hit or miss at
runtime.

then let's look at kmalloc_node, which is called by bpf_map_kmalloc_node.

static inline __attribute__((__gnu_inline__)) __attribute__((__unused__)) __attribute__((no_instrument_function)) __attribute__((__always_inline__)) 
void *kmalloc_node(size_t size, gfp_t flags, int node){

 if ( (__builtin_constant_p(
       !!(__builtin_constant_p(size)
       && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))))
            ? (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))))
            : ({ 
			static struct ftrace_branch_data __attribute__ ((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) 
			__if_trace = 
			{	.func = __func__,
				.file = "include/linux/slab.h",
				.line = 601,
			};
			(!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))) 
				? (__if_trace.miss_hit[1]++,1) 
				: (__if_trace.miss_hit[0]++,0); })) )
                                  {
	unsigned int i = __kmalloc_index(size, true);

It seems that gcc evaluates

__builtin_constant_p(
				!!(builtin_constant_p(size)
				&& size <= KMALLOC_MAX_CACHE_SIZE)
			)
	as true.

mm.. when the size passed to bpf_map_kmalloc_node is not constant,
(__builtin_constant_p(size) && size <= KMALLOC_MAX_CACHE_SIZE) is false.
then __builtin_constant_p(!!false) is true. So it calls kmalloc_index.

what change should be made?
just checking CONFIG_PROFILE_ALL_BRANCHES to kmalloc_index's if condition
doesn't make sense, because kmalloc_node is not working as expected.

Plus, BUILD_BUG_ON_MSG seems not working with clang.
Below is generated by clang 11.0.0 preprocessor, when compiling
mm/kfence/kfence_test.o

Well, I'm going to read more code on BUILD_BUG_XXX family,
but if it doens't work on clang, we should change condition that we
made.

if ( (__builtin_constant_p(!!((0 || 110000 >= 110000) && size_is_constant)) ? (!!((0 || 110000 >= 110000) && size_is_constant)) : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 416, }; (!!((0 || 110000 >= 110000) && size_is_constant)) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) )
  do {


        extern void __compiletime_assert_131(void) ;
	  // here - compiletime_assert_131 does NOTHING

		if ( (__builtin_constant_p(!!(!(!(1))))
              ? (!!(!(!(1))))
              : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 417, }; (!!(!(!(1)))) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) __compiletime_assert_131(); } while (0);
 else
  do { do { } while(0); do { asm __inline volatile("1:\t" ".byte 0x0f, 0x0b" "\n" ".pushsection __bug_table,\"aw\"\n" "2:\t" ".long " "1b" " - 2b" "\t# bug_entry::bug_addr\n" "\t.word %c0" "\t# bug_entry::flags\n" "\t.org 2b+%c1\n" ".popsection" : : "i" (0), "i" (sizeof(struct bug_entry))); } while (0); do { ({ asm volatile("132" ":\n\t" ".pushsection .discard.unreachable\n\t" ".long " "132" "b - .\n\t" ".popsection\n\t"); }); __builtin_unreachable(); } while (0); } while (0);


 return -1;
}

Maybe it's because of definition of __compiletime_assert.

#ifdef __OPTIMIZE__
# define __compiletime_assert(condition, msg, prefix, suffix)           \
      do {                                            \
            extern void prefix ## suffix(void) __compiletime_error(msg); \
            if (!(condition))                         \
                  prefix ## suffix();                       \
      } while (0)
#else
# define __compiletime_assert(condition, msg, prefix, suffix) do { } while (0)
#endif


There can be an logical error or misunderstanding on my words.
If so, please tell me!

Thanks,
Hyeonggon

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]

[-- Attachment #1: Type: text/plain, Size: 6704 bytes --]

On Tue, Jun 08, 2021 at 09:57:18AM +0200, Vlastimil Babka wrote:
> On 6/7/21 5:49 PM, Hyeonggon Yoo wrote:
> > On Mon, Jun 07, 2021 at 05:27:27PM +0200, Vlastimil Babka wrote:
> >> On 6/7/21 2:25 PM, Hyeonggon Yoo wrote:
> >> > 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:
> >> >> 
> >> >> 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'm involving my gcc colleagues, will report results...
> 
> Well, it seems the bot's .config included CONFIG_PROFILE_ALL_BRANCHES as the
> main factor to trigger the problem. And (according to my colleagues)

Wow, AWESOME! How did your colleague find it? that was a big hint for me.
when CONFIG_PROFILE_ALL_BRANCHES is not set, it doesn't make an error.

> it might add too many instrumented if conditions to sustain the builtin-constant
> tracking, which is not unlimited, or interact with optimizations in some other
> corner case way.

> I guess we could add IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES) to the list of
> conditions that excludes using BUILD_BUG_ON_MSG().

Well I wanted to understand how CONFIG_PROFILE_ALL_BRANCHES excludes
BUILD_BUG_ON This is gcc's preprocessor output.

So let's start from what CONFIG_PROFILE_ALL_BRANCHES does.

#ifdef CONFIG_PROFILE_ALL_BRANCHES
/*
 * "Define 'is'", Bill Clinton
 * "Define 'if'", Steven Rostedt
 */
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )

#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))

#define __trace_if_value(cond) ({               \
      static struct ftrace_branch_data          \
            __aligned(4)                        \
            __section("_ftrace_branch")         \
            __if_trace = {                      \
                  .func = __func__,       \
                  .file = __FILE__,       \
                  .line = __LINE__,       \
            };                            \
      (cond) ?                            \
            (__if_trace.miss_hit[1]++,1) :            \
            (__if_trace.miss_hit[0]++,0);       \
})

#endif /* CONFIG_PROFILE_ALL_BRANCHES */

it seems it records non-constant condition's hit or miss.
if cond is constant, do nothing. else, records its hit or miss at
runtime.

then let's look at kmalloc_node, which is called by bpf_map_kmalloc_node.

static inline __attribute__((__gnu_inline__)) __attribute__((__unused__)) __attribute__((no_instrument_function)) __attribute__((__always_inline__)) 
void *kmalloc_node(size_t size, gfp_t flags, int node){

 if ( (__builtin_constant_p(
       !!(__builtin_constant_p(size)
       && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))))
            ? (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))))
            : ({ 
			static struct ftrace_branch_data __attribute__ ((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) 
			__if_trace = 
			{	.func = __func__,
				.file = "include/linux/slab.h",
				.line = 601,
			};
			(!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))) 
				? (__if_trace.miss_hit[1]++,1) 
				: (__if_trace.miss_hit[0]++,0); })) )
                                  {
	unsigned int i = __kmalloc_index(size, true);

It seems that gcc evaluates

__builtin_constant_p(
				!!(builtin_constant_p(size)
				&& size <= KMALLOC_MAX_CACHE_SIZE)
			)
	as true.

mm.. when the size passed to bpf_map_kmalloc_node is not constant,
(__builtin_constant_p(size) && size <= KMALLOC_MAX_CACHE_SIZE) is false.
then __builtin_constant_p(!!false) is true. So it calls kmalloc_index.

what change should be made?
just checking CONFIG_PROFILE_ALL_BRANCHES to kmalloc_index's if condition
doesn't make sense, because kmalloc_node is not working as expected.

Plus, BUILD_BUG_ON_MSG seems not working with clang.
Below is generated by clang 11.0.0 preprocessor, when compiling
mm/kfence/kfence_test.o

Well, I'm going to read more code on BUILD_BUG_XXX family,
but if it doens't work on clang, we should change condition that we
made.

if ( (__builtin_constant_p(!!((0 || 110000 >= 110000) && size_is_constant)) ? (!!((0 || 110000 >= 110000) && size_is_constant)) : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 416, }; (!!((0 || 110000 >= 110000) && size_is_constant)) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) )
  do {


        extern void __compiletime_assert_131(void) ;
	  // here - compiletime_assert_131 does NOTHING

		if ( (__builtin_constant_p(!!(!(!(1))))
              ? (!!(!(!(1))))
              : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 417, }; (!!(!(!(1)))) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) __compiletime_assert_131(); } while (0);
 else
  do { do { } while(0); do { asm __inline volatile("1:\t" ".byte 0x0f, 0x0b" "\n" ".pushsection __bug_table,\"aw\"\n" "2:\t" ".long " "1b" " - 2b" "\t# bug_entry::bug_addr\n" "\t.word %c0" "\t# bug_entry::flags\n" "\t.org 2b+%c1\n" ".popsection" : : "i" (0), "i" (sizeof(struct bug_entry))); } while (0); do { ({ asm volatile("132" ":\n\t" ".pushsection .discard.unreachable\n\t" ".long " "132" "b - .\n\t" ".popsection\n\t"); }); __builtin_unreachable(); } while (0); } while (0);


 return -1;
}

Maybe it's because of definition of __compiletime_assert.

#ifdef __OPTIMIZE__
# define __compiletime_assert(condition, msg, prefix, suffix)           \
      do {                                            \
            extern void prefix ## suffix(void) __compiletime_error(msg); \
            if (!(condition))                         \
                  prefix ## suffix();                       \
      } while (0)
#else
# define __compiletime_assert(condition, msg, prefix, suffix) do { } while (0)
#endif


There can be an logical error or misunderstanding on my words.
If so, please tell me!

Thanks,
Hyeonggon

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]

On 6/8/21 7:05 PM, Hyeonggon Yoo wrote:
> On Tue, Jun 08, 2021 at 09:57:18AM +0200, Vlastimil Babka wrote:
>> On 6/7/21 5:49 PM, Hyeonggon Yoo wrote:
>> > On Mon, Jun 07, 2021 at 05:27:27PM +0200, Vlastimil Babka wrote:
>> >> On 6/7/21 2:25 PM, Hyeonggon Yoo wrote:
>> >> > 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:
>> >> >> 
>> >> >> 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'm involving my gcc colleagues, will report results...
>> 
>> Well, it seems the bot's .config included CONFIG_PROFILE_ALL_BRANCHES as the
>> main factor to trigger the problem. And (according to my colleagues)
> 
> Wow, AWESOME! How did your colleague find it? that was a big hint for me.
> when CONFIG_PROFILE_ALL_BRANCHES is not set, it doesn't make an error.

Well, we started with me doing "make kernel/bpf/local_storage.i" to create a
preprocessed C source that can be copied out and debugged outside of the whole
kernel build context. I send that to my colleague and he reduced the testcase
using cvise:

https://gcc.gnu.org/wiki/A_guide_to_testcase_reduction

While the reduction wasn't successful in preserving enough of the testcase, from
the result it was clear that there was lots of ftrace_branch_data stuff and so
it was easy to find this is due to
CONFIG_PROFILE_ALL_BRANCHES.

>> it might add too many instrumented if conditions to sustain the builtin-constant
>> tracking, which is not unlimited, or interact with optimizations in some other
>> corner case way.
> 
>> I guess we could add IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES) to the list of
>> conditions that excludes using BUILD_BUG_ON_MSG().
> 
> Well I wanted to understand how CONFIG_PROFILE_ALL_BRANCHES excludes
> BUILD_BUG_ON This is gcc's preprocessor output.
> 
> So let's start from what CONFIG_PROFILE_ALL_BRANCHES does.
> 
> #ifdef CONFIG_PROFILE_ALL_BRANCHES
> /*
>  * "Define 'is'", Bill Clinton
>  * "Define 'if'", Steven Rostedt
>  */
> #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
> 
> #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
> 
> #define __trace_if_value(cond) ({               \
>       static struct ftrace_branch_data          \
>             __aligned(4)                        \
>             __section("_ftrace_branch")         \
>             __if_trace = {                      \
>                   .func = __func__,       \
>                   .file = __FILE__,       \
>                   .line = __LINE__,       \
>             };                            \
>       (cond) ?                            \
>             (__if_trace.miss_hit[1]++,1) :            \
>             (__if_trace.miss_hit[0]++,0);       \
> })
> 
> #endif /* CONFIG_PROFILE_ALL_BRANCHES */
> 
> it seems it records non-constant condition's hit or miss.
> if cond is constant, do nothing. else, records its hit or miss at
> runtime.
> 
> then let's look at kmalloc_node, which is called by bpf_map_kmalloc_node.
> 
> static inline __attribute__((__gnu_inline__)) __attribute__((__unused__)) __attribute__((no_instrument_function)) __attribute__((__always_inline__)) 
> void *kmalloc_node(size_t size, gfp_t flags, int node){
> 
>  if ( (__builtin_constant_p(
>        !!(__builtin_constant_p(size)
>        && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))))
>             ? (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))))
>             : ({ 
> 			static struct ftrace_branch_data __attribute__ ((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) 
> 			__if_trace = 
> 			{	.func = __func__,
> 				.file = "include/linux/slab.h",
> 				.line = 601,
> 			};
> 			(!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))) 
> 				? (__if_trace.miss_hit[1]++,1) 
> 				: (__if_trace.miss_hit[0]++,0); })) )
>                                   {
> 	unsigned int i = __kmalloc_index(size, true);
> 
> It seems that gcc evaluates
> 
> __builtin_constant_p(
> 				!!(builtin_constant_p(size)
> 				&& size <= KMALLOC_MAX_CACHE_SIZE)
> 			)
> 	as true.
> 
> mm.. when the size passed to bpf_map_kmalloc_node is not constant,
> (__builtin_constant_p(size) && size <= KMALLOC_MAX_CACHE_SIZE) is false.
> then __builtin_constant_p(!!false) is true. So it calls kmalloc_index.
> 
> what change should be made?
> just checking CONFIG_PROFILE_ALL_BRANCHES to kmalloc_index's if condition
> doesn't make sense, because kmalloc_node is not working as expected.

If I understood my colleagues right, the problem is that, while kmalloc_index()
seems to contains a number of *independent* "if (size <= X) conditions in a
sequence, the machinery of CONFIG_PROFILE_ALL_BRANCHES turns it to a deeply
nested if-then-else { if-then-else { if-then-else {...}}} thing (in fact using
the ternary operators, not if-then-else). At some point of the deep nesting gcc
"forgets" that __builtin_constant_p() is true and starts behaving as if it wasn't.

Without CONFIG_PROFILE_ALL_BRANCHES it's able to keep track of it fine.

> Plus, BUILD_BUG_ON_MSG seems not working with clang.
> Below is generated by clang 11.0.0 preprocessor, when compiling
> mm/kfence/kfence_test.o
> 
> Well, I'm going to read more code on BUILD_BUG_XXX family,
> but if it doens't work on clang, we should change condition that we
> made.
> 
> if ( (__builtin_constant_p(!!((0 || 110000 >= 110000) && size_is_constant)) ? (!!((0 || 110000 >= 110000) && size_is_constant)) : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 416, }; (!!((0 || 110000 >= 110000) && size_is_constant)) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) )
>   do {
> 
> 
>         extern void __compiletime_assert_131(void) ;
> 	  // here - compiletime_assert_131 does NOTHING

It doesn't seem to do nothing? see below

> 		if ( (__builtin_constant_p(!!(!(!(1))))
>               ? (!!(!(!(1))))
>               : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 417, }; (!!(!(!(1)))) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) __compiletime_assert_131(); } while (0);

The thing above seems to be exactly the "if (!(condition))
prefix ## suffix();   } while (0)" as the definition you posted below.

Anyway, you can verify that clang works by commenting out the highest size
checks and passing a constant size that makes it reach the  BUILD_BUG_ON_MSG() ?

>  else
>   do { do { } while(0); do { asm __inline volatile("1:\t" ".byte 0x0f, 0x0b" "\n" ".pushsection __bug_table,\"aw\"\n" "2:\t" ".long " "1b" " - 2b" "\t# bug_entry::bug_addr\n" "\t.word %c0" "\t# bug_entry::flags\n" "\t.org 2b+%c1\n" ".popsection" : : "i" (0), "i" (sizeof(struct bug_entry))); } while (0); do { ({ asm volatile("132" ":\n\t" ".pushsection .discard.unreachable\n\t" ".long " "132" "b - .\n\t" ".popsection\n\t"); }); __builtin_unreachable(); } while (0); } while (0);
> 
> 
>  return -1;
> }
> 
> Maybe it's because of definition of __compiletime_assert.
> 
> #ifdef __OPTIMIZE__
> # define __compiletime_assert(condition, msg, prefix, suffix)           \
>       do {                                            \
>             extern void prefix ## suffix(void) __compiletime_error(msg); \
>             if (!(condition))                         \
>                   prefix ## suffix();                       \
>       } while (0)
> #else
> # define __compiletime_assert(condition, msg, prefix, suffix) do { } while (0)
> #endif
> 
> 
> There can be an logical error or misunderstanding on my words.
> If so, please tell me!
> 
> Thanks,
> Hyeonggon
> 

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]

[-- Attachment #1: Type: text/plain, Size: 8453 bytes --]

On 6/8/21 7:05 PM, Hyeonggon Yoo wrote:
> On Tue, Jun 08, 2021 at 09:57:18AM +0200, Vlastimil Babka wrote:
>> On 6/7/21 5:49 PM, Hyeonggon Yoo wrote:
>> > On Mon, Jun 07, 2021 at 05:27:27PM +0200, Vlastimil Babka wrote:
>> >> On 6/7/21 2:25 PM, Hyeonggon Yoo wrote:
>> >> > 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:
>> >> >> 
>> >> >> 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'm involving my gcc colleagues, will report results...
>> 
>> Well, it seems the bot's .config included CONFIG_PROFILE_ALL_BRANCHES as the
>> main factor to trigger the problem. And (according to my colleagues)
> 
> Wow, AWESOME! How did your colleague find it? that was a big hint for me.
> when CONFIG_PROFILE_ALL_BRANCHES is not set, it doesn't make an error.

Well, we started with me doing "make kernel/bpf/local_storage.i" to create a
preprocessed C source that can be copied out and debugged outside of the whole
kernel build context. I send that to my colleague and he reduced the testcase
using cvise:

https://gcc.gnu.org/wiki/A_guide_to_testcase_reduction

While the reduction wasn't successful in preserving enough of the testcase, from
the result it was clear that there was lots of ftrace_branch_data stuff and so
it was easy to find this is due to
CONFIG_PROFILE_ALL_BRANCHES.

>> it might add too many instrumented if conditions to sustain the builtin-constant
>> tracking, which is not unlimited, or interact with optimizations in some other
>> corner case way.
> 
>> I guess we could add IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES) to the list of
>> conditions that excludes using BUILD_BUG_ON_MSG().
> 
> Well I wanted to understand how CONFIG_PROFILE_ALL_BRANCHES excludes
> BUILD_BUG_ON This is gcc's preprocessor output.
> 
> So let's start from what CONFIG_PROFILE_ALL_BRANCHES does.
> 
> #ifdef CONFIG_PROFILE_ALL_BRANCHES
> /*
>  * "Define 'is'", Bill Clinton
>  * "Define 'if'", Steven Rostedt
>  */
> #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
> 
> #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
> 
> #define __trace_if_value(cond) ({               \
>       static struct ftrace_branch_data          \
>             __aligned(4)                        \
>             __section("_ftrace_branch")         \
>             __if_trace = {                      \
>                   .func = __func__,       \
>                   .file = __FILE__,       \
>                   .line = __LINE__,       \
>             };                            \
>       (cond) ?                            \
>             (__if_trace.miss_hit[1]++,1) :            \
>             (__if_trace.miss_hit[0]++,0);       \
> })
> 
> #endif /* CONFIG_PROFILE_ALL_BRANCHES */
> 
> it seems it records non-constant condition's hit or miss.
> if cond is constant, do nothing. else, records its hit or miss at
> runtime.
> 
> then let's look at kmalloc_node, which is called by bpf_map_kmalloc_node.
> 
> static inline __attribute__((__gnu_inline__)) __attribute__((__unused__)) __attribute__((no_instrument_function)) __attribute__((__always_inline__)) 
> void *kmalloc_node(size_t size, gfp_t flags, int node){
> 
>  if ( (__builtin_constant_p(
>        !!(__builtin_constant_p(size)
>        && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))))
>             ? (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))))
>             : ({ 
> 			static struct ftrace_branch_data __attribute__ ((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) 
> 			__if_trace = 
> 			{	.func = __func__,
> 				.file = "include/linux/slab.h",
> 				.line = 601,
> 			};
> 			(!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))) 
> 				? (__if_trace.miss_hit[1]++,1) 
> 				: (__if_trace.miss_hit[0]++,0); })) )
>                                   {
> 	unsigned int i = __kmalloc_index(size, true);
> 
> It seems that gcc evaluates
> 
> __builtin_constant_p(
> 				!!(builtin_constant_p(size)
> 				&& size <= KMALLOC_MAX_CACHE_SIZE)
> 			)
> 	as true.
> 
> mm.. when the size passed to bpf_map_kmalloc_node is not constant,
> (__builtin_constant_p(size) && size <= KMALLOC_MAX_CACHE_SIZE) is false.
> then __builtin_constant_p(!!false) is true. So it calls kmalloc_index.
> 
> what change should be made?
> just checking CONFIG_PROFILE_ALL_BRANCHES to kmalloc_index's if condition
> doesn't make sense, because kmalloc_node is not working as expected.

If I understood my colleagues right, the problem is that, while kmalloc_index()
seems to contains a number of *independent* "if (size <= X) conditions in a
sequence, the machinery of CONFIG_PROFILE_ALL_BRANCHES turns it to a deeply
nested if-then-else { if-then-else { if-then-else {...}}} thing (in fact using
the ternary operators, not if-then-else). At some point of the deep nesting gcc
"forgets" that __builtin_constant_p() is true and starts behaving as if it wasn't.

Without CONFIG_PROFILE_ALL_BRANCHES it's able to keep track of it fine.

> Plus, BUILD_BUG_ON_MSG seems not working with clang.
> Below is generated by clang 11.0.0 preprocessor, when compiling
> mm/kfence/kfence_test.o
> 
> Well, I'm going to read more code on BUILD_BUG_XXX family,
> but if it doens't work on clang, we should change condition that we
> made.
> 
> if ( (__builtin_constant_p(!!((0 || 110000 >= 110000) && size_is_constant)) ? (!!((0 || 110000 >= 110000) && size_is_constant)) : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 416, }; (!!((0 || 110000 >= 110000) && size_is_constant)) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) )
>   do {
> 
> 
>         extern void __compiletime_assert_131(void) ;
> 	  // here - compiletime_assert_131 does NOTHING

It doesn't seem to do nothing? see below

> 		if ( (__builtin_constant_p(!!(!(!(1))))
>               ? (!!(!(!(1))))
>               : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 417, }; (!!(!(!(1)))) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) __compiletime_assert_131(); } while (0);

The thing above seems to be exactly the "if (!(condition))
prefix ## suffix();   } while (0)" as the definition you posted below.

Anyway, you can verify that clang works by commenting out the highest size
checks and passing a constant size that makes it reach the  BUILD_BUG_ON_MSG() ?

>  else
>   do { do { } while(0); do { asm __inline volatile("1:\t" ".byte 0x0f, 0x0b" "\n" ".pushsection __bug_table,\"aw\"\n" "2:\t" ".long " "1b" " - 2b" "\t# bug_entry::bug_addr\n" "\t.word %c0" "\t# bug_entry::flags\n" "\t.org 2b+%c1\n" ".popsection" : : "i" (0), "i" (sizeof(struct bug_entry))); } while (0); do { ({ asm volatile("132" ":\n\t" ".pushsection .discard.unreachable\n\t" ".long " "132" "b - .\n\t" ".popsection\n\t"); }); __builtin_unreachable(); } while (0); } while (0);
> 
> 
>  return -1;
> }
> 
> Maybe it's because of definition of __compiletime_assert.
> 
> #ifdef __OPTIMIZE__
> # define __compiletime_assert(condition, msg, prefix, suffix)           \
>       do {                                            \
>             extern void prefix ## suffix(void) __compiletime_error(msg); \
>             if (!(condition))                         \
>                   prefix ## suffix();                       \
>       } while (0)
> #else
> # define __compiletime_assert(condition, msg, prefix, suffix) do { } while (0)
> #endif
> 
> 
> There can be an logical error or misunderstanding on my words.
> If so, please tell me!
> 
> Thanks,
> Hyeonggon
> 

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]

On Tue, Jun 08, 2021 at 07:27:52PM +0200, Vlastimil Babka wrote:
> On 6/8/21 7:05 PM, Hyeonggon Yoo wrote:
> > On Tue, Jun 08, 2021 at 09:57:18AM +0200, Vlastimil Babka wrote:
> >> On 6/7/21 5:49 PM, Hyeonggon Yoo wrote:
> >> > On Mon, Jun 07, 2021 at 05:27:27PM +0200, Vlastimil Babka wrote:
> >> >> On 6/7/21 2:25 PM, Hyeonggon Yoo wrote:
> >> >> > 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:
> >> >> >> 
> >> >> >> 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'm involving my gcc colleagues, will report results...
> >> 
> >> Well, it seems the bot's .config included CONFIG_PROFILE_ALL_BRANCHES as the
> >> main factor to trigger the problem. And (according to my colleagues)
> > 
> > Wow, AWESOME! How did your colleague find it? that was a big hint for me.
> > when CONFIG_PROFILE_ALL_BRANCHES is not set, it doesn't make an error.
> 
> Well, we started with me doing "make kernel/bpf/local_storage.i" to create a
> preprocessed C source that can be copied out and debugged outside of the whole
> kernel build context. I send that to my colleague and he reduced the testcase
> using cvise:
> 
> https://gcc.gnu.org/wiki/A_guide_to_testcase_reduction
> 
> While the reduction wasn't successful in preserving enough of the testcase, from
> the result it was clear that there was lots of ftrace_branch_data stuff and so
> it was easy to find this is due to
> CONFIG_PROFILE_ALL_BRANCHES.
> 

First time to hear about testcase reduction and cvise!
Thank you for letting me know it :)
I'm going to read it. but I ask some questions before that (see below)

> > mm.. when the size passed to bpf_map_kmalloc_node is not constant,
> > (__builtin_constant_p(size) && size <= KMALLOC_MAX_CACHE_SIZE) is false.
> > then __builtin_constant_p(!!false) is true. So it calls kmalloc_index.
> > 
> > what change should be made?
> > just checking CONFIG_PROFILE_ALL_BRANCHES to kmalloc_index's if condition
> > doesn't make sense, because kmalloc_node is not working as expected.
>
> If I understood my colleagues right, the problem is that, while kmalloc_index()
> seems to contains a number of *independent* "if (size <= X) conditions in a
> sequence, the machinery of CONFIG_PROFILE_ALL_BRANCHES turns it to a deeply
> nested if-then-else { if-then-else { if-then-else {...}}} thing (in fact using
> the ternary operators, not if-then-else).
> At some point of the deep nesting gcc
> "forgets" that __builtin_constant_p() is true and starts behaving as if it wasn't.
> 
> Without CONFIG_PROFILE_ALL_BRANCHES it's able to keep track of it fine.
>

I think you are talking about some if statements in kmalloc_index.

How do you know gcc "forgets" __builtin_constant_p() is true?
can it be debugged using cvise? (not offending, just because
I don't know about cvise yet).

Also, as I understand right, kmalloc_index doesn't have information
about the value of __builtin_constant_p(size). the caller of
kmalloc_index (kmalloc_node here) has that information.

If I understand right, what CONFIG_PROFILE_ALL_BRANCHES does is below.

replacing if(cond) { body } -> if (
						__builtin_constant_p(cond) ?
							then (cond)
						else (cond, record something)
					) { body }

I think it does not make deep nested statements.

the below is some part of preprocessor output.
CONFIG_PROFILE_ALL_BRANCHES makes some ugly ternary operators,
but there is no deep-nested if-then-else statements.

if ( (__builtin_constant_p(!!(size <= 8)) ? (!!(size <= 8)) : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 392, }; (!!(size <= 8)) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) return 3;

if ( (__builtin_constant_p(!!(size <= 16)) ? (!!(size <= 16)) : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 393, }; (!!(size <= 16)) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) return 4;

if ( (__builtin_constant_p(!!(size <= 32)) ? (!!(size <= 32)) : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 394, }; (!!(size <= 32)) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) return 5;

.... and some if statements ...

And I think, the problem is on kmalloc_node, not on kmalloc_index.
the original code of kmalloc_node is below:


static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
  {
  #ifndef CONFIG_SLOB
        if (__builtin_constant_p(size) &&
              size <= KMALLOC_MAX_CACHE_SIZE) {
              unsigned int i = kmalloc_index(size);

and which is changed to below: (by CONFIG_PROFILE_ALL_BRANCHES)

static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
{

	if ( (
		__builtin_constant_p(
		!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))
		)
			? (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))) 
			: ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 601, }; (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) )
                                  {
  unsigned int i = __kmalloc_index(size, true);


they are so ugly but the point is:

> > It seems that gcc evaluates
> > 
> > __builtin_constant_p(
> > 				!!(builtin_constant_p(size)
> > 				&& size <= KMALLOC_MAX_CACHE_SIZE)
> > 			)
> > 	as true.
> > 
> > mm.. when the size passed to bpf_map_kmalloc_node is not constant,
> > (__builtin_constant_p(size) && size <= KMALLOC_MAX_CACHE_SIZE) is false.
> > then __builtin_constant_p(!!false) is true. So it calls kmalloc_index.
> > 
> > what change should be made?
> > just checking CONFIG_PROFILE_ALL_BRANCHES to kmalloc_index's if condition
> > doesn't make sense, because kmalloc_node is not working as expected.

some evidence to add:

	there are 4 calls of bpf_map_kmalloc_node.
	if you comment out two calls of bpf_map_kmalloc_node with non-constant
	(in line 168, 508), it doesn't make an error. So I thought
	it was problem when non-constant size was passed.

	And if "kmalloc_index makes problem with non-constant size" is
	true, then it is caller's fault because it is not allowed (except
	in __kmalloc_index)

	kmalloc_node shouldn't call kmalloc_index if the size was not
	constant.

> > void *kmalloc_node(size_t size, gfp_t flags, int node){
> > 
> >  if ( (__builtin_constant_p(
> >        !!(__builtin_constant_p(size)
> >        && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))))
> >             ? (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))))


> > if ( (__builtin_constant_p(!!((0 || 110000 >= 110000) && size_is_constant)) ? (!!((0 || 110000 >= 110000) && size_is_constant)) : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 416, }; (!!((0 || 110000 >= 110000) && size_is_constant)) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) )
> >   do {
> > 
> > 
> >         extern void __compiletime_assert_131(void) ;
> > 	  // here - compiletime_assert_131 does NOTHING
> 
> It doesn't seem to do nothing? see below
> 
> > 		if ( (__builtin_constant_p(!!(!(!(1))))
> >               ? (!!(!(!(1))))
> >               : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 417, }; (!!(!(!(1)))) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) __compiletime_assert_131(); } while (0);
> 
> The thing above seems to be exactly the "if (!(condition))
> prefix ## suffix();   } while (0)" as the definition you posted below.
> 
> Anyway, you can verify that clang works by commenting out the highest size
> checks and passing a constant size that makes it reach the  BUILD_BUG_ON_MSG() ?
> 

I verified as below.
clang didn't make an error, gcc worked as expected.

then I can explain why there is no error with clang:
	it just did nothing with BUILD_BUG_ON.

hyeyoo@hyeyoo:~/바탕화면/linux-next$ git diff
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 8d8dd8571261..f2f9a6a7e663 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -379,6 +379,9 @@ static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
 static __always_inline unsigned int __kmalloc_index(size_t size,
                                                    bool size_is_constant)
 {
+
+       BUILD_BUG_ON(1);
+
        if (!size)
                return 0;

hyeyoo@hyeyoo:~/바탕화면/linux-next$ make mm/kfence/kfence_test.o CC=clang-11
  CALL    scripts/checksyscalls.sh

	... some headers omitted ...

  CC      mm/kfence/kfence_test.o
In file included from <command-line>:
In function ‘__kmalloc_index’,
    inlined from ‘kmalloc_cache_alignment’ at mm/kfence/kfence_test.c:200:50:
././include/linux/compiler_types.h:328:38: error: call to ‘__compiletime_assert_131’ declared with attribute error: BUILD_BUG_ON failed: 1
  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/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
   50 |  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
      |  ^~~~~~~~~~~~~~~~
./include/linux/slab.h:383:2: note: in expansion of macro ‘BUILD_BUG_ON’
  383 |  BUILD_BUG_ON(1);
      |  ^~~~~~~~~~~~
In function ‘__kmalloc_index’,
    inlined from ‘test_alloc’ at mm/kfence/kfence_test.c:271:47:
././include/linux/compiler_types.h:328:38: error: call to ‘__compiletime_assert_131’ declared with attribute error: BUILD_BUG_ON failed: 1
  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/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
   50 |  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
      |  ^~~~~~~~~~~~~~~~
./include/linux/slab.h:383:2: note: in expansion of macro ‘BUILD_BUG_ON’
  383 |  BUILD_BUG_ON(1);
      |  ^~~~~~~~~~~~
make[2]: *** [scripts/Makefile.build:272: mm/kfence/kfence_test.o] 오류 1
make[1]: *** [scripts/Makefile.build:533: mm/kfence] 오류 2
make: *** [Makefile:1948: mm] 오류 2

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]

[-- Attachment #1: Type: text/plain, Size: 12800 bytes --]

On Tue, Jun 08, 2021 at 07:27:52PM +0200, Vlastimil Babka wrote:
> On 6/8/21 7:05 PM, Hyeonggon Yoo wrote:
> > On Tue, Jun 08, 2021 at 09:57:18AM +0200, Vlastimil Babka wrote:
> >> On 6/7/21 5:49 PM, Hyeonggon Yoo wrote:
> >> > On Mon, Jun 07, 2021 at 05:27:27PM +0200, Vlastimil Babka wrote:
> >> >> On 6/7/21 2:25 PM, Hyeonggon Yoo wrote:
> >> >> > 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:
> >> >> >> 
> >> >> >> 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'm involving my gcc colleagues, will report results...
> >> 
> >> Well, it seems the bot's .config included CONFIG_PROFILE_ALL_BRANCHES as the
> >> main factor to trigger the problem. And (according to my colleagues)
> > 
> > Wow, AWESOME! How did your colleague find it? that was a big hint for me.
> > when CONFIG_PROFILE_ALL_BRANCHES is not set, it doesn't make an error.
> 
> Well, we started with me doing "make kernel/bpf/local_storage.i" to create a
> preprocessed C source that can be copied out and debugged outside of the whole
> kernel build context. I send that to my colleague and he reduced the testcase
> using cvise:
> 
> https://gcc.gnu.org/wiki/A_guide_to_testcase_reduction
> 
> While the reduction wasn't successful in preserving enough of the testcase, from
> the result it was clear that there was lots of ftrace_branch_data stuff and so
> it was easy to find this is due to
> CONFIG_PROFILE_ALL_BRANCHES.
> 

First time to hear about testcase reduction and cvise!
Thank you for letting me know it :)
I'm going to read it. but I ask some questions before that (see below)

> > mm.. when the size passed to bpf_map_kmalloc_node is not constant,
> > (__builtin_constant_p(size) && size <= KMALLOC_MAX_CACHE_SIZE) is false.
> > then __builtin_constant_p(!!false) is true. So it calls kmalloc_index.
> > 
> > what change should be made?
> > just checking CONFIG_PROFILE_ALL_BRANCHES to kmalloc_index's if condition
> > doesn't make sense, because kmalloc_node is not working as expected.
>
> If I understood my colleagues right, the problem is that, while kmalloc_index()
> seems to contains a number of *independent* "if (size <= X) conditions in a
> sequence, the machinery of CONFIG_PROFILE_ALL_BRANCHES turns it to a deeply
> nested if-then-else { if-then-else { if-then-else {...}}} thing (in fact using
> the ternary operators, not if-then-else).
> At some point of the deep nesting gcc
> "forgets" that __builtin_constant_p() is true and starts behaving as if it wasn't.
> 
> Without CONFIG_PROFILE_ALL_BRANCHES it's able to keep track of it fine.
>

I think you are talking about some if statements in kmalloc_index.

How do you know gcc "forgets" __builtin_constant_p() is true?
can it be debugged using cvise? (not offending, just because
I don't know about cvise yet).

Also, as I understand right, kmalloc_index doesn't have information
about the value of __builtin_constant_p(size). the caller of
kmalloc_index (kmalloc_node here) has that information.

If I understand right, what CONFIG_PROFILE_ALL_BRANCHES does is below.

replacing if(cond) { body } -> if (
						__builtin_constant_p(cond) ?
							then (cond)
						else (cond, record something)
					) { body }

I think it does not make deep nested statements.

the below is some part of preprocessor output.
CONFIG_PROFILE_ALL_BRANCHES makes some ugly ternary operators,
but there is no deep-nested if-then-else statements.

if ( (__builtin_constant_p(!!(size <= 8)) ? (!!(size <= 8)) : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 392, }; (!!(size <= 8)) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) return 3;

if ( (__builtin_constant_p(!!(size <= 16)) ? (!!(size <= 16)) : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 393, }; (!!(size <= 16)) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) return 4;

if ( (__builtin_constant_p(!!(size <= 32)) ? (!!(size <= 32)) : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 394, }; (!!(size <= 32)) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) return 5;

.... and some if statements ...

And I think, the problem is on kmalloc_node, not on kmalloc_index.
the original code of kmalloc_node is below:


static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
  {
  #ifndef CONFIG_SLOB
        if (__builtin_constant_p(size) &&
              size <= KMALLOC_MAX_CACHE_SIZE) {
              unsigned int i = kmalloc_index(size);

and which is changed to below: (by CONFIG_PROFILE_ALL_BRANCHES)

static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
{

	if ( (
		__builtin_constant_p(
		!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))
		)
			? (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))) 
			: ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 601, }; (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) )
                                  {
  unsigned int i = __kmalloc_index(size, true);


they are so ugly but the point is:

> > It seems that gcc evaluates
> > 
> > __builtin_constant_p(
> > 				!!(builtin_constant_p(size)
> > 				&& size <= KMALLOC_MAX_CACHE_SIZE)
> > 			)
> > 	as true.
> > 
> > mm.. when the size passed to bpf_map_kmalloc_node is not constant,
> > (__builtin_constant_p(size) && size <= KMALLOC_MAX_CACHE_SIZE) is false.
> > then __builtin_constant_p(!!false) is true. So it calls kmalloc_index.
> > 
> > what change should be made?
> > just checking CONFIG_PROFILE_ALL_BRANCHES to kmalloc_index's if condition
> > doesn't make sense, because kmalloc_node is not working as expected.

some evidence to add:

	there are 4 calls of bpf_map_kmalloc_node.
	if you comment out two calls of bpf_map_kmalloc_node with non-constant
	(in line 168, 508), it doesn't make an error. So I thought
	it was problem when non-constant size was passed.

	And if "kmalloc_index makes problem with non-constant size" is
	true, then it is caller's fault because it is not allowed (except
	in __kmalloc_index)

	kmalloc_node shouldn't call kmalloc_index if the size was not
	constant.

> > void *kmalloc_node(size_t size, gfp_t flags, int node){
> > 
> >  if ( (__builtin_constant_p(
> >        !!(__builtin_constant_p(size)
> >        && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))))
> >             ? (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))))


> > if ( (__builtin_constant_p(!!((0 || 110000 >= 110000) && size_is_constant)) ? (!!((0 || 110000 >= 110000) && size_is_constant)) : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 416, }; (!!((0 || 110000 >= 110000) && size_is_constant)) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) )
> >   do {
> > 
> > 
> >         extern void __compiletime_assert_131(void) ;
> > 	  // here - compiletime_assert_131 does NOTHING
> 
> It doesn't seem to do nothing? see below
> 
> > 		if ( (__builtin_constant_p(!!(!(!(1))))
> >               ? (!!(!(!(1))))
> >               : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 417, }; (!!(!(!(1)))) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) __compiletime_assert_131(); } while (0);
> 
> The thing above seems to be exactly the "if (!(condition))
> prefix ## suffix();   } while (0)" as the definition you posted below.
> 
> Anyway, you can verify that clang works by commenting out the highest size
> checks and passing a constant size that makes it reach the  BUILD_BUG_ON_MSG() ?
> 

I verified as below.
clang didn't make an error, gcc worked as expected.

then I can explain why there is no error with clang:
	it just did nothing with BUILD_BUG_ON.

hyeyoo(a)hyeyoo:~/바탕화면/linux-next$ git diff
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 8d8dd8571261..f2f9a6a7e663 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -379,6 +379,9 @@ static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
 static __always_inline unsigned int __kmalloc_index(size_t size,
                                                    bool size_is_constant)
 {
+
+       BUILD_BUG_ON(1);
+
        if (!size)
                return 0;

hyeyoo(a)hyeyoo:~/바탕화면/linux-next$ make mm/kfence/kfence_test.o CC=clang-11
  CALL    scripts/checksyscalls.sh

	... some headers omitted ...

  CC      mm/kfence/kfence_test.o
In file included from <command-line>:
In function ‘__kmalloc_index’,
    inlined from ‘kmalloc_cache_alignment’ at mm/kfence/kfence_test.c:200:50:
././include/linux/compiler_types.h:328:38: error: call to ‘__compiletime_assert_131’ declared with attribute error: BUILD_BUG_ON failed: 1
  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/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
   50 |  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
      |  ^~~~~~~~~~~~~~~~
./include/linux/slab.h:383:2: note: in expansion of macro ‘BUILD_BUG_ON’
  383 |  BUILD_BUG_ON(1);
      |  ^~~~~~~~~~~~
In function ‘__kmalloc_index’,
    inlined from ‘test_alloc’ at mm/kfence/kfence_test.c:271:47:
././include/linux/compiler_types.h:328:38: error: call to ‘__compiletime_assert_131’ declared with attribute error: BUILD_BUG_ON failed: 1
  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/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
   50 |  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
      |  ^~~~~~~~~~~~~~~~
./include/linux/slab.h:383:2: note: in expansion of macro ‘BUILD_BUG_ON’
  383 |  BUILD_BUG_ON(1);
      |  ^~~~~~~~~~~~
make[2]: *** [scripts/Makefile.build:272: mm/kfence/kfence_test.o] 오류 1
make[1]: *** [scripts/Makefile.build:533: mm/kfence] 오류 2
make: *** [Makefile:1948: mm] 오류 2

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]

On Wed, Jun 09, 2021 at 03:45:01AM +0900, Hyeonggon Yoo wrote:
> I verified as below.
> clang didn't make an error, gcc worked as expected.
> 
> then I can explain why there is no error with clang:
> 	it just did nothing with BUILD_BUG_ON.
>

Well I deleted output something too much... (making it confusing)
I send output without deleting part of it

hyeyoo@hyeyoo:~/바탕화면/linux-next$ git diff
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 8d8dd8571261..f2f9a6a7e663 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -379,6 +379,9 @@ static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
 static __always_inline unsigned int __kmalloc_index(size_t size,
                                                    bool size_is_constant)
 {
+
+       BUILD_BUG_ON(1);
+
        if (!size)
                return 0;
 
hyeyoo@hyeyoo:~/바탕화면/linux-next$ make mm/kfence/kfence_test.o
  CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  DESCEND objtool
  CC      mm/kfence/kfence_test.o
In file included from <command-line>:
In function ‘__kmalloc_index’,
    inlined from ‘kmalloc_cache_alignment’ at mm/kfence/kfence_test.c:200:50:
././include/linux/compiler_types.h:328:38: error: call to ‘__compiletime_assert_131’ declared with attribute error: BUILD_BUG_ON failed: 1
  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/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
   50 |  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
      |  ^~~~~~~~~~~~~~~~
./include/linux/slab.h:383:2: note: in expansion of macro ‘BUILD_BUG_ON’
  383 |  BUILD_BUG_ON(1);
      |  ^~~~~~~~~~~~
In function ‘__kmalloc_index’,
    inlined from ‘test_alloc’ at mm/kfence/kfence_test.c:271:47:
././include/linux/compiler_types.h:328:38: error: call to ‘__compiletime_assert_131’ declared with attribute error: BUILD_BUG_ON failed: 1
  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/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
   50 |  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
      |  ^~~~~~~~~~~~~~~~
./include/linux/slab.h:383:2: note: in expansion of macro ‘BUILD_BUG_ON’
  383 |  BUILD_BUG_ON(1);
      |  ^~~~~~~~~~~~
make[2]: *** [scripts/Makefile.build:272: mm/kfence/kfence_test.o] 오류 1
make[1]: *** [scripts/Makefile.build:533: mm/kfence] 오류 2
make: *** [Makefile:1948: mm] 오류 2
hyeyoo@hyeyoo:~/바탕화면/linux-next$ make mm/kfence/kfence_test.o CC=clang-11
  SYNC    include/config/auto.conf.cmd
*
* Restart config...
*
*
* Memory initialization
*
Initialize kernel stack variables at function entry
> 1. no automatic initialization (weakest) (INIT_STACK_NONE)
  2. 0xAA-init everything on the stack (strongest) (INIT_STACK_ALL_PATTERN) (NEW)
  3. zero-init everything on the stack (strongest and safest) (INIT_STACK_ALL_ZERO) (NEW)
choice[1-3?]: 
Enable heap memory zeroing on allocation by default (INIT_ON_ALLOC_DEFAULT_ON) [Y/n/?] y
Enable heap memory zeroing on free by default (INIT_ON_FREE_DEFAULT_ON) [N/y/?] n
*
* KCSAN: dynamic data race detector
*
KCSAN: dynamic data race detector (KCSAN) [N/y/?] (NEW) 
  CC      scripts/mod/empty.o
  MKELF   scripts/mod/elfconfig.h
  HOSTCC  scripts/mod/modpost.o
  CC      scripts/mod/devicetable-offsets.s
  HOSTCC  scripts/mod/file2alias.o
  HOSTCC  scripts/mod/sumversion.o
  HOSTLD  scripts/mod/modpost
  CC      kernel/bounds.s
  CC      arch/x86/kernel/asm-offsets.s
  CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  DESCEND objtool
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/arch/x86/special.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/arch/x86/decode.o
  LD      /home/hyeyoo/바탕화면/linux-next/tools/objtool/arch/x86/objtool-in.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/weak.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/check.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/special.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/orc_gen.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/orc_dump.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/builtin-check.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/builtin-orc.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/elf.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/objtool.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/libstring.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/libctype.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/str_error_r.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/librbtree.o
  LD      /home/hyeyoo/바탕화면/linux-next/tools/objtool/objtool-in.o
  LINK    /home/hyeyoo/바탕화면/linux-next/tools/objtool/objtool
  CC      mm/kfence/kfence_test.o
hyeyoo@hyeyoo:~/바탕화면/linux-next$ 

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]

[-- Attachment #1: Type: text/plain, Size: 6502 bytes --]

On Wed, Jun 09, 2021 at 03:45:01AM +0900, Hyeonggon Yoo wrote:
> I verified as below.
> clang didn't make an error, gcc worked as expected.
> 
> then I can explain why there is no error with clang:
> 	it just did nothing with BUILD_BUG_ON.
>

Well I deleted output something too much... (making it confusing)
I send output without deleting part of it

hyeyoo(a)hyeyoo:~/바탕화면/linux-next$ git diff
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 8d8dd8571261..f2f9a6a7e663 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -379,6 +379,9 @@ static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
 static __always_inline unsigned int __kmalloc_index(size_t size,
                                                    bool size_is_constant)
 {
+
+       BUILD_BUG_ON(1);
+
        if (!size)
                return 0;
 
hyeyoo(a)hyeyoo:~/바탕화면/linux-next$ make mm/kfence/kfence_test.o
  CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  DESCEND objtool
  CC      mm/kfence/kfence_test.o
In file included from <command-line>:
In function ‘__kmalloc_index’,
    inlined from ‘kmalloc_cache_alignment’ at mm/kfence/kfence_test.c:200:50:
././include/linux/compiler_types.h:328:38: error: call to ‘__compiletime_assert_131’ declared with attribute error: BUILD_BUG_ON failed: 1
  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/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
   50 |  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
      |  ^~~~~~~~~~~~~~~~
./include/linux/slab.h:383:2: note: in expansion of macro ‘BUILD_BUG_ON’
  383 |  BUILD_BUG_ON(1);
      |  ^~~~~~~~~~~~
In function ‘__kmalloc_index’,
    inlined from ‘test_alloc’ at mm/kfence/kfence_test.c:271:47:
././include/linux/compiler_types.h:328:38: error: call to ‘__compiletime_assert_131’ declared with attribute error: BUILD_BUG_ON failed: 1
  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/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
   50 |  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
      |  ^~~~~~~~~~~~~~~~
./include/linux/slab.h:383:2: note: in expansion of macro ‘BUILD_BUG_ON’
  383 |  BUILD_BUG_ON(1);
      |  ^~~~~~~~~~~~
make[2]: *** [scripts/Makefile.build:272: mm/kfence/kfence_test.o] 오류 1
make[1]: *** [scripts/Makefile.build:533: mm/kfence] 오류 2
make: *** [Makefile:1948: mm] 오류 2
hyeyoo(a)hyeyoo:~/바탕화면/linux-next$ make mm/kfence/kfence_test.o CC=clang-11
  SYNC    include/config/auto.conf.cmd
*
* Restart config...
*
*
* Memory initialization
*
Initialize kernel stack variables at function entry
> 1. no automatic initialization (weakest) (INIT_STACK_NONE)
  2. 0xAA-init everything on the stack (strongest) (INIT_STACK_ALL_PATTERN) (NEW)
  3. zero-init everything on the stack (strongest and safest) (INIT_STACK_ALL_ZERO) (NEW)
choice[1-3?]: 
Enable heap memory zeroing on allocation by default (INIT_ON_ALLOC_DEFAULT_ON) [Y/n/?] y
Enable heap memory zeroing on free by default (INIT_ON_FREE_DEFAULT_ON) [N/y/?] n
*
* KCSAN: dynamic data race detector
*
KCSAN: dynamic data race detector (KCSAN) [N/y/?] (NEW) 
  CC      scripts/mod/empty.o
  MKELF   scripts/mod/elfconfig.h
  HOSTCC  scripts/mod/modpost.o
  CC      scripts/mod/devicetable-offsets.s
  HOSTCC  scripts/mod/file2alias.o
  HOSTCC  scripts/mod/sumversion.o
  HOSTLD  scripts/mod/modpost
  CC      kernel/bounds.s
  CC      arch/x86/kernel/asm-offsets.s
  CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  DESCEND objtool
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/arch/x86/special.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/arch/x86/decode.o
  LD      /home/hyeyoo/바탕화면/linux-next/tools/objtool/arch/x86/objtool-in.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/weak.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/check.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/special.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/orc_gen.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/orc_dump.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/builtin-check.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/builtin-orc.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/elf.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/objtool.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/libstring.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/libctype.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/str_error_r.o
  CC      /home/hyeyoo/바탕화면/linux-next/tools/objtool/librbtree.o
  LD      /home/hyeyoo/바탕화면/linux-next/tools/objtool/objtool-in.o
  LINK    /home/hyeyoo/바탕화면/linux-next/tools/objtool/objtool
  CC      mm/kfence/kfence_test.o
hyeyoo(a)hyeyoo:~/바탕화면/linux-next$ 

Some logical errors on my words, but I still wonder...

[Hyeonggon Yoo]

On Wed, Jun 09, 2021 at 03:45:01AM +0900, Hyeonggon Yoo wrote:
> static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
> {
> 
> 	if ( (
> 		__builtin_constant_p(
> 		!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))
> 		)
> 			? (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))) 
> 			: ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 601, }; (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) )
>                                   {
>   unsigned int i = __kmalloc_index(size, true);
> 
> 
> they are so ugly but the point is:
> 
> > > It seems that gcc evaluates
> > > 
> > > __builtin_constant_p(
> > > 				!!(builtin_constant_p(size)
> > > 				&& size <= KMALLOC_MAX_CACHE_SIZE)
> > > 			)
> > > 	as true.
> > >
> > > mm.. when the size passed to bpf_map_kmalloc_node is not constant,
> > > (__builtin_constant_p(size) && size <= KMALLOC_MAX_CACHE_SIZE) is false.
> > > then __builtin_constant_p(!!false) is true. So it calls kmalloc_index.
> > >

There were some logical errors on my words. I still think the compiler
evaluate it as true, but that is not reason why kmalloc_node calls
__kmalloc_index for non-constant size.

I wonder why kmalloc_node called __kmalloc_index(size, size_is_constant=true)
for non-constant size.

Thanks,
Hyeonggon

> > > what change should be made?
> > > just checking CONFIG_PROFILE_ALL_BRANCHES to kmalloc_index's if condition
> > > doesn't make sense, because kmalloc_node is not working as expected.


> some evidence to add:
> 
> 	there are 4 calls of bpf_map_kmalloc_node.
> 	if you comment out two calls of bpf_map_kmalloc_node with non-constant
> 	(in line 168, 508), it doesn't make an error. So I thought
> 	it was problem when non-constant size was passed.
> 
> 	And if "kmalloc_index makes problem with non-constant size" is
> 	true, then it is caller's fault because it is not allowed (except
> 	in __kmalloc_index)
> 
> 	kmalloc_node shouldn't call kmalloc_index if the size was not
> 	constant.

Some logical errors on my words, but I still wonder...

[Hyeonggon Yoo]

[-- Attachment #1: Type: text/plain, Size: 2366 bytes --]

On Wed, Jun 09, 2021 at 03:45:01AM +0900, Hyeonggon Yoo wrote:
> static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
> {
> 
> 	if ( (
> 		__builtin_constant_p(
> 		!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))
> 		)
> 			? (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))) 
> 			: ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 601, }; (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) )
>                                   {
>   unsigned int i = __kmalloc_index(size, true);
> 
> 
> they are so ugly but the point is:
> 
> > > It seems that gcc evaluates
> > > 
> > > __builtin_constant_p(
> > > 				!!(builtin_constant_p(size)
> > > 				&& size <= KMALLOC_MAX_CACHE_SIZE)
> > > 			)
> > > 	as true.
> > >
> > > mm.. when the size passed to bpf_map_kmalloc_node is not constant,
> > > (__builtin_constant_p(size) && size <= KMALLOC_MAX_CACHE_SIZE) is false.
> > > then __builtin_constant_p(!!false) is true. So it calls kmalloc_index.
> > >

There were some logical errors on my words. I still think the compiler
evaluate it as true, but that is not reason why kmalloc_node calls
__kmalloc_index for non-constant size.

I wonder why kmalloc_node called __kmalloc_index(size, size_is_constant=true)
for non-constant size.

Thanks,
Hyeonggon

> > > what change should be made?
> > > just checking CONFIG_PROFILE_ALL_BRANCHES to kmalloc_index's if condition
> > > doesn't make sense, because kmalloc_node is not working as expected.


> some evidence to add:
> 
> 	there are 4 calls of bpf_map_kmalloc_node.
> 	if you comment out two calls of bpf_map_kmalloc_node with non-constant
> 	(in line 168, 508), it doesn't make an error. So I thought
> 	it was problem when non-constant size was passed.
> 
> 	And if "kmalloc_index makes problem with non-constant size" is
> 	true, then it is caller's fault because it is not allowed (except
> 	in __kmalloc_index)
> 
> 	kmalloc_node shouldn't call kmalloc_index if the size was not
> 	constant.

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]

On 6/8/21 8:45 PM, Hyeonggon Yoo wrote:
>> > mm.. when the size passed to bpf_map_kmalloc_node is not constant,
>> > (__builtin_constant_p(size) && size <= KMALLOC_MAX_CACHE_SIZE) is false.
>> > then __builtin_constant_p(!!false) is true. So it calls kmalloc_index.
>> > 
>> > what change should be made?
>> > just checking CONFIG_PROFILE_ALL_BRANCHES to kmalloc_index's if condition
>> > doesn't make sense, because kmalloc_node is not working as expected.
>>
>> If I understood my colleagues right, the problem is that, while kmalloc_index()
>> seems to contains a number of *independent* "if (size <= X) conditions in a
>> sequence, the machinery of CONFIG_PROFILE_ALL_BRANCHES turns it to a deeply
>> nested if-then-else { if-then-else { if-then-else {...}}} thing (in fact using
>> the ternary operators, not if-then-else).
>> At some point of the deep nesting gcc
>> "forgets" that __builtin_constant_p() is true and starts behaving as if it wasn't.
>> 
>> Without CONFIG_PROFILE_ALL_BRANCHES it's able to keep track of it fine.
>>
> 
> I think you are talking about some if statements in kmalloc_index.
> 
> How do you know gcc "forgets" __builtin_constant_p() is true?
> can it be debugged using cvise? (not offending, just because
> I don't know about cvise yet).
> 
> Also, as I understand right, kmalloc_index doesn't have information
> about the value of __builtin_constant_p(size). the caller of
> kmalloc_index (kmalloc_node here) has that information.
> 
> If I understand right, what CONFIG_PROFILE_ALL_BRANCHES does is below.
> 
> replacing if(cond) { body } -> if (
> 						__builtin_constant_p(cond) ?
> 							then (cond)
> 						else (cond, record something)
> 					) { body }
> 
> I think it does not make deep nested statements.

OK, might have been a misunderstanding of cvise output.
So I don't know why exactly there are false positives with
CONFIG_PROFILE_ALL_BRANCHES.

> the below is some part of preprocessor output.
> CONFIG_PROFILE_ALL_BRANCHES makes some ugly ternary operators,
> but there is no deep-nested if-then-else statements.
> 
> if ( (__builtin_constant_p(!!(size <= 8)) ? (!!(size <= 8)) : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 392, }; (!!(size <= 8)) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) return 3;
> 
> if ( (__builtin_constant_p(!!(size <= 16)) ? (!!(size <= 16)) : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 393, }; (!!(size <= 16)) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) return 4;
> 
> if ( (__builtin_constant_p(!!(size <= 32)) ? (!!(size <= 32)) : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 394, }; (!!(size <= 32)) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) return 5;
> 
> .... and some if statements ...
> 
> And I think, the problem is on kmalloc_node, not on kmalloc_index.
> the original code of kmalloc_node is below:
> 
> 
> static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
>   {
>   #ifndef CONFIG_SLOB
>         if (__builtin_constant_p(size) &&
>               size <= KMALLOC_MAX_CACHE_SIZE) {
>               unsigned int i = kmalloc_index(size);
> 
> and which is changed to below: (by CONFIG_PROFILE_ALL_BRANCHES)
> 
> static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
> {
> 
> 	if ( (
> 		__builtin_constant_p(
> 		!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))
> 		)
> 			? (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))) 
> 			: ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 601, }; (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) )
>                                   {
>   unsigned int i = __kmalloc_index(size, true);
> 
> 
> they are so ugly but the point is:
> 
>> > It seems that gcc evaluates
>> > 
>> > __builtin_constant_p(
>> > 				!!(builtin_constant_p(size)
>> > 				&& size <= KMALLOC_MAX_CACHE_SIZE)
>> > 			)
>> > 	as true.
>> > 
>> > mm.. when the size passed to bpf_map_kmalloc_node is not constant,
>> > (__builtin_constant_p(size) && size <= KMALLOC_MAX_CACHE_SIZE) is false.
>> > then __builtin_constant_p(!!false) is true. So it calls kmalloc_index.
>> > 
>> > what change should be made?
>> > just checking CONFIG_PROFILE_ALL_BRANCHES to kmalloc_index's if condition
>> > doesn't make sense, because kmalloc_node is not working as expected.
> 
> some evidence to add:
> 
> 	there are 4 calls of bpf_map_kmalloc_node.
> 	if you comment out two calls of bpf_map_kmalloc_node with non-constant
> 	(in line 168, 508), it doesn't make an error. So I thought
> 	it was problem when non-constant size was passed.
> 
> 	And if "kmalloc_index makes problem with non-constant size" is
> 	true, then it is caller's fault because it is not allowed (except
> 	in __kmalloc_index)
> 
> 	kmalloc_node shouldn't call kmalloc_index if the size was not
> 	constant.

Yes. You could perhaps exclude CONFIG_PROFILE_ALL_BRANCHES for now, and fill
official gcc bugzilla with the preprocessed file.
Bonus points if you can use cvise in a way that reduces the testcase but does
not remove any of the important parts. All we could get were degenerate cases
like this one
https://paste.opensuse.org/9320186

>> > void *kmalloc_node(size_t size, gfp_t flags, int node){
>> > 
>> >  if ( (__builtin_constant_p(
>> >        !!(__builtin_constant_p(size)
>> >        && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))))
>> >             ? (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))))
> 
> 
>> > if ( (__builtin_constant_p(!!((0 || 110000 >= 110000) && size_is_constant)) ? (!!((0 || 110000 >= 110000) && size_is_constant)) : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 416, }; (!!((0 || 110000 >= 110000) && size_is_constant)) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) )
>> >   do {
>> > 
>> > 
>> >         extern void __compiletime_assert_131(void) ;
>> > 	  // here - compiletime_assert_131 does NOTHING
>> 
>> It doesn't seem to do nothing? see below
>> 
>> > 		if ( (__builtin_constant_p(!!(!(!(1))))
>> >               ? (!!(!(!(1))))
>> >               : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 417, }; (!!(!(!(1)))) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) __compiletime_assert_131(); } while (0);
>> 
>> The thing above seems to be exactly the "if (!(condition))
>> prefix ## suffix();   } while (0)" as the definition you posted below.
>> 
>> Anyway, you can verify that clang works by commenting out the highest size
>> checks and passing a constant size that makes it reach the  BUILD_BUG_ON_MSG() ?
>> 
> 
> I verified as below.
> clang didn't make an error, gcc worked as expected.
> 
> then I can explain why there is no error with clang:
> 	it just did nothing with BUILD_BUG_ON.

That might warrant a clang bug report too. We excluded clang 10 due to false
positives. If the BUILD_BUG_ON doesn't work at all in clang 11 we might have to
exclude clang completely.

> hyeyoo@hyeyoo:~/바탕화면/linux-next$ git diff
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 8d8dd8571261..f2f9a6a7e663 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -379,6 +379,9 @@ static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
>  static __always_inline unsigned int __kmalloc_index(size_t size,
>                                                     bool size_is_constant)
>  {
> +
> +       BUILD_BUG_ON(1);
> +
>         if (!size)
>                 return 0;
> 
> hyeyoo@hyeyoo:~/바탕화면/linux-next$ make mm/kfence/kfence_test.o CC=clang-11
>   CALL    scripts/checksyscalls.sh
> 
> 	... some headers omitted ...
> 
>   CC      mm/kfence/kfence_test.o
> In file included from <command-line>:
> In function ‘__kmalloc_index’,
>     inlined from ‘kmalloc_cache_alignment’ at mm/kfence/kfence_test.c:200:50:
> ././include/linux/compiler_types.h:328:38: error: call to ‘__compiletime_assert_131’ declared with attribute error: BUILD_BUG_ON failed: 1
>   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/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
>    50 |  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
>       |  ^~~~~~~~~~~~~~~~
> ./include/linux/slab.h:383:2: note: in expansion of macro ‘BUILD_BUG_ON’
>   383 |  BUILD_BUG_ON(1);
>       |  ^~~~~~~~~~~~
> In function ‘__kmalloc_index’,
>     inlined from ‘test_alloc’ at mm/kfence/kfence_test.c:271:47:
> ././include/linux/compiler_types.h:328:38: error: call to ‘__compiletime_assert_131’ declared with attribute error: BUILD_BUG_ON failed: 1
>   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/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
>    50 |  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
>       |  ^~~~~~~~~~~~~~~~
> ./include/linux/slab.h:383:2: note: in expansion of macro ‘BUILD_BUG_ON’
>   383 |  BUILD_BUG_ON(1);
>       |  ^~~~~~~~~~~~
> make[2]: *** [scripts/Makefile.build:272: mm/kfence/kfence_test.o] 오류 1
> make[1]: *** [scripts/Makefile.build:533: mm/kfence] 오류 2
> make: *** [Makefile:1948: mm] 오류 2
> 

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]

[-- Attachment #1: Type: text/plain, Size: 11811 bytes --]

On 6/8/21 8:45 PM, Hyeonggon Yoo wrote:
>> > mm.. when the size passed to bpf_map_kmalloc_node is not constant,
>> > (__builtin_constant_p(size) && size <= KMALLOC_MAX_CACHE_SIZE) is false.
>> > then __builtin_constant_p(!!false) is true. So it calls kmalloc_index.
>> > 
>> > what change should be made?
>> > just checking CONFIG_PROFILE_ALL_BRANCHES to kmalloc_index's if condition
>> > doesn't make sense, because kmalloc_node is not working as expected.
>>
>> If I understood my colleagues right, the problem is that, while kmalloc_index()
>> seems to contains a number of *independent* "if (size <= X) conditions in a
>> sequence, the machinery of CONFIG_PROFILE_ALL_BRANCHES turns it to a deeply
>> nested if-then-else { if-then-else { if-then-else {...}}} thing (in fact using
>> the ternary operators, not if-then-else).
>> At some point of the deep nesting gcc
>> "forgets" that __builtin_constant_p() is true and starts behaving as if it wasn't.
>> 
>> Without CONFIG_PROFILE_ALL_BRANCHES it's able to keep track of it fine.
>>
> 
> I think you are talking about some if statements in kmalloc_index.
> 
> How do you know gcc "forgets" __builtin_constant_p() is true?
> can it be debugged using cvise? (not offending, just because
> I don't know about cvise yet).
> 
> Also, as I understand right, kmalloc_index doesn't have information
> about the value of __builtin_constant_p(size). the caller of
> kmalloc_index (kmalloc_node here) has that information.
> 
> If I understand right, what CONFIG_PROFILE_ALL_BRANCHES does is below.
> 
> replacing if(cond) { body } -> if (
> 						__builtin_constant_p(cond) ?
> 							then (cond)
> 						else (cond, record something)
> 					) { body }
> 
> I think it does not make deep nested statements.

OK, might have been a misunderstanding of cvise output.
So I don't know why exactly there are false positives with
CONFIG_PROFILE_ALL_BRANCHES.

> the below is some part of preprocessor output.
> CONFIG_PROFILE_ALL_BRANCHES makes some ugly ternary operators,
> but there is no deep-nested if-then-else statements.
> 
> if ( (__builtin_constant_p(!!(size <= 8)) ? (!!(size <= 8)) : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 392, }; (!!(size <= 8)) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) return 3;
> 
> if ( (__builtin_constant_p(!!(size <= 16)) ? (!!(size <= 16)) : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 393, }; (!!(size <= 16)) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) return 4;
> 
> if ( (__builtin_constant_p(!!(size <= 32)) ? (!!(size <= 32)) : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 394, }; (!!(size <= 32)) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) return 5;
> 
> .... and some if statements ...
> 
> And I think, the problem is on kmalloc_node, not on kmalloc_index.
> the original code of kmalloc_node is below:
> 
> 
> static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
>   {
>   #ifndef CONFIG_SLOB
>         if (__builtin_constant_p(size) &&
>               size <= KMALLOC_MAX_CACHE_SIZE) {
>               unsigned int i = kmalloc_index(size);
> 
> and which is changed to below: (by CONFIG_PROFILE_ALL_BRANCHES)
> 
> static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
> {
> 
> 	if ( (
> 		__builtin_constant_p(
> 		!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))
> 		)
> 			? (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))) 
> 			: ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 601, }; (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25)))) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) )
>                                   {
>   unsigned int i = __kmalloc_index(size, true);
> 
> 
> they are so ugly but the point is:
> 
>> > It seems that gcc evaluates
>> > 
>> > __builtin_constant_p(
>> > 				!!(builtin_constant_p(size)
>> > 				&& size <= KMALLOC_MAX_CACHE_SIZE)
>> > 			)
>> > 	as true.
>> > 
>> > mm.. when the size passed to bpf_map_kmalloc_node is not constant,
>> > (__builtin_constant_p(size) && size <= KMALLOC_MAX_CACHE_SIZE) is false.
>> > then __builtin_constant_p(!!false) is true. So it calls kmalloc_index.
>> > 
>> > what change should be made?
>> > just checking CONFIG_PROFILE_ALL_BRANCHES to kmalloc_index's if condition
>> > doesn't make sense, because kmalloc_node is not working as expected.
> 
> some evidence to add:
> 
> 	there are 4 calls of bpf_map_kmalloc_node.
> 	if you comment out two calls of bpf_map_kmalloc_node with non-constant
> 	(in line 168, 508), it doesn't make an error. So I thought
> 	it was problem when non-constant size was passed.
> 
> 	And if "kmalloc_index makes problem with non-constant size" is
> 	true, then it is caller's fault because it is not allowed (except
> 	in __kmalloc_index)
> 
> 	kmalloc_node shouldn't call kmalloc_index if the size was not
> 	constant.

Yes. You could perhaps exclude CONFIG_PROFILE_ALL_BRANCHES for now, and fill
official gcc bugzilla with the preprocessed file.
Bonus points if you can use cvise in a way that reduces the testcase but does
not remove any of the important parts. All we could get were degenerate cases
like this one
https://paste.opensuse.org/9320186

>> > void *kmalloc_node(size_t size, gfp_t flags, int node){
>> > 
>> >  if ( (__builtin_constant_p(
>> >        !!(__builtin_constant_p(size)
>> >        && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))))
>> >             ? (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))))
> 
> 
>> > if ( (__builtin_constant_p(!!((0 || 110000 >= 110000) && size_is_constant)) ? (!!((0 || 110000 >= 110000) && size_is_constant)) : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 416, }; (!!((0 || 110000 >= 110000) && size_is_constant)) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) )
>> >   do {
>> > 
>> > 
>> >         extern void __compiletime_assert_131(void) ;
>> > 	  // here - compiletime_assert_131 does NOTHING
>> 
>> It doesn't seem to do nothing? see below
>> 
>> > 		if ( (__builtin_constant_p(!!(!(!(1))))
>> >               ? (!!(!(!(1))))
>> >               : ({ static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func = __func__, .file = "include/linux/slab.h", .line = 417, }; (!!(!(!(1)))) ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) __compiletime_assert_131(); } while (0);
>> 
>> The thing above seems to be exactly the "if (!(condition))
>> prefix ## suffix();   } while (0)" as the definition you posted below.
>> 
>> Anyway, you can verify that clang works by commenting out the highest size
>> checks and passing a constant size that makes it reach the  BUILD_BUG_ON_MSG() ?
>> 
> 
> I verified as below.
> clang didn't make an error, gcc worked as expected.
> 
> then I can explain why there is no error with clang:
> 	it just did nothing with BUILD_BUG_ON.

That might warrant a clang bug report too. We excluded clang 10 due to false
positives. If the BUILD_BUG_ON doesn't work at all in clang 11 we might have to
exclude clang completely.

> hyeyoo(a)hyeyoo:~/바탕화면/linux-next$ git diff
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 8d8dd8571261..f2f9a6a7e663 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -379,6 +379,9 @@ static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
>  static __always_inline unsigned int __kmalloc_index(size_t size,
>                                                     bool size_is_constant)
>  {
> +
> +       BUILD_BUG_ON(1);
> +
>         if (!size)
>                 return 0;
> 
> hyeyoo(a)hyeyoo:~/바탕화면/linux-next$ make mm/kfence/kfence_test.o CC=clang-11
>   CALL    scripts/checksyscalls.sh
> 
> 	... some headers omitted ...
> 
>   CC      mm/kfence/kfence_test.o
> In file included from <command-line>:
> In function ‘__kmalloc_index’,
>     inlined from ‘kmalloc_cache_alignment’ at mm/kfence/kfence_test.c:200:50:
> ././include/linux/compiler_types.h:328:38: error: call to ‘__compiletime_assert_131’ declared with attribute error: BUILD_BUG_ON failed: 1
>   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/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
>    50 |  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
>       |  ^~~~~~~~~~~~~~~~
> ./include/linux/slab.h:383:2: note: in expansion of macro ‘BUILD_BUG_ON’
>   383 |  BUILD_BUG_ON(1);
>       |  ^~~~~~~~~~~~
> In function ‘__kmalloc_index’,
>     inlined from ‘test_alloc’ at mm/kfence/kfence_test.c:271:47:
> ././include/linux/compiler_types.h:328:38: error: call to ‘__compiletime_assert_131’ declared with attribute error: BUILD_BUG_ON failed: 1
>   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/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
>    50 |  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
>       |  ^~~~~~~~~~~~~~~~
> ./include/linux/slab.h:383:2: note: in expansion of macro ‘BUILD_BUG_ON’
>   383 |  BUILD_BUG_ON(1);
>       |  ^~~~~~~~~~~~
> make[2]: *** [scripts/Makefile.build:272: mm/kfence/kfence_test.o] 오류 1
> make[1]: *** [scripts/Makefile.build:533: mm/kfence] 오류 2
> make: *** [Makefile:1948: mm] 오류 2
> 

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]

[-- Attachment #1: Type: text/plain, Size: 10569 bytes --]

On Thu, Jun 10, 2021, 8:43 PM Vlastimil Babka <vbabka@suse.cz> wrote:


>> > void *kmalloc_node(size_t size, gfp_t flags, int node){
>> >
>> >  if ( (__builtin_constant_p(
>> >        !!(__builtin_constant_p(size)
>> >        && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))))
>> >             ? (!!(__builtin_constant_p(size) && size <= (1UL << ((11 +
12 - 1) <= 25 ? (11 + 12 - 1) : 25))))
>
>
>> > if ( (__builtin_constant_p(!!((0 || 110000 >= 110000) &&
size_is_constant)) ? (!!((0 || 110000 >= 110000) && size_is_constant)) : ({
static struct ftrace_branch_data __attribute__((__aligned__(4)))
__attribute__((__section__("_ftrace_branch"))) __if_trace = { .func =
__func__, .file = "include/linux/slab.h", .line = 416, }; (!!((0 || 110000
>= 110000) && size_is_constant)) ? (__if_trace.miss_hit[1]++,1) :
(__if_trace.miss_hit[0]++,0); })) )
>> >   do {
>> >
>> >
>> >         extern void __compiletime_assert_131(void) ;
>> >      // here - compiletime_assert_131 does NOTHING
>>
>> It doesn't seem to do nothing? see below
>>
>> >            if ( (__builtin_constant_p(!!(!(!(1))))
>> >               ? (!!(!(!(1))))
>> >               : ({ static struct ftrace_branch_data
__attribute__((__aligned__(4)))
__attribute__((__section__("_ftrace_branch"))) __if_trace = { .func =
__func__, .file = "include/linux/slab.h", .line = 417, }; (!!(!(!(1)))) ?
(__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) )
__compiletime_assert_131(); } while (0);
>>
>> The thing above seems to be exactly the "if (!(condition))
>> prefix ## suffix();   } while (0)" as the definition you posted below.
>>
>> Anyway, you can verify that clang works by commenting out the highest
size
>> checks and passing a constant size that makes it reach the
BUILD_BUG_ON_MSG() ?
>>
>
> I verified as below.
> clang didn't make an error, gcc worked as expected.
>
> then I can explain why there is no error with clang:
>       it just did nothing with BUILD_BUG_ON.

That might warrant a clang bug report too. We excluded clang 10 due to false
positives. If the BUILD_BUG_ON doesn't work at all in clang 11 we might
have to
exclude clang completely.


After playing with clang more a bit, I got to know that compiletime_assert
makes weird link error (undefined reference to compiletime_assert_XXX), Not
a compile error.


I think it's time to CC ClangBuiltLinux maintainers, who work on clang/llvm
build support.

[+CC Nathan and Nick]

I assumeed that compiletime_assert (in linux/compiler.h) will make compiler
error, but it makes no compile error, just makes weird link error.

I'm not sure it it works well with clang, or somewhat buggy status?



> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 8d8dd8571261..f2f9a6a7e663 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -379,6 +379,9 @@ static __always_inline enum kmalloc_cache_type
kmalloc_type(gfp_t flags)
>  static __always_inline unsigned int __kmalloc_index(size_t size,
>                                                     bool size_is_constant)
>  {
> +
> +       BUILD_BUG_ON(1);
> +
>         if (!size)
>                 return 0;
>



On 6/8/21 8:45 PM, Hyeonggon Yoo wrote:
> >> > mm.. when the size passed to bpf_map_kmalloc_node is not constant,
> >> > (__builtin_constant_p(size) && size <= KMALLOC_MAX_CACHE_SIZE) is
> false.
> >> > then __builtin_constant_p(!!false) is true. So it calls kmalloc_index.
> >> >
> >> > what change should be made?
> >> > just checking CONFIG_PROFILE_ALL_BRANCHES to kmalloc_index's if
> condition
> >> > doesn't make sense, because kmalloc_node is not working as expected.
> >>
> >> If I understood my colleagues right, the problem is that, while
> kmalloc_index()
> >> seems to contains a number of *independent* "if (size <= X) conditions
> in a
> >> sequence, the machinery of CONFIG_PROFILE_ALL_BRANCHES turns it to a
> deeply
> >> nested if-then-else { if-then-else { if-then-else {...}}} thing (in
> fact using
> >> the ternary operators, not if-then-else).
> >> At some point of the deep nesting gcc
> >> "forgets" that __builtin_constant_p() is true and starts behaving as if
> it wasn't.
> >>
> >> Without CONFIG_PROFILE_ALL_BRANCHES it's able to keep track of it fine.
> >>
> >
> > I think you are talking about some if statements in kmalloc_index.
> >
> > How do you know gcc "forgets" __builtin_constant_p() is true?
> > can it be debugged using cvise? (not offending, just because
> > I don't know about cvise yet).
> >
> > Also, as I understand right, kmalloc_index doesn't have information
> > about the value of __builtin_constant_p(size). the caller of
> > kmalloc_index (kmalloc_node here) has that information.
> >
> > If I understand right, what CONFIG_PROFILE_ALL_BRANCHES does is below.
> >
> > replacing if(cond) { body } -> if (
> >                                               __builtin_constant_p(cond)
> ?
> >                                                       then (cond)
> >                                               else (cond, record
> something)
> >                                       ) { body }
> >
> > I think it does not make deep nested statements.
>
> OK, might have been a misunderstanding of cvise output.
> So I don't know why exactly there are false positives with
> CONFIG_PROFILE_ALL_BRANCHES.
>
> > the below is some part of preprocessor output.
> > CONFIG_PROFILE_ALL_BRANCHES makes some ugly ternary operators,
> > but there is no deep-nested if-then-else statements.
> >
> > if ( (__builtin_constant_p(!!(size <= 8)) ? (!!(size <= 8)) : ({ static
> struct ftrace_branch_data __attribute__((__aligned__(4)))
> __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func =
> __func__, .file = "include/linux/slab.h", .line = 392, }; (!!(size <= 8)) ?
> (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) return 3;
> >
> > if ( (__builtin_constant_p(!!(size <= 16)) ? (!!(size <= 16)) : ({
> static struct ftrace_branch_data __attribute__((__aligned__(4)))
> __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func =
> __func__, .file = "include/linux/slab.h", .line = 393, }; (!!(size <= 16))
> ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) return
> 4;
> >
> > if ( (__builtin_constant_p(!!(size <= 32)) ? (!!(size <= 32)) : ({
> static struct ftrace_branch_data __attribute__((__aligned__(4)))
> __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func =
> __func__, .file = "include/linux/slab.h", .line = 394, }; (!!(size <= 32))
> ? (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) ) return
> 5;
> >
> > .... and some if statements ...
> >
> > And I think, the problem is on kmalloc_node, not on kmalloc_index.
> > the original code of kmalloc_node is below:
> >
> >
> > static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int
> node)
> >   {
> >   #ifndef CONFIG_SLOB
> >         if (__builtin_constant_p(size) &&
> >               size <= KMALLOC_MAX_CACHE_SIZE) {
> >               unsigned int i = kmalloc_index(size);
> >
> > and which is changed to below: (by CONFIG_PROFILE_ALL_BRANCHES)
> >
> > static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int
> node)
> > {
> >
> >       if ( (
> >               __builtin_constant_p(
> >               !!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12
> - 1) <= 25 ? (11 + 12 - 1) : 25)))
> >               )
> >                       ? (!!(__builtin_constant_p(size) && size <= (1UL
> << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))))
> >                       : ({ static struct ftrace_branch_data
> __attribute__((__aligned__(4)))
> __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func =
> __func__, .file = "include/linux/slab.h", .line = 601, };
> (!!(__builtin_constant_p(size) && size <= (1UL << ((11 + 12 - 1) <= 25 ?
> (11 + 12 - 1) : 25)))) ? (__if_trace.miss_hit[1]++,1) :
> (__if_trace.miss_hit[0]++,0); })) )
> >                                   {
> >   unsigned int i = __kmalloc_index(size, true);
> >
> >
> > they are so ugly but the point is:
> >
> >> > It seems that gcc evaluates
> >> >
> >> > __builtin_constant_p(
> >> >                            !!(builtin_constant_p(size)
> >> >                            && size <= KMALLOC_MAX_CACHE_SIZE)
> >> >                    )
> >> >    as true.
> >> >
> >> > mm.. when the size passed to bpf_map_kmalloc_node is not constant,
> >> > (__builtin_constant_p(size) && size <= KMALLOC_MAX_CACHE_SIZE) is
> false.
> >> > then __builtin_constant_p(!!false) is true. So it calls kmalloc_index.
> >> >
> >> > what change should be made?
> >> > just checking CONFIG_PROFILE_ALL_BRANCHES to kmalloc_index's if
> condition
> >> > doesn't make sense, because kmalloc_node is not working as expected.
> >
> > some evidence to add:
> >
> >       there are 4 calls of bpf_map_kmalloc_node.
> >       if you comment out two calls of bpf_map_kmalloc_node with
> non-constant
> >       (in line 168, 508), it doesn't make an error. So I thought
> >       it was problem when non-constant size was passed.
> >
> >       And if "kmalloc_index makes problem with non-constant size" is
> >       true, then it is caller's fault because it is not allowed (except
> >       in __kmalloc_index)
> >
> >       kmalloc_node shouldn't call kmalloc_index if the size was not
> >       constant.
>
> Yes. You could perhaps exclude CONFIG_PROFILE_ALL_BRANCHES for now,


You do you mean by "exclude CONFIG_PROFILE_ALL_BRANCHES for now"?

Did you mean we don't need to fix it for now? (Anyway I have no idea what
the fix patch would be for now...)

and fill
> official gcc bugzilla with the preprocessed file.
> Bonus points if you can use cvise in a way that reduces the testcase but
> does
> not remove any of the important parts. All we could get were degenerate
> cases
> like this one
> https://paste.opensuse.org/9320186


I'll try that and report it to gcc bugzilla soon!

I think I should let you know that I'm going to korean army next week, So I
can't access internet for next two months... (Because they don't allow
internet in military training period)

I'll be back after 2 months, But can I ask you (Vlastimil) to write follow
up patch if we can fix it before I'm back?

Sorry I didn't know compiler will bother us at that time I wrote the patch
:(

[-- Attachment #2: attachment.htm --]
[-- Type: text/html, Size: 14412 bytes --]

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]

[-- Attachment #1: Type: text/plain, Size: 4491 bytes --]

On 6/11/21 10:58 AM, Hyeonggon Yoo wrote:
> On Thu, Jun 10, 2021, 8:43 PM Vlastimil Babka <vbabka@suse.cz
> <mailto:vbabka@suse.cz>> wrote:
>  
> 
>     >> > void *kmalloc_node(size_t size, gfp_t flags, int node){
>     >> >
>     >> >  if ( (__builtin_constant_p(
>     >> >        !!(__builtin_constant_p(size)
>     >> >        && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1) : 25))))
>     >> >             ? (!!(__builtin_constant_p(size) && size <= (1UL << ((11 +
>     12 - 1) <= 25 ? (11 + 12 - 1) : 25))))
>     >
>     >
>     >> > if ( (__builtin_constant_p(!!((0 || 110000 >= 110000) &&
>     size_is_constant)) ? (!!((0 || 110000 >= 110000) && size_is_constant)) : ({
>     static struct ftrace_branch_data __attribute__((__aligned__(4)))
>     __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func =
>     __func__, .file = "include/linux/slab.h", .line = 416, }; (!!((0 || 110000
>     >= 110000) && size_is_constant)) ? (__if_trace.miss_hit[1]++,1) :
>     (__if_trace.miss_hit[0]++,0); })) )
>     >> >   do {
>     >> >
>     >> >
>     >> >         extern void __compiletime_assert_131(void) ;
>     >> >      // here - compiletime_assert_131 does NOTHING
>     >>
>     >> It doesn't seem to do nothing? see below
>     >>
>     >> >            if ( (__builtin_constant_p(!!(!(!(1))))
>     >> >               ? (!!(!(!(1))))
>     >> >               : ({ static struct ftrace_branch_data
>     __attribute__((__aligned__(4)))
>     __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func =
>     __func__, .file = "include/linux/slab.h", .line = 417, }; (!!(!(!(1)))) ?
>     (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) )
>     __compiletime_assert_131(); } while (0);
>     >>
>     >> The thing above seems to be exactly the "if (!(condition))
>     >> prefix ## suffix();   } while (0)" as the definition you posted below.
>     >>
>     >> Anyway, you can verify that clang works by commenting out the highest size
>     >> checks and passing a constant size that makes it reach the 
>     BUILD_BUG_ON_MSG() ?
>     >>
>     >
>     > I verified as below.
>     > clang didn't make an error, gcc worked as expected.
>     >
>     > then I can explain why there is no error with clang:
>     >       it just did nothing with BUILD_BUG_ON.
> 
>     That might warrant a clang bug report too. We excluded clang 10 due to false
>     positives. If the BUILD_BUG_ON doesn't work at all in clang 11 we might have to
>     exclude clang completely.
> 
> 
> After playing with clang more a bit, I got to know that compiletime_assert makes
> weird link error (undefined reference to compiletime_assert_XXX), Not a compile
> error.

Ah well that's much better than no error at all.

> I think it's time to CC ClangBuiltLinux maintainers, who work on clang/llvm
> build support.
> 
> [+CC Nathan and Nick]

Good.

> I assumeed that compiletime_assert (in linux/compiler.h) will make compiler
> error, but it makes no compile error, just makes weird link error.
> 
> I'm not sure it it works well with clang, or somewhat buggy status?
> 
> 
> 

...

>     Yes. You could perhaps exclude CONFIG_PROFILE_ALL_BRANCHES for now, 
> 
> 
> You do you mean by "exclude CONFIG_PROFILE_ALL_BRANCHES for now"?
> 
> Did you mean we don't need to fix it for now? (Anyway I have no idea what the
> fix patch would be for now...)

I meant the the condition to use BUILD_BUG_ON instead of BUG_ON would include
!IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES)

>     and fill
>     official gcc bugzilla with the preprocessed file.
>     Bonus points if you can use cvise in a way that reduces the testcase but does
>     not remove any of the important parts. All we could get were degenerate cases
>     like this one
>     https://paste.opensuse.org/9320186 <https://paste.opensuse.org/9320186>
> 
> 
> I'll try that and report it to gcc bugzilla soon!
> 
> I think I should let you know that I'm going to korean army next week, So I
> can't access internet for next two months... (Because they don't allow internet
> in military training period)
> 
> I'll be back after 2 months, But can I ask you (Vlastimil) to write follow up
> patch if we can fix it before I'm back?

Sure, take care!

> Sorry I didn't know compiler will bother us at that time I wrote the patch :(
> 

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]

[-- Attachment #1: Type: text/plain, Size: 5685 bytes --]

On Fri, Jun 11, 2021, 7:27 PM Vlastimil Babka <vbabka@suse.cz> wrote:

> On 6/11/21 10:58 AM, Hyeonggon Yoo wrote:
> > On Thu, Jun 10, 2021, 8:43 PM Vlastimil Babka <vbabka@suse.cz
> > <mailto:vbabka@suse.cz>> wrote:
> >
> >
> >     >> > void *kmalloc_node(size_t size, gfp_t flags, int node){
> >     >> >
> >     >> >  if ( (__builtin_constant_p(
> >     >> >        !!(__builtin_constant_p(size)
> >     >> >        && size <= (1UL << ((11 + 12 - 1) <= 25 ? (11 + 12 - 1)
> : 25))))
> >     >> >             ? (!!(__builtin_constant_p(size) && size <= (1UL <<
> ((11 +
> >     12 - 1) <= 25 ? (11 + 12 - 1) : 25))))
> >     >
> >     >
> >     >> > if ( (__builtin_constant_p(!!((0 || 110000 >= 110000) &&
> >     size_is_constant)) ? (!!((0 || 110000 >= 110000) &&
> size_is_constant)) : ({
> >     static struct ftrace_branch_data __attribute__((__aligned__(4)))
> >     __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func =
> >     __func__, .file = "include/linux/slab.h", .line = 416, }; (!!((0 ||
> 110000
> >     >= 110000) && size_is_constant)) ? (__if_trace.miss_hit[1]++,1) :
> >     (__if_trace.miss_hit[0]++,0); })) )
> >     >> >   do {
> >     >> >
> >     >> >
> >     >> >         extern void __compiletime_assert_131(void) ;
> >     >> >      // here - compiletime_assert_131 does NOTHING
> >     >>
> >     >> It doesn't seem to do nothing? see below
> >     >>
> >     >> >            if ( (__builtin_constant_p(!!(!(!(1))))
> >     >> >               ? (!!(!(!(1))))
> >     >> >               : ({ static struct ftrace_branch_data
> >     __attribute__((__aligned__(4)))
> >     __attribute__((__section__("_ftrace_branch"))) __if_trace = { .func =
> >     __func__, .file = "include/linux/slab.h", .line = 417, };
> (!!(!(!(1)))) ?
> >     (__if_trace.miss_hit[1]++,1) : (__if_trace.miss_hit[0]++,0); })) )
> >     __compiletime_assert_131(); } while (0);
> >     >>
> >     >> The thing above seems to be exactly the "if (!(condition))
> >     >> prefix ## suffix();   } while (0)" as the definition you posted
> below.
> >     >>
> >     >> Anyway, you can verify that clang works by commenting out the
> highest size
> >     >> checks and passing a constant size that makes it reach the
> >     BUILD_BUG_ON_MSG() ?
> >     >>
> >     >
> >     > I verified as below.
> >     > clang didn't make an error, gcc worked as expected.
> >     >
> >     > then I can explain why there is no error with clang:
> >     >       it just did nothing with BUILD_BUG_ON.
> >
> >     That might warrant a clang bug report too. We excluded clang 10 due
> to false
> >     positives. If the BUILD_BUG_ON doesn't work at all in clang 11 we
> might have to
> >     exclude clang completely.
> >
> >
> > After playing with clang more a bit, I got to know that
> compiletime_assert makes
> > weird link error (undefined reference to compiletime_assert_XXX), Not a
> compile
> > error.
>
> Ah well that's much better than no error at all.
>
> > I think it's time to CC ClangBuiltLinux maintainers, who work on
> clang/llvm
> > build support.
> >
> > [+CC Nathan and Nick]
>
> Good.
>

Forgot to link our previous work:

https://lore.kernel.org/lkml/20210515210950.GA52841(a)hyeyoo/

https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org/thread/45UNTPJX7VUKLB6TUYPJOS7DMREVW5US/


> > I assumeed that compiletime_assert (in linux/compiler.h) will make
> compiler
> > error, but it makes no compile error, just makes weird link error.
> >
> > I'm not sure it it works well with clang, or somewhat buggy status?
> >
> >
> >
>
> ...
>
...?

>
> >     Yes. You could perhaps exclude CONFIG_PROFILE_ALL_BRANCHES for now,
> >
> >
> > You do you mean by "exclude CONFIG_PROFILE_ALL_BRANCHES for now"?
> >
> > Did you mean we don't need to fix it for now? (Anyway I have no idea
> what the
> > fix patch would be for now...)
>
> I meant the the condition to use BUILD_BUG_ON instead of BUG_ON would
> include
> !IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES)
>

You mean this? (This will make kmalloc_index return -1 without BUG())

if ((IS_ENABLED(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION >= 110000) &&
size_is_constant)
        BUILD_BUG_ON_MSG(!IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES),
"unexpected size in kmalloc_index()");
else
        BUG();

Or This?

if ((IS_ENABLED(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION >= 110000) &&
size_is_constant && !IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES))
        BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()");
else
        BUG();

Maybe this version seems better

But little bit worried :(
The code is getting too complicated...
How do you think?


> >     and fill
> >     official gcc bugzilla with the preprocessed file.
> >     Bonus points if you can use cvise in a way that reduces the testcase
> but does
> >     not remove any of the important parts. All we could get were
> degenerate cases
> >     like this one
> >     https://paste.opensuse.org/9320186 <
> https://paste.opensuse.org/9320186>
> >
> >
> > I'll try that and report it to gcc bugzilla soon!
> >
> > I think I should let you know that I'm going to korean army next week,
> So I
> > can't access internet for next two months... (Because they don't allow
> internet
> > in military training period)
> >
> > I'll be back after 2 months, But can I ask you (Vlastimil) to write
> follow up
> > patch if we can fix it before I'm back?
>
> Sure, take care!
>

Thank you a lot!

>
> > Sorry I didn't know compiler will bother us at that time I wrote the
> patch :(
> >
>
>

[-- Attachment #2: attachment.htm --]
[-- Type: text/html, Size: 9173 bytes --]

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]

On 6/11/21 1:56 PM, Hyeonggon Yoo wrote:
> On Fri, Jun 11, 2021, 7:27 PM Vlastimil Babka <vbabka@suse.cz
> <mailto:vbabka@suse.cz>> wrote:
>     I meant the the condition to use BUILD_BUG_ON instead of BUG_ON would include
>     !IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES)
> 
> 
> You mean this? (This will make kmalloc_index return -1 without BUG())
> 
> if ((IS_ENABLED(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION >= 110000) &&
> size_is_constant)
>         BUILD_BUG_ON_MSG(!IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES), "unexpected
> size in kmalloc_index()");
> else
>         BUG();

No,

> Or This?
> 
> if ((IS_ENABLED(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION >= 110000) &&
> size_is_constant && !IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES))
>         BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()");
> else
>         BUG();
> 
> Maybe this version seems better

Yeah, meant that.

> But little bit worried :(
> The code is getting too complicated...
> How do you think?

Yeah, I expected that problems like this could occur as we're poking at some
rare corner cases of compiler implementations here. But if that leads to fixes
in compilers, good for everyone I'd say.

So I would try this, even if it becomes complicated.

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]

[-- Attachment #1: Type: text/plain, Size: 1281 bytes --]

On 6/11/21 1:56 PM, Hyeonggon Yoo wrote:
> On Fri, Jun 11, 2021, 7:27 PM Vlastimil Babka <vbabka@suse.cz
> <mailto:vbabka@suse.cz>> wrote:
>     I meant the the condition to use BUILD_BUG_ON instead of BUG_ON would include
>     !IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES)
> 
> 
> You mean this? (This will make kmalloc_index return -1 without BUG())
> 
> if ((IS_ENABLED(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION >= 110000) &&
> size_is_constant)
>         BUILD_BUG_ON_MSG(!IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES), "unexpected
> size in kmalloc_index()");
> else
>         BUG();

No,

> Or This?
> 
> if ((IS_ENABLED(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION >= 110000) &&
> size_is_constant && !IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES))
>         BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()");
> else
>         BUG();
> 
> Maybe this version seems better

Yeah, meant that.

> But little bit worried :(
> The code is getting too complicated...
> How do you think?

Yeah, I expected that problems like this could occur as we're poking at some
rare corner cases of compiler implementations here. But if that leads to fixes
in compilers, good for everyone I'd say.

So I would try this, even if it becomes complicated.

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]

[-- Attachment #1: Type: text/plain, Size: 1640 bytes --]

On Sat, Jun 12, 2021, 8:59 AM Vlastimil Babka <vbabka@suse.cz> wrote:

> On 6/11/21 1:56 PM, Hyeonggon Yoo wrote:
> > On Fri, Jun 11, 2021, 7:27 PM Vlastimil Babka <vbabka@suse.cz
> > <mailto:vbabka@suse.cz>> wrote:
> >     I meant the the condition to use BUILD_BUG_ON instead of BUG_ON
> would include
> >     !IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES)
> >
> >
> > You mean this? (This will make kmalloc_index return -1 without BUG())
> >
> > if ((IS_ENABLED(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION >= 110000) &&
> > size_is_constant)
> >         BUILD_BUG_ON_MSG(!IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES),
> "unexpected
> > size in kmalloc_index()");
> > else
> >         BUG();
>
> No,
>
> > Or This?
> >
> > if ((IS_ENABLED(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION >= 110000) &&
> > size_is_constant && !IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES))
> >         BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()");
> > else
> >         BUG();
> >
> > Maybe this version seems better
>
> Yeah, meant that.
>

Ah, okay!


> > But little bit worried :(
> > The code is getting too complicated...
> > How do you think?
>
> Yeah, I expected that problems like this could occur as we're poking at
> some
> rare corner cases of compiler implementations here. But if that leads to
> fixes
> in compilers, good for everyone I'd say.

So I would try this, even if it becomes complicated.
>

Okay, I see the code is okay, but one thing to suggest:
    The problem already existed in kmalloc_node before the patch And we
found it by adding BUILD_BUG_ON in kmalloc_index.

    So I suggest adding the condition in kmalloc_node. How about this?

>

[-- Attachment #2: Type: text/html, Size: 3014 bytes --]

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]

[-- Attachment #1: Type: text/plain, Size: 1698 bytes --]

On Sat, Jun 12, 2021, 8:59 AM Vlastimil Babka <vbabka@suse.cz> wrote:

> On 6/11/21 1:56 PM, Hyeonggon Yoo wrote:
> > On Fri, Jun 11, 2021, 7:27 PM Vlastimil Babka <vbabka@suse.cz
> > <mailto:vbabka@suse.cz>> wrote:
> >     I meant the the condition to use BUILD_BUG_ON instead of BUG_ON
> would include
> >     !IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES)
> >
> >
> > You mean this? (This will make kmalloc_index return -1 without BUG())
> >
> > if ((IS_ENABLED(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION >= 110000) &&
> > size_is_constant)
> >         BUILD_BUG_ON_MSG(!IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES),
> "unexpected
> > size in kmalloc_index()");
> > else
> >         BUG();
>
> No,
>
> > Or This?
> >
> > if ((IS_ENABLED(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION >= 110000) &&
> > size_is_constant && !IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES))
> >         BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()");
> > else
> >         BUG();
> >
> > Maybe this version seems better
>
> Yeah, meant that.
>

Ah, okay!


> > But little bit worried :(
> > The code is getting too complicated...
> > How do you think?
>
> Yeah, I expected that problems like this could occur as we're poking at
> some
> rare corner cases of compiler implementations here. But if that leads to
> fixes
> in compilers, good for everyone I'd say.

So I would try this, even if it becomes complicated.
>

Okay, I see the code is okay, but one thing to suggest:
    The problem already existed in kmalloc_node before the patch And we
found it by adding BUILD_BUG_ON in kmalloc_index.

    So I suggest adding the condition in kmalloc_node. How about this?

>

[-- Attachment #2: attachment.htm --]
[-- Type: text/html, Size: 2976 bytes --]

[PATCH FIX -next] 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]

On 6/12/21 2:19 AM, Hyeonggon Yoo wrote:
> On Sat, Jun 12, 2021, 8:59 AM Vlastimil Babka <vbabka@suse.cz
> <mailto:vbabka@suse.cz>> wrote:
> 
> 
>     > But little bit worried :(
>     > The code is getting too complicated...
>     > How do you think?
> 
>     Yeah, I expected that problems like this could occur as we're poking at some
>     rare corner cases of compiler implementations here. But if that leads to fixes
>     in compilers, good for everyone I'd say.
> 
>     So I would try this, even if it becomes complicated.
> 
> 
> Okay, I see the code is okay, but one thing to suggest:
>     The problem already existed in kmalloc_node before the patch And we found it
> by adding BUILD_BUG_ON in kmalloc_index.
> 
>     So I suggest adding the condition in kmalloc_node. How about this?
> 

If we add it to kmalloc_node() it means with CONFIG_PROFILE_ALL_BRANCHES we will
not be using the compile-time optimized kmalloc_index anywhere. But we get the
error only with one file, so in all the other cases the gcc optimizer seems to
work fine even with this config enabled. Placing the condition to
kmalloc_index() thus limits it only to cases where it doesn't work.

As you indicated you would be away starting this week, let me just sent the fix
right now so we stop getting the -next reports:

----8<----
From 56c164dc5c0485c2e133190c0f56e4069844ad62 Mon Sep 17 00:00:00 2001
From: Vlastimil Babka <vbabka@suse.cz>
Date: Mon, 14 Jun 2021 11:16:03 +0200
Subject: [PATCH] 
 mm-slub-change-run-time-assertion-in-kmalloc_index-to-compile-time-fix-3

Kernel test robot reports a false-positive compile-time assert while compiling
kernel/bpf/local_storage.c

   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()");
         |  ^~~~~~~~~~~~~~~~

This only happens with CONFIG_PROFILE_ALL_BRANCHES enabled. Informal
conversation with gcc developers suggest that this config pushes the optimizer
to a corner case where in kmalloc_node() the 'size' is considered a
builtin-constant and thus kmalloc_index() is chosen, but later in
kmalloc_index() the notion of compile-time constant is lost and thus the
compiletime assert becomes reachable.

While there's plan to submit a proper gcc bug report with reduced testcase,
for now add CONFIG_PROFILE_ALL_BRANCHES to the list of situations where we
keep using the runtime BUG_ON() instead of compile-time assert.

This is a 3rd fix for mmotm patch
mm-slub-change-run-time-assertion-in-kmalloc_index-to-compile-time.patch

[1] https://lore.kernel.org/linux-mm/202106051442.G1VJubTz-lkp@intel.com/

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 include/linux/slab.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 8d8dd8571261..083f3ce550bc 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -413,7 +413,8 @@ 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 ((IS_ENABLED(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION >= 110000) && size_is_constant)
+	if ((IS_ENABLED(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION >= 110000)
+	    && !IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES) && size_is_constant)
 		BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()");
 	else
 		BUG();
-- 
2.32.0

[PATCH FIX -next] 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]

[-- Attachment #1: Type: text/plain, Size: 4679 bytes --]


On 6/12/21 2:19 AM, Hyeonggon Yoo wrote:
> On Sat, Jun 12, 2021, 8:59 AM Vlastimil Babka <vbabka@suse.cz
> <mailto:vbabka@suse.cz>> wrote:
> 
> 
>     > But little bit worried :(
>     > The code is getting too complicated...
>     > How do you think?
> 
>     Yeah, I expected that problems like this could occur as we're poking at some
>     rare corner cases of compiler implementations here. But if that leads to fixes
>     in compilers, good for everyone I'd say.
> 
>     So I would try this, even if it becomes complicated.
> 
> 
> Okay, I see the code is okay, but one thing to suggest:
>     The problem already existed in kmalloc_node before the patch And we found it
> by adding BUILD_BUG_ON in kmalloc_index.
> 
>     So I suggest adding the condition in kmalloc_node. How about this?
> 

If we add it to kmalloc_node() it means with CONFIG_PROFILE_ALL_BRANCHES we will
not be using the compile-time optimized kmalloc_index anywhere. But we get the
error only with one file, so in all the other cases the gcc optimizer seems to
work fine even with this config enabled. Placing the condition to
kmalloc_index() thus limits it only to cases where it doesn't work.

As you indicated you would be away starting this week, let me just sent the fix
right now so we stop getting the -next reports:

----8<----
>From 56c164dc5c0485c2e133190c0f56e4069844ad62 Mon Sep 17 00:00:00 2001
From: Vlastimil Babka <vbabka@suse.cz>
Date: Mon, 14 Jun 2021 11:16:03 +0200
Subject: [PATCH] 
 mm-slub-change-run-time-assertion-in-kmalloc_index-to-compile-time-fix-3

Kernel test robot reports a false-positive compile-time assert while compiling
kernel/bpf/local_storage.c

   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()");
         |  ^~~~~~~~~~~~~~~~

This only happens with CONFIG_PROFILE_ALL_BRANCHES enabled. Informal
conversation with gcc developers suggest that this config pushes the optimizer
to a corner case where in kmalloc_node() the 'size' is considered a
builtin-constant and thus kmalloc_index() is chosen, but later in
kmalloc_index() the notion of compile-time constant is lost and thus the
compiletime assert becomes reachable.

While there's plan to submit a proper gcc bug report with reduced testcase,
for now add CONFIG_PROFILE_ALL_BRANCHES to the list of situations where we
keep using the runtime BUG_ON() instead of compile-time assert.

This is a 3rd fix for mmotm patch
mm-slub-change-run-time-assertion-in-kmalloc_index-to-compile-time.patch

[1] https://lore.kernel.org/linux-mm/202106051442.G1VJubTz-lkp(a)intel.com/

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 include/linux/slab.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 8d8dd8571261..083f3ce550bc 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -413,7 +413,8 @@ 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 ((IS_ENABLED(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION >= 110000) && size_is_constant)
+	if ((IS_ENABLED(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION >= 110000)
+	    && !IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES) && size_is_constant)
 		BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()");
 	else
 		BUG();
-- 
2.32.0

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

[Nathan Chancellor]

On 6/11/2021 1:58 AM, Hyeonggon Yoo wrote:> After playing with clang 
more a bit, I got to know that
> compiletime_assert makes weird link error (undefined reference to 
> compiletime_assert_XXX), Not a compile error.
> 
> 
> I think it's time to CC ClangBuiltLinux maintainers, who work on 
> clang/llvm build support.
> 
> [+CC Nathan and Nick]
> 
> I assumeed that compiletime_assert (in linux/compiler.h) will make 
> compiler error, but it makes no compile error, just makes weird link error.
> 
> I'm not sure it it works well with clang, or somewhat buggy status?

I am guessing this alone is why we were keyed into the thread so I am 
just going to respond to this.

Unfortunately, this is a known issue with clang:

https://github.com/ClangBuiltLinux/linux/issues/1173

https://bugs.llvm.org/show_bug.cgi?id=16428

As you noticed, building the full kernel will result in a link error but 
it would certainly be nicer if it were a compiler error. Something for 
us to improve indeed but I am not sure when we will be able to allocate 
resources for that. Until then, you can build a full kernel to get the 
failing translation unit then use nm or readelf when building the single 
translation unit to see if there are any "__compiletime_assert" symbols.

Cheers,
Nathan

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

[Nathan Chancellor]

[-- Attachment #1: Type: text/plain, Size: 1310 bytes --]

On 6/11/2021 1:58 AM, Hyeonggon Yoo wrote:> After playing with clang 
more a bit, I got to know that
> compiletime_assert makes weird link error (undefined reference to 
> compiletime_assert_XXX), Not a compile error.
> 
> 
> I think it's time to CC ClangBuiltLinux maintainers, who work on 
> clang/llvm build support.
> 
> [+CC Nathan and Nick]
> 
> I assumeed that compiletime_assert (in linux/compiler.h) will make 
> compiler error, but it makes no compile error, just makes weird link error.
> 
> I'm not sure it it works well with clang, or somewhat buggy status?

I am guessing this alone is why we were keyed into the thread so I am 
just going to respond to this.

Unfortunately, this is a known issue with clang:

https://github.com/ClangBuiltLinux/linux/issues/1173

https://bugs.llvm.org/show_bug.cgi?id=16428

As you noticed, building the full kernel will result in a link error but 
it would certainly be nicer if it were a compiler error. Something for 
us to improve indeed but I am not sure when we will be able to allocate 
resources for that. Until then, you can build a full kernel to get the 
failing translation unit then use nm or readelf when building the single 
translation unit to see if there are any "__compiletime_assert" symbols.

Cheers,
Nathan

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]

[-- Attachment #1: Type: text/plain, Size: 1797 bytes --]

On Sat, Jun 12, 2021, 1:56 AM Nathan Chancellor <nathan@kernel.org> wrote:

> On 6/11/2021 1:58 AM, Hyeonggon Yoo wrote:> After playing with clang
> more a bit, I got to know that
> > compiletime_assert makes weird link error (undefined reference to
> > compiletime_assert_XXX), Not a compile error.
> >
> >
> > I think it's time to CC ClangBuiltLinux maintainers, who work on
> > clang/llvm build support.
> >
> > [+CC Nathan and Nick]
> >
> > I assumeed that compiletime_assert (in linux/compiler.h) will make
> > compiler error, but it makes no compile error, just makes weird link
> error.
> >
> > I'm not sure it it works well with clang, or somewhat buggy status?
>
> I am guessing this alone is why we were keyed into the thread so I am
> just going to respond to this.
>

Thank you for quick reply!

Unfortunately, this is a known issue with clang:
>
> https://github.com/ClangBuiltLinux/linux/issues/1173
>
> https://bugs.llvm.org/show_bug.cgi?id=16428



> As you noticed, building the full kernel will result in a link error but
> it would certainly be nicer if it were a compiler error. Something for
> us to improve indeed but I am not sure when we will be able to allocate
> resources for that.


I wanted to be sure if we can use compiletime_assert for clang. Then there
is room for improvement, but it seems okay.

Until then, you can build a full kernel to get the
> failing translation unit then use nm or readelf when building the single
> translation unit to see if there are any "__compiletime_assert" symbols.
>

Okay, then we should find symbol like that until improved.

It may confuse developer, but it seems okay for our code to support clang
as it meets minimal condition - build failure.

And I hope it become improved in future!

Thanks,
Hyeonggon

Cheers,
> Nathan
>

[-- Attachment #2: Type: text/html, Size: 3515 bytes --]

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]

[-- Attachment #1: Type: text/plain, Size: 1857 bytes --]

On Sat, Jun 12, 2021, 1:56 AM Nathan Chancellor <nathan@kernel.org> wrote:

> On 6/11/2021 1:58 AM, Hyeonggon Yoo wrote:> After playing with clang
> more a bit, I got to know that
> > compiletime_assert makes weird link error (undefined reference to
> > compiletime_assert_XXX), Not a compile error.
> >
> >
> > I think it's time to CC ClangBuiltLinux maintainers, who work on
> > clang/llvm build support.
> >
> > [+CC Nathan and Nick]
> >
> > I assumeed that compiletime_assert (in linux/compiler.h) will make
> > compiler error, but it makes no compile error, just makes weird link
> error.
> >
> > I'm not sure it it works well with clang, or somewhat buggy status?
>
> I am guessing this alone is why we were keyed into the thread so I am
> just going to respond to this.
>

Thank you for quick reply!

Unfortunately, this is a known issue with clang:
>
> https://github.com/ClangBuiltLinux/linux/issues/1173
>
> https://bugs.llvm.org/show_bug.cgi?id=16428



> As you noticed, building the full kernel will result in a link error but
> it would certainly be nicer if it were a compiler error. Something for
> us to improve indeed but I am not sure when we will be able to allocate
> resources for that.


I wanted to be sure if we can use compiletime_assert for clang. Then there
is room for improvement, but it seems okay.

Until then, you can build a full kernel to get the
> failing translation unit then use nm or readelf when building the single
> translation unit to see if there are any "__compiletime_assert" symbols.
>

Okay, then we should find symbol like that until improved.

It may confuse developer, but it seems okay for our code to support clang
as it meets minimal condition - build failure.

And I hope it become improved in future!

Thanks,
Hyeonggon

Cheers,
> Nathan
>

[-- Attachment #2: attachment.htm --]
[-- Type: text/html, Size: 3483 bytes --]