Re: [PATCH bpf-next v3 3/7] bpftool: Support inline annotations when dumping the CFG of a program

[Sven Schnelle <svens@linux.ibm.com>]

Quentin Monnet <quentin@isovalent.com> writes:

> 2023-04-12 08:04 UTC+0200 ~ Sven Schnelle <svens@linux.ibm.com>
>> Quentin Monnet <quentin@isovalent.com> writes:
>> 
>>> diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
>>> index e7f6ec3a8f35..583aa843df92 100644
>>> --- a/tools/bpf/bpftool/btf_dumper.c
>>> +++ b/tools/bpf/bpftool/btf_dumper.c
>>> @@ -821,3 +821,37 @@ void btf_dump_linfo_json(const struct btf *btf,
>>>  					BPF_LINE_INFO_LINE_COL(linfo->line_col));
>>>  	}
>>>  }
>>> +
>>> +static void dotlabel_puts(const char *s)
>>> +{
>>> +	for (; *s; ++s) {
>>> +		switch (*s) {
>>> +		case '\\':
>>> +		case '"':
>>> +		case '{':
>>> +		case '}':
>>> +		case '<':
>>> +		case '>':
>>> +		case '|':
>>> +		case ' ':
>>> +			putchar('\\');
>>> +			__fallthrough;
>> 
>> Is __fallthrough correct? I see the following compile error on s390 in
>> linux-next (20230412):
>> 
>>   CC      btf_dumper.o
>> btf_dumper.c: In function ‘dotlabel_puts’:
>> btf_dumper.c:838:25: error: ‘__fallthrough’ undeclared (first use in this function); did you mean ‘fallthrough’?
>>   838 |                         __fallthrough;
>>       |                         ^~~~~~~~~~~~~
>> 
>> removing the two underscores fixes this.
>
> I thought so? Perf seems to use the double underscores as well. Just
> "fallthrough" does not seem to be the right fix anyway, it gives me an
> error similar to yours on x86_64 with "fallthrough" undeclared.
>
> The definition should be pulled from tools/include/linux/compiler.h (and
> .../compiler-gcc.h). I thought this file would be at least included from
> bpftool's main.h, in turn included in btf_dumper.c. Looking at the chain
> of inclusions, on my system I get the following path:
>
>     $ CFLAGS=-H make btf_dumper.o
>     [...]
>     . /root/dev/linux/tools/include/linux/bitops.h
>     [...]
>     .. /root/dev/linux/tools/include/linux/bits.h
>     [...]
>     ... /root/dev/linux/tools/include/linux/build_bug.h
>     .... /root/dev/linux/tools/include/linux/compiler.h
>     ..... /root/dev/linux/tools/include/linux/compiler_types.h
>     ...... /root/dev/linux/tools/include/linux/compiler-gcc.h
>     [...]
>
> What do you get on your side?
>
> If you add "#include <linux/compiler.h>" to btf_dumper.c directly, does
> it fix the issue?

This seems to clash with:

commit f7a858bffcddaaf70c71b6b656e7cc21b6107cec
Author: Liam Howlett <liam.howlett@oracle.com>
Date:   Fri Nov 25 15:50:16 2022 +0000

    tools: Rename __fallthrough to fallthrough

    Rename the fallthrough attribute to better align with the kernel
    version.  Copy the definition from include/linux/compiler_attributes.h
    including the #else clause.  Adding the #else clause allows the tools
    compiler.h header to drop the check for a definition entirely and keeps
    both definitions together.

    Change any __fallthrough statements to fallthrough anywhere it was used
    within perf.

    This allows other tools to use the same key word as the kernel.

Which was also merged in linux-next.