Re: [PATCH v2] overflow: Match arithmetic in both parts of __builtin_choose_expr

[David Laight <david.laight.linux@gmail.com>]

On Wed, 14 Oct 2026 13:39:03 -0700
Bill Wendling <morbo@google.com> wrote:

> On Wed, Oct 14, 2026 at 5:38 AM David Laight
> <david.laight.linux@gmail.com> wrote:
> > On Tue, 13 Oct 2026 20:31:46 +0000
> > Bill Wendling <morbo@google.com> wrote:
> >  
> > > The two parts of '__builtin_choose_expr()' had different arithmetic
> > > expressions. This patch corrects it so that both parts calculate:
> > >
> > >   COUNT * sizeof(*MEMBER) + __must_be_array(MEMBER)
> > >
> > > In practice, '__must_be_array' is meant to assert if 'MEMBER' isn't an
> > > array and return '0' otherwise. However, symmetry is less prone to
> > > subtle errors (and far more satisfying).
> > >
> > > Signed-off-by: Bill Wendling <morbo@google.com>
> > > ---
> > > v2: Don't use 'size_add', which just adds more complicated macro cruft to the
> > >     expression. Instead, just adjust the parentheses.
> > > ---
> > >  include/linux/overflow.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/overflow.h b/include/linux/overflow.h
> > > index a8cb6319b4fb..d0f04436343e 100644
> > > --- a/include/linux/overflow.h
> > > +++ b/include/linux/overflow.h
> > > @@ -418,7 +418,7 @@ static __always_inline size_t __must_check size_sub(size_t minuend, size_t subtr
> > >  #define flex_array_size(p, member, count)                            \
> > >       __builtin_choose_expr(__is_constexpr(count),                    \
> > >               (count) * sizeof(*(p)->member) + __must_be_array((p)->member),  \
> > > -             size_mul(count, sizeof(*(p)->member) + __must_be_array((p)->member)))
> > > +             size_mul(count, sizeof(*(p)->member)) + __must_be_array((p)->member))  
> >
> > Why does this use __builtin_choose_expr().
> > The types of the two expressions are the same so it might as well use ?:
> >  
> It's to emit only one of the expressions rather than both. If the
> 'count' is a constant expression, then it's possible for the compiler
> to perform the calculation without having to hold onto the "false"
> path of the ternary operator. So much more efficient in the long-run.

I doubt it.
Even with __builtin_choose_expr() the compile has to do most of the work
for compiling the 'false' path.
If you use ?: then you can also use the much simpler __builtin_constant_p()
and possibly pick up cases where it is only known to be constant later.

> 
> > Also doesn't this optimisation apply to all uses of size_mul() (aka saturating_mul()).
> > Presumably relying on the compiler to report the compilation time numeric overflow.  
> 
> I'm not familiar with saturating_mul(). I'm sure others on here can talk to it.

That is me giving is a better name.
The same is true for size_add().
IMHO saturation size_t calculations is really broken because it allows
through 'bigger than sensible' sizes that still might cause problems
(eg kmalloc() of sizes just below the limit.)

David

> 
> -bw
> 
> > David
> >  
> > >
> > >  /**
> > >   * struct_size() - Calculate size of structure with trailing flexible array.  
> >