public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: Yury Norov <ynorov@nvidia.com>
Cc: Matt Coster <matt.coster@imgtec.com>,
	Yury Norov <yury.norov@gmail.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Frank Binns <frank.binns@imgtec.com>,
	Alessio Belle <alessio.belle@imgtec.com>,
	Brajesh Gupta <brajesh.gupta@imgtec.com>,
	Alexandru Dadu <alexandru.dadu@imgtec.com>,
	linux-kernel@vger.kernel.org,
	Vincent Mailhol <mailhol@kernel.org>,
	kernel test robot <lkp@intel.com>
Subject: Re: [PATCH] bitfield: Fix FIELD_PREP_CONST() with __GENMASK_ULL() on gcc<14
Date: Fri, 10 Apr 2026 10:09:57 +0100	[thread overview]
Message-ID: <20260410100957.7f9bba98@pumpkin> (raw)
In-Reply-To: <adfl7OgbblyixtKV@yury>

On Thu, 9 Apr 2026 13:46:20 -0400
Yury Norov <ynorov@nvidia.com> wrote:

> On Thu, Apr 09, 2026 at 03:57:53PM +0100, Matt Coster wrote:
> > There is a bug in gcc<14[1] that causes the following minimal example to
> > not be treated as a constant:
> > 
> >   int main() {
> >       sizeof(struct {
> >           int t : !(__builtin_ffsll(~0ULL) + 1 < 0);
> >       });
> >   }
> > 
> >   test.c: In function 'main':
> >   test.c:3:21: error: bit-field 't' width not an integer constant
> >       3 |                 int t : !(__builtin_ffsll(~0ULL) + 1 < 0);
> >         |                     ^
> >   
> 
> Hi Matt,
> 
> Thanks for digging this!
> 
> So for me even
> 
>         int t : __builtin_ffsll(~1ul);
> 
> doesn't work. But '~1' or '1ull' works well. My GCC is 13.3.0.
> Clearly a compiler bug.

I one line comment that expressions involving __builtin_ffsll(1ull << 63) aren't
always constant is probably enough.

Change the test from !(...) to (...)?1:2 to avoid the zero-sized bitfield error.
(and s/main/fubar/;s/sizeof/return sizeof/ to avoid other errors.)
It is all very strange (I suspect there is a uninitialised value somewhere).
Change the '+ 1' to '+ 0' and it is ok, but '+ 0u' fails.

If it is uninitialised stack then some of the changes really are random.
So even the 'long long' cast may not guarantee to avoid the problem.

I've just 'lobbed in' a different patch.

	David

