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

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

On 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