9:56 p.m.
On Fri, Jun 11, 2021, 7:27 PM Vlastimil Babka <vbabka(a)suse.cz> wrote:
On 6/11/21 10:58 AM, Hyeonggon Yoo wrote:
> On Thu, Jun 10, 2021, 8:43 PM Vlastimil Babka <vbabka(a)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@hyeyoo/
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org/thread/45UNT...
> 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 :(
>