From: Tyler Retzlaff <roretzla@linux.microsoft.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>, dev@dpdk.org
Subject: Re: static_assert, sfc, and clang issues
Date: Tue, 16 Jan 2024 13:50:34 -0800 [thread overview]
Message-ID: <20240116215034.GA351@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> (raw)
In-Reply-To: <20240116090301.4c29d5b7@hermes.local>
On Tue, Jan 16, 2024 at 09:03:01AM -0800, Stephen Hemminger wrote:
> Ran into a corner case issue, so sending to mailing list for wider discussion.
>
> One improvement to DPDK code base planned is getting rid of variable length arrays.
> VLA's can cause bugs and are not supported by the Windows compiler.
> Gcc and Clang have a flag to warn on use of VLA's (-Wvla).
>
> In DPDK one use of VLA's is in the RTE_BUILD_BUG_ON macro.
> The best path is to replace the undefined array access with a better
> static_assert() which is builtin to compilers.
>
> Using static_assert() is relatively easy, there are a few places where
> it does detect use of non-constant expressions in existing code but these
> are fixable.
>
> But there is one case where use static_assert() runs into a Clang bug
> which is not fixed in distros:
>
> https://github.com/llvm/llvm-project/issues/55821
>
> The code in question is in the sfc driver which for better or worse
> has lots of assertions. The problem is that clang (except in unreleased trunk)
> can not handle static_assert in a switch leg.
>
> switch (actions->type) {
> case RTE_FLOW_ACTION_TYPE_VOID:
> SFC_BUILD_SET_OVERFLOW(RTE_FLOW_ACTION_TYPE_VOID,
> actions_set);
> break;
>
> ../drivers/net/sfc/sfc_flow.c:1635:4: error: expected expression
> SFC_BUILD_SET_OVERFLOW(RTE_FLOW_ACTION_TYPE_VOID,
> ^
> ../drivers/net/sfc/sfc_flow.h:36:2: note: expanded from macro 'SFC_BUILD_SET_OVERFLOW'
> RTE_BUILD_BUG_ON((_action) >= sizeof(_set) * CHAR_BIT)
> ^
>
>
> There are some workarounds:
> 0. Ignore it, works on Gcc, and Clang fix is pending.
> 1. Remove many of these RTE_BUILD_BUG_ON's. They are really not that helpful.
> 2. Add additional { } to these switch cases so they become basic blocks
> which works around the bug.
> 3. Move the assertions out of the switch
>
> My preference is #2 but open to other options.
+1 for #2 just make it a block.
next prev parent reply other threads:[~2024-01-16 21:50 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-16 17:03 Stephen Hemminger
2024-01-16 21:50 ` Tyler Retzlaff [this message]
2024-01-16 22:14 ` Morten Brørup
2024-01-16 22:18 ` Morten Brørup
2024-01-16 22:49 ` Stephen Hemminger
2024-01-17 8:07 ` Andrew Rybchenko
2024-01-17 9:37 ` Morten Brørup
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=20240116215034.GA351@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net \
--to=roretzla@linux.microsoft.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=dev@dpdk.org \
--cc=stephen@networkplumber.org \
/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 NNTP newsgroup(s).