From: David Faust <david.faust@oracle.com>
To: Yonghong Song <yonghong.song@linux.dev>,
"Jose E. Marchesi" <jose.marchesi@oracle.com>,
bpf@vger.kernel.org
Cc: cupertino.miranda@oracle.com, Eduard Zingerman <eddyz87@gmail.com>
Subject: Re: [PATCH bpf-next] bpf: disable some `attribute ignored' warnings in GCC
Date: Mon, 6 May 2024 12:09:18 -0700 [thread overview]
Message-ID: <18123833-2d51-432d-9803-20dd76d7cccd@oracle.com> (raw)
In-Reply-To: <a95b1917-80aa-4c81-942d-91f369d31bb2@linux.dev>
On 5/6/24 11:32 AM, Yonghong Song wrote:
>
> On 5/3/24 5:32 AM, Jose E. Marchesi wrote:
>> This patch modifies selftests/bpf/Makefile to pass -Wno-attributes to
>> GCC. This is because of the following attributes which are ignored:
>>
>> - btf_decl_tag
>> - btf_type_tag
>>
>> There are many of these. At the moment none of these are
>> recognized/handled by gcc-bpf.
>>
>> We are aware that btf_decl_tag is necessary for some of the
>> selftest harness to communicate test failure/success. Support for
>> it is in progress in GCC upstream:
>>
>> https://gcc.gnu.org/pipermail/gcc-patches/2024-May/650482.html
>>
>> However, the GCC master branch is not yet open, so the series
>> above (currently under review upstream) wont be able to make it
>> there until 14.1 gets released, probably mid next week.
>
> Thanks. It would be great if the patch can be merged soon.
A small note here - the above series does not itself contain the patch
to support decl_tag, it is just some prerequisite structural changes and
the option to prune BTF before emission similar to clang to slim the
selftest (and other) program sizes down.
The patch to enable decl_tag for functions in BTF, enough for the
selftest harness, can go up after that. But, it will require some
approvals from the C front-end maintainers, since it is a new attribute,
so it may take longer, and may be contentious.
>
>>
>> As for btf_type_tag, more extensive work will be needed in GCC
>> upstream to support it in both BTF and DWARF. We have a WIP big
>> patch for that, but that is not needed to compile/build the
>> selftests.
>
> Thanks. Eduard has implemented in llvm with agreed new format. Since
> the old phabricator becomes readonly, he will upstream the original
> patch to llvm-project soon.
>
>>
>> - used
>>
>> There are SEC macros defined in the selftests as:
>>
>> #define SEC(N) __attribute__((section(N),used))
>>
>> The SEC macro is used for both functions and global variables.
>> According to the GCC documentation `used' attribute is really only
>> meaningful for functions, and it warns when the attribute is used
>> for other global objects, like for example ctl_array in
>> test_xdp_noinline.c.
>>
>> Ignoring this is bening.
>
> Bening -> Benign?
>
>>
>> - visibility
>>
>> In progs/cpumask_common.h:13 there is:
>>
>> #define private(name) SEC(".bss." #name) __hidden __attribute__((aligned(8)))
>> private(MASK) static struct bpf_cpumask __kptr * global_mask;
>>
>> The __hidden macro defines to:
>>
>> tools/lib/bpf/bpf_helpers.h:#define __hidden __attribute__((visibility("hidden")))
>>
>> GCC emits an "attribute ignored" warning because static implies
>> hidden visibility.
>>
>> Ignoring this warning is benign. An alternative would be to make
>> global_mask as non-static.
>
> In the above, let us just remove __hidden from the '#define'.
> As you mentioned, the 'global_mask' is already a static variable,
> adding '__hidden' is not really needed at all.
>
>>
>> - align_value
>>
>> In progs/test_cls_redirect.c:127 there is:
>>
>> typedef uint8_t *net_ptr __attribute__((align_value(8)));
>>
>> GCC warns that it is ignoring this attribute, because it is not
>> implemented by GCC.
>>
>> I think ignoring this attribute in GCC is bening, because according
>
> Bening -> Benign?
>
>> to the clang documentation [1] its purpose seems to be merely
>> declarative and doesn't seem to translate into extra checks at
>> run-time, only to pehaps better optimized code ("runtime behavior is
>> undefined if the pointed memory object is not aligned to the
>> specified alignment").
>
> Yes, the attribute does not really enforce at runtime. It merely
> give a declarative requirement.
>
>>
>> [1] https://clang.llvm.org/docs/AttributeReference.html#align-value
>>
>> Tested in bpf-next master.
>>
>> Signed-off-by: Jose E. Marchesi <jose.marchesi@oracle.com>
>> Cc: david.faust@oracle.com
>> Cc: cupertino.miranda@oracle.com
>> Cc: Yonghong Song <yonghong.song@linux.dev>
>> Cc: Eduard Zingerman <eddyz87@gmail.com>
>> ---
>> tools/testing/selftests/bpf/Makefile | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
>> index ba28d42b74db..5d9c906bc3cb 100644
>> --- a/tools/testing/selftests/bpf/Makefile
>> +++ b/tools/testing/selftests/bpf/Makefile
>> @@ -431,7 +431,7 @@ endef
>> # Build BPF object using GCC
>> define GCC_BPF_BUILD_RULE
>> $(call msg,GCC-BPF,$(TRUNNER_BINARY),$2)
>> - $(Q)$(BPF_GCC) $3 -O2 -c $1 -o $2
>> + $(Q)$(BPF_GCC) $3 -Wno-attributes -O2 -c $1 -o $2
>
> LGTM with above a few nits.
> Acked-by: Yonghong Song <yonghong.song@linux.dev>
>
>> endef
>>
>> SKEL_BLACKLIST := btf__% test_pinning_invalid.c test_sk_assign.c
next prev parent reply other threads:[~2024-05-06 19:06 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-03 12:32 [PATCH bpf-next] bpf: disable some `attribute ignored' warnings in GCC Jose E. Marchesi
2024-05-06 18:32 ` Yonghong Song
2024-05-06 19:09 ` David Faust [this message]
2024-05-06 21:42 ` Yonghong Song
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=18123833-2d51-432d-9803-20dd76d7cccd@oracle.com \
--to=david.faust@oracle.com \
--cc=bpf@vger.kernel.org \
--cc=cupertino.miranda@oracle.com \
--cc=eddyz87@gmail.com \
--cc=jose.marchesi@oracle.com \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).