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