> 
> > The result of this bug is a bizarre interaction between FIELD_PREP_CONST()
> > and __GENMASK_ULL(). Note that this does not occur with GENMASK_ULL() since
> > that has not been based on the UAPI variant since commit 104ea1c84b91
> > ("bits: unify the non-asm GENMASK*()").
> > 
> > The underlying compiler bug has been fixed in gcc since 14.1.0, but the fix
> > appears to have been incidental in a larger change[2].
> > 
> > [1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=124699
> > [2]: https://gcc.gnu.org/cgit/gcc/commit/?id=0d00385eaf72ccacff17935b0d214a26773e095f
> > 
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202603222211.A2XiR1YU-lkp@intel.com/
> > Signed-off-by: Matt Coster <matt.coster@imgtec.com>
> > ---
> > Some below-the-dash thoughts:
> > 
> > This is the most minimal workaround I could find. I'm not sure what a
> > "real" fix would look like here so I'm open to suggestions.
> > 
> > The reproduction case is amazing because changing almost any subtle
> > detail of it will result in the expression correctly being parsed as
> > constant (even raising the construct to file scope; the use of
> > BUILD_BUG_ON_ZERO() is part of the bizarre confluence required to
> > trigger the bug).
> > 
> > The complexity of the associated change in gcc makes it difficult to
> > trace what actually changed to fix the underlying bug. If I had more
> > time, I'd have dug in further and (tried to) come up with an answer.
> > 
> > In reality, the (long long) cast could probably just be univerally
> > applied without the conditional compilation. My only concern with that
> > approach is that it risks turning the workaround into an arcane
> > incantation whose meaning will eventually have been lost to time.
> > ---
> >  include/linux/bitfield.h | 30 +++++++++++++++++++++++++++---
> >  1 file changed, 27 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
> > index 54aeeef1f0ec7..12f5c5a3c8d72 100644
> > --- a/include/linux/bitfield.h
> > +++ b/include/linux/bitfield.h
> > @@ -46,6 +46,30 @@
> >  
> >  #define __bf_shf(x) (__builtin_ffsll(x) - 1)
> >  
> > +#if defined(GCC_VERSION) && (GCC_VERSION < 140000)
> > +/*
> > + * This workaround is required for gcc<14. The issue is an interaction between
> > + * FIELD_PREP_CONST() and __GENMASK_ULL() that can be boiled down this MCVE,
> > + * which fails to compile as GCC doesn't recognize the expression as constant:
> > + *
> > + *   int main() {
> > + *       sizeof(struct {
> > + *           int t : !(__builtin_ffsll(~0ULL) + 1 < 0);
> > + *       });
> > + *   }
> > + *
> > + * The underlying issue was inadvertently "fixed" (or perhaps sidestepped) in
> > + * commit 0d00385eaf7 ("wide-int: Allow up to 16320 bits wide_int and change
> > + * widest_int precision to 32640 bits [PR102989]"), which first appeared in
> > + * GCC 14.1.  
> 
> So GCC_VERSION should be < 140100 I guess?
> 
> > + *
> > + * See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=124699
> > + */
> > +#define __const_bf_shf(x) __bf_shf((long long)(x))
> > +#else
> > +#define __const_bf_shf(x) __bf_shf(x)
> > +#endif  
> 
> The 'const' part looks misleading. There's really nothing const in
> there. Can you just fix the __bf_shf() instead?
> 
> >  #define __scalar_type_to_unsigned_cases(type)				\
> >  		unsigned type:	(unsigned type)0,			\
> >  		signed type:	(unsigned type)0
> > @@ -157,11 +181,11 @@
> >  		/* mask must be non-zero */				\
> >  		BUILD_BUG_ON_ZERO((_mask) == 0) +			\
> >  		/* check if value fits */				\
> > -		BUILD_BUG_ON_ZERO(~((_mask) >> __bf_shf(_mask)) & (_val)) + \
> > +		BUILD_BUG_ON_ZERO(~((_mask) >> __const_bf_shf(_mask)) & (_val)) + \
> >  		/* check if mask is contiguous */			\
> > -		__BF_CHECK_POW2((_mask) + (1ULL << __bf_shf(_mask))) +	\
> > +		__BF_CHECK_POW2((_mask) + (1ULL << __const_bf_shf(_mask))) + \
> >  		/* and create the value */				\
> > -		(((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask))	\
> > +		(((typeof(_mask))(_val) << __const_bf_shf(_mask)) & (_mask)) \
> >  	)  
> 
> Now you fix FIELD_PREP_CONST() only, but there might be other
> potential victims, so fixing __bf_shf() would help them all.
> 
> Thanks,
> Yury
>   
> >  /**
> > 
> > ---
> > base-commit: b20a9b5f9c4baeae0b2e143046b195b910c59714
> > change-id: 20260324-field-prep-fix-fdfc4eb68156  
> 


  reply	other threads:[~2026-04-10  9:10 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-09 14:57 [PATCH] bitfield: Fix FIELD_PREP_CONST() with __GENMASK_ULL() on gcc<14 Matt Coster
2026-04-09 17:46 ` Yury Norov
2026-04-10  9:09   ` David Laight [this message]
2026-04-09 18:48 ` David Laight

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=20260410100957.7f9bba98@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=alessio.belle@imgtec.com \
    --cc=alexandru.dadu@imgtec.com \
    --cc=brajesh.gupta@imgtec.com \
    --cc=frank.binns@imgtec.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=lkp@intel.com \
    --cc=mailhol@kernel.org \
    --cc=matt.coster@imgtec.com \
    --cc=ynorov@nvidia.com \
    --cc=yury.norov@gmail.com \
    /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