2023-04-12 14:26 UTC+0200 ~ Sven Schnelle <svens@xxxxxxxxxxxxx>
> Quentin Monnet <quentin@xxxxxxxxxxxxx> writes:
>
>> 2023-04-12 08:04 UTC+0200 ~ Sven Schnelle <svens@xxxxxxxxxxxxx>
>>> Quentin Monnet <quentin@xxxxxxxxxxxxx> 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@xxxxxxxxxx>
> 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.
Right, I was not aware of this commit. In that case, replacing with
"fallthrough" in linux-next makes sense indeed.
Thomas Richter just submitted that fix at
https://lore.kernel.org/all/20230412123636.2358949-1-tmricht@xxxxxxxxxxxxx/.
Thanks!
Quentin
