All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Richard Biener <richard.guenther@gmail.com>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>,
	Siddhesh Poyarekar <siddhesh@gotplt.org>,
	Kees Cook <kees@kernel.org>,
	gcc-patches@gcc.gnu.org, Martin Uecker <uecker@tugraz.at>,
	josmyers@redhat.com, Bill Wendling <morbo@google.com>,
	linux-hardening@vger.kernel.org
Subject: Re: [PATCH] tree-object-size: Fix type-1 size for pointers to FAM-containing subobjects [PR126975]
Date: Fri, 21 Aug 2026 11:06:39 +0200	[thread overview]
Message-ID: <aogVH8UtICMP1Ihv@tucnak> (raw)
In-Reply-To: <CAFiYyc2Uhw0iCwfmCET7gWLnXXYeV7wwPJv9Ny0uEcOrYA2f4A@mail.gmail.com>

On Fri, Aug 21, 2026 at 10:56:34AM +0200, Richard Biener wrote:
> On Fri, Aug 21, 2026 at 8:30 AM Gustavo A. R. Silva
> <gustavoars@kernel.org> wrote:
> >
> > For a pointer to a subobject whose record/union type ends in a flexible-array
> > member (directly, or through its trailing nested struct), addr_object_size()
> > walked up to the enclosing object (v = TREE_OPERAND (v, 0)) instead of
> > measuring the referenced subobject. __builtin_object_size() and
> > __builtin_dynamic_object_size() type 1 therefore returned the whole-object
> > size, collapsing type 1 onto type 0 and losing the distinction between
> > &p->inner and p. FORTIFY_SOURCE relies on the type-1 distinction, so this
> > weakens its bounds checks.
> >
> > Fix this by computing the size directly from the referenced record/union
> > instead of walking up, restoring the type-0/type-1 distinction that Clang
> > already implements.
> >
> > Bootstraped and regtested on x86_64-linux-gnu.
> >
> >         PR tree-optimization/126975
> >
> > gcc/ChangeLog:
> >
> >         * tree-object-size.cc (addr_object_size): For a reference to a
> >         record or union type, compute the object size from the referenced
> >         subobject instead of walking up to the enclosing object when the
> >         type recursively includes a flexible array member.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * gcc.dg/builtin-object-size-pr101832.c (main): Update the expected
> >         results of __builtin_object_size (..., 1) queries on subobjects
> >         whose type contains a flexible array member.
> > ---
> >  .../gcc.dg/builtin-object-size-pr101832.c      |  6 +++---
> >  gcc/tree-object-size.cc                        | 18 +++++-------------
> >  2 files changed, 8 insertions(+), 16 deletions(-)
> >
> > diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
> > index 60078e11634..d76286ae454 100644
> > --- a/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
> > +++ b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
> > @@ -92,9 +92,9 @@ int main (int argc, char *argv[])
> >    outer = (void *)magic1;
> >    outest = (void *)magic2;
> >
> > -  expect (__builtin_object_size (&outer->a, 1), -1);
> > -  expect (__builtin_object_size (&outest->b, 1), -1);
> > -  expect (__builtin_object_size (&outest->b.a, 1), -1);
> > +  expect (__builtin_object_size (&outer->a, 1), sizeof(outer->a));
> 
> I think this warrants updating the documentation in extend.texi where
> I think the
> question is how for
> 
> struct A {
>   int n;
>   char data[];
> };
> 
> struct B {
>   int m;
>   struct A a;
> };
> 
> structr B *outer;
> 
> and object &outer->a, how, based on 'type', the enclosing object
> size is constrained by its enclosing object.  Also for outer->a.data
> what the enclosing object is and similar how the constraint from the
> enclosing objects are handled.
> 
> I'll note that __builtin_object_size behavior might be in conflict
> with constraints set by -fstrict-flex-arrays.  IIRC "nested" flex arrays
> are a GNU extension, flex arrays not at the end of an (enclusing)
> object as well (a particularly bad one).
> 
> You also need to update the toplevel comment of the testcase
> which explicitly says your change is wrong.

I actually think we don't want to change this, the handling of nested
flexible and flexible-like arrays has been completely intentional, it was
based on investigation of real-world code and finding some reasonable
middle-ground on what we consider already invalid and what we allow as an
extension, especially because it occurs a lot in real-world code.

What we certainly can change is the behavior when
-fstrict-flex-arrays unless it already behaves the expected way,
and perhaps also when the flex array has counted_by attribute.

Changing this unconditionally will just break a lot of software in the wild
when compiled with -O2 -D_FORTIFY_SOURCE=2 or -O2 -D_FORTIFY_SOURCE=3, which
is very common.

	Jakub


      reply	other threads:[~2026-08-21  9:07 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-21 21:29 [PATCH] tree-object-size: Fix type-1 size for pointers to FAM-containing subobjects [PR126975] Gustavo A. R. Silva
2026-08-21  8:56 ` Richard Biener
2026-08-21  9:06   ` Jakub Jelinek [this message]

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=aogVH8UtICMP1Ihv@tucnak \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=gustavoars@kernel.org \
    --cc=josmyers@redhat.com \
    --cc=kees@kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=morbo@google.com \
    --cc=richard.guenther@gmail.com \
    --cc=siddhesh@gotplt.org \
    --cc=uecker@tugraz.at \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.