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

From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
To: kbuild-all@lists.01.org
Subject: 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()
Date: Fri, 11 Jun 2021 20:56:55 +0900	[thread overview]
Message-ID: <CAB=+i9Snmu7ML3Zqrbii1-jtS0BF_KeGEhn-R49Z2bh=uW-rGg@mail.gmail.com> (raw)
In-Reply-To: <d89798b1-ac4f-ab3e-27be-b1d40b8d7193@suse.cz>

[-- 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 --]