All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] c: handle .ACCESS_WITH_SIZE in build_unary_op for !/+/- [PR123569]
@ 2026-06-18  5:12 Kees Cook
  2026-06-18  5:21 ` Martin Uecker
  0 siblings, 1 reply; 4+ messages in thread
From: Kees Cook @ 2026-06-18  5:12 UTC (permalink / raw)
  To: Martin Uecker; +Cc: Kees Cook, Richard Biener, gcc-patches, linux-hardening

PR123569 fixed wrong code with the counted_by attribute on a pointer
member by suppressing creation of the .ACCESS_WITH_SIZE wrapper in the
parser's pre/post-increment paths, and added a checking assertion in
build_unary_op to ensure no .ACCESS_WITH_SIZE call reaches it.

The other rvalue-consuming unary operators (!, -, +) still
rvalue-convert their operand via convert_lvalue_to_rvalue, which
produces the .ACCESS_WITH_SIZE wrapper as before.  They then reach
parser_build_unary_op then build_unary_op carrying the wrapper and
trip the assertion.  A minimal reproducer:

  struct s {
    int n;
    char *p __attribute__((__counted_by__(n)));
  };
  int f (struct s *o) { return !o->p; }

Unwrap the .ACCESS_WITH_SIZE call at the top of build_unary_op via
get_ref_from_access_with_size.  These unary operators consume the
pointer rvalue rather than the pointed-to data, so the bounds-checking
wrapper is not useful here.  The assertion is retained: after the unwrap
it documents the post-condition the rest of the function relies on.

	PR c/123569

gcc/c/ChangeLog:

	* c-typeck.cc (build_unary_op): Unwrap .ACCESS_WITH_SIZE
	from the operand before further processing.

gcc/testsuite/ChangeLog:

	* gcc.dg/counted-by-unary.c: New test.
---
 gcc/testsuite/gcc.dg/counted-by-unary.c | 32 +++++++++++++++++++++++++
 gcc/c/c-typeck.cc                       | 12 ++++++++++
 2 files changed, 44 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/counted-by-unary.c

diff --git a/gcc/testsuite/gcc.dg/counted-by-unary.c b/gcc/testsuite/gcc.dg/counted-by-unary.c
new file mode 100644
index 000000000000..b16b819d63b6
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/counted-by-unary.c
@@ -0,0 +1,32 @@
+/* Verify that unary operators that rvalue-convert a counted_by-annotated
+   pointer access do not ICE.  The original PR123569 fix suppressed the
+   .ACCESS_WITH_SIZE wrapper creation for ++/-- (which keep an lvalue);
+   the rvalue-consuming ops (!, -, +) still receive the wrapper and must
+   strip it in build_unary_op.  */
+/* { dg-do run } */
+/* { dg-options "-std=c99" } */
+
+struct s {
+    int n;
+    char *p __attribute__((__counted_by__(n)));
+};
+
+int test_not (struct s *o)    { return !o->p; }
+long test_neg (struct s *o)   { return -(long)o->p; }
+long test_plus (struct s *o)  { return +(long)o->p; }
+
+extern void abort (void);
+
+int main (void) {
+    char buf[4] = { 0 };
+    struct s s = { .n = 4, .p = buf };
+    if (test_not (&s) != 0)  abort ();    /* p is non-null */
+
+    struct s e = { .n = 0, .p = (void*)0 };
+    if (test_not (&e) != 1)  abort ();    /* p is null */
+
+    /* test_neg / test_plus must just compile and run without ICE.  */
+    (void) test_neg (&s);
+    (void) test_plus (&s);
+    return 0;
+}
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index 6bce8c427543..0f96de200743 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -5779,6 +5779,18 @@ build_unary_op (location_t location, enum tree_code code, tree xarg,
   tree eptype = NULL_TREE;
   const char *invalid_op_diag;
 
+  /* If ARG is wrapped in a call to .ACCESS_WITH_SIZE (created when the
+     C parser rvalue-converts a counted_by-annotated member access, see
+     PR123569) unwrap it here.  Unary operators that consume an rvalue
+     (!, -, +) read the value itself rather than dereferencing into the
+     pointed-to data, so the bounds-checking wrapper is unnecessary and
+     would otherwise reach build_unary_op while it is not equipped to
+     handle the wrapped form.  PR123569 fixed the ++/-- side of this by
+     suppressing wrapper creation in the parser; the rvalue-consuming
+     ops still receive the wrapper and need to strip it here.  */
+  if (is_access_with_size_p (arg))
+    xarg = arg = get_ref_from_access_with_size (arg);
+
   gcc_checking_assert (!is_access_with_size_p (arg));
 
   bool int_operands = EXPR_INT_CONST_OPERANDS (xarg);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] c: handle .ACCESS_WITH_SIZE in build_unary_op for !/+/- [PR123569]
  2026-06-18  5:12 [PATCH] c: handle .ACCESS_WITH_SIZE in build_unary_op for !/+/- [PR123569] Kees Cook
@ 2026-06-18  5:21 ` Martin Uecker
  2026-06-18  5:30   ` Martin Uecker
  0 siblings, 1 reply; 4+ messages in thread
From: Martin Uecker @ 2026-06-18  5:21 UTC (permalink / raw)
  To: Kees Cook; +Cc: Richard Biener, gcc-patches, linux-hardening, Joseph Myers


I was about to fix this but did not have time yet, so thank you!

To me the patch looks fine. I add Joseph (C FE maintainer)
for review.

Martin

Am Mittwoch, dem 17.06.2026 um 22:12 -0700 schrieb Kees Cook:
> PR123569 fixed wrong code with the counted_by attribute on a pointer
> member by suppressing creation of the .ACCESS_WITH_SIZE wrapper in the
> parser's pre/post-increment paths, and added a checking assertion in
> build_unary_op to ensure no .ACCESS_WITH_SIZE call reaches it.
> 
> The other rvalue-consuming unary operators (!, -, +) still
> rvalue-convert their operand via convert_lvalue_to_rvalue, which
> produces the .ACCESS_WITH_SIZE wrapper as before.  They then reach
> parser_build_unary_op then build_unary_op carrying the wrapper and
> trip the assertion.  A minimal reproducer:
> 
>   struct s {
>     int n;
>     char *p __attribute__((__counted_by__(n)));
>   };
>   int f (struct s *o) { return !o->p; }
> 
> Unwrap the .ACCESS_WITH_SIZE call at the top of build_unary_op via
> get_ref_from_access_with_size.  These unary operators consume the
> pointer rvalue rather than the pointed-to data, so the bounds-checking
> wrapper is not useful here.  The assertion is retained: after the unwrap
> it documents the post-condition the rest of the function relies on.
> 
> 	PR c/123569
> 
> gcc/c/ChangeLog:
> 
> 	* c-typeck.cc (build_unary_op): Unwrap .ACCESS_WITH_SIZE
> 	from the operand before further processing.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/counted-by-unary.c: New test.
> ---
>  gcc/testsuite/gcc.dg/counted-by-unary.c | 32 +++++++++++++++++++++++++
>  gcc/c/c-typeck.cc                       | 12 ++++++++++
>  2 files changed, 44 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.dg/counted-by-unary.c
> 
> diff --git a/gcc/testsuite/gcc.dg/counted-by-unary.c b/gcc/testsuite/gcc.dg/counted-by-unary.c
> new file mode 100644
> index 000000000000..b16b819d63b6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/counted-by-unary.c
> @@ -0,0 +1,32 @@
> +/* Verify that unary operators that rvalue-convert a counted_by-annotated
> +   pointer access do not ICE.  The original PR123569 fix suppressed the
> +   .ACCESS_WITH_SIZE wrapper creation for ++/-- (which keep an lvalue);
> +   the rvalue-consuming ops (!, -, +) still receive the wrapper and must
> +   strip it in build_unary_op.  */
> +/* { dg-do run } */
> +/* { dg-options "-std=c99" } */
> +
> +struct s {
> +    int n;
> +    char *p __attribute__((__counted_by__(n)));
> +};
> +
> +int test_not (struct s *o)    { return !o->p; }
> +long test_neg (struct s *o)   { return -(long)o->p; }
> +long test_plus (struct s *o)  { return +(long)o->p; }
> +
> +extern void abort (void);
> +
> +int main (void) {
> +    char buf[4] = { 0 };
> +    struct s s = { .n = 4, .p = buf };
> +    if (test_not (&s) != 0)  abort ();    /* p is non-null */
> +
> +    struct s e = { .n = 0, .p = (void*)0 };
> +    if (test_not (&e) != 1)  abort ();    /* p is null */
> +
> +    /* test_neg / test_plus must just compile and run without ICE.  */
> +    (void) test_neg (&s);
> +    (void) test_plus (&s);
> +    return 0;
> +}
> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
> index 6bce8c427543..0f96de200743 100644
> --- a/gcc/c/c-typeck.cc
> +++ b/gcc/c/c-typeck.cc
> @@ -5779,6 +5779,18 @@ build_unary_op (location_t location, enum tree_code code, tree xarg,
>    tree eptype = NULL_TREE;
>    const char *invalid_op_diag;
>  
> +  /* If ARG is wrapped in a call to .ACCESS_WITH_SIZE (created when the
> +     C parser rvalue-converts a counted_by-annotated member access, see
> +     PR123569) unwrap it here.  Unary operators that consume an rvalue
> +     (!, -, +) read the value itself rather than dereferencing into the
> +     pointed-to data, so the bounds-checking wrapper is unnecessary and
> +     would otherwise reach build_unary_op while it is not equipped to
> +     handle the wrapped form.  PR123569 fixed the ++/-- side of this by
> +     suppressing wrapper creation in the parser; the rvalue-consuming
> +     ops still receive the wrapper and need to strip it here.  */
> +  if (is_access_with_size_p (arg))
> +    xarg = arg = get_ref_from_access_with_size (arg);
> +
>    gcc_checking_assert (!is_access_with_size_p (arg));
>  
>    bool int_operands = EXPR_INT_CONST_OPERANDS (xarg);

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] c: handle .ACCESS_WITH_SIZE in build_unary_op for !/+/- [PR123569]
  2026-06-18  5:21 ` Martin Uecker
@ 2026-06-18  5:30   ` Martin Uecker
  2026-06-18 11:54     ` Joseph Myers
  0 siblings, 1 reply; 4+ messages in thread
From: Martin Uecker @ 2026-06-18  5:30 UTC (permalink / raw)
  To: Kees Cook; +Cc: Richard Biener, gcc-patches, linux-hardening, Joseph Myers


It should reference PR125604 though.

Am Donnerstag, dem 18.06.2026 um 07:21 +0200 schrieb Martin Uecker:
> I was about to fix this but did not have time yet, so thank you!
> 
> To me the patch looks fine. I add Joseph (C FE maintainer)
> for review.
> 
> Martin
> 
> Am Mittwoch, dem 17.06.2026 um 22:12 -0700 schrieb Kees Cook:
> > PR123569 fixed wrong code with the counted_by attribute on a pointer
> > member by suppressing creation of the .ACCESS_WITH_SIZE wrapper in the
> > parser's pre/post-increment paths, and added a checking assertion in
> > build_unary_op to ensure no .ACCESS_WITH_SIZE call reaches it.
> > 
> > The other rvalue-consuming unary operators (!, -, +) still
> > rvalue-convert their operand via convert_lvalue_to_rvalue, which
> > produces the .ACCESS_WITH_SIZE wrapper as before.  They then reach
> > parser_build_unary_op then build_unary_op carrying the wrapper and
> > trip the assertion.  A minimal reproducer:
> > 
> >   struct s {
> >     int n;
> >     char *p __attribute__((__counted_by__(n)));
> >   };
> >   int f (struct s *o) { return !o->p; }
> > 
> > Unwrap the .ACCESS_WITH_SIZE call at the top of build_unary_op via
> > get_ref_from_access_with_size.  These unary operators consume the
> > pointer rvalue rather than the pointed-to data, so the bounds-checking
> > wrapper is not useful here.  The assertion is retained: after the unwrap
> > it documents the post-condition the rest of the function relies on.
> > 
> > 	PR c/123569
> > 
> > gcc/c/ChangeLog:
> > 
> > 	* c-typeck.cc (build_unary_op): Unwrap .ACCESS_WITH_SIZE
> > 	from the operand before further processing.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* gcc.dg/counted-by-unary.c: New test.
> > ---
> >  gcc/testsuite/gcc.dg/counted-by-unary.c | 32 +++++++++++++++++++++++++
> >  gcc/c/c-typeck.cc                       | 12 ++++++++++
> >  2 files changed, 44 insertions(+)
> >  create mode 100644 gcc/testsuite/gcc.dg/counted-by-unary.c
> > 
> > diff --git a/gcc/testsuite/gcc.dg/counted-by-unary.c b/gcc/testsuite/gcc.dg/counted-by-unary.c
> > new file mode 100644
> > index 000000000000..b16b819d63b6
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/counted-by-unary.c
> > @@ -0,0 +1,32 @@
> > +/* Verify that unary operators that rvalue-convert a counted_by-annotated
> > +   pointer access do not ICE.  The original PR123569 fix suppressed the
> > +   .ACCESS_WITH_SIZE wrapper creation for ++/-- (which keep an lvalue);
> > +   the rvalue-consuming ops (!, -, +) still receive the wrapper and must
> > +   strip it in build_unary_op.  */
> > +/* { dg-do run } */
> > +/* { dg-options "-std=c99" } */
> > +
> > +struct s {
> > +    int n;
> > +    char *p __attribute__((__counted_by__(n)));
> > +};
> > +
> > +int test_not (struct s *o)    { return !o->p; }
> > +long test_neg (struct s *o)   { return -(long)o->p; }
> > +long test_plus (struct s *o)  { return +(long)o->p; }
> > +
> > +extern void abort (void);
> > +
> > +int main (void) {
> > +    char buf[4] = { 0 };
> > +    struct s s = { .n = 4, .p = buf };
> > +    if (test_not (&s) != 0)  abort ();    /* p is non-null */
> > +
> > +    struct s e = { .n = 0, .p = (void*)0 };
> > +    if (test_not (&e) != 1)  abort ();    /* p is null */
> > +
> > +    /* test_neg / test_plus must just compile and run without ICE.  */
> > +    (void) test_neg (&s);
> > +    (void) test_plus (&s);
> > +    return 0;
> > +}
> > diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
> > index 6bce8c427543..0f96de200743 100644
> > --- a/gcc/c/c-typeck.cc
> > +++ b/gcc/c/c-typeck.cc
> > @@ -5779,6 +5779,18 @@ build_unary_op (location_t location, enum tree_code code, tree xarg,
> >    tree eptype = NULL_TREE;
> >    const char *invalid_op_diag;
> >  
> > +  /* If ARG is wrapped in a call to .ACCESS_WITH_SIZE (created when the
> > +     C parser rvalue-converts a counted_by-annotated member access, see
> > +     PR123569) unwrap it here.  Unary operators that consume an rvalue
> > +     (!, -, +) read the value itself rather than dereferencing into the
> > +     pointed-to data, so the bounds-checking wrapper is unnecessary and
> > +     would otherwise reach build_unary_op while it is not equipped to
> > +     handle the wrapped form.  PR123569 fixed the ++/-- side of this by
> > +     suppressing wrapper creation in the parser; the rvalue-consuming
> > +     ops still receive the wrapper and need to strip it here.  */
> > +  if (is_access_with_size_p (arg))
> > +    xarg = arg = get_ref_from_access_with_size (arg);
> > +
> >    gcc_checking_assert (!is_access_with_size_p (arg));
> >  
> >    bool int_operands = EXPR_INT_CONST_OPERANDS (xarg);

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] c: handle .ACCESS_WITH_SIZE in build_unary_op for !/+/- [PR123569]
  2026-06-18  5:30   ` Martin Uecker
@ 2026-06-18 11:54     ` Joseph Myers
  0 siblings, 0 replies; 4+ messages in thread
From: Joseph Myers @ 2026-06-18 11:54 UTC (permalink / raw)
  To: Martin Uecker; +Cc: Kees Cook, Richard Biener, gcc-patches, linux-hardening

On Thu, 18 Jun 2026, Martin Uecker wrote:

> It should reference PR125604 though.
> 
> Am Donnerstag, dem 18.06.2026 um 07:21 +0200 schrieb Martin Uecker:
> > I was about to fix this but did not have time yet, so thank you!
> > 
> > To me the patch looks fine. I add Joseph (C FE maintainer)
> > for review.

The patch is OK with that bug reference, and with a portability fix in the 
testcase:

> > > +long test_neg (struct s *o)   { return -(long)o->p; }
> > > +long test_plus (struct s *o)  { return +(long)o->p; }

Should use __INTPTR_TYPE__ instead of long here.

-- 
Joseph S. Myers
josmyers@redhat.com


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-06-18 11:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-18  5:12 [PATCH] c: handle .ACCESS_WITH_SIZE in build_unary_op for !/+/- [PR123569] Kees Cook
2026-06-18  5:21 ` Martin Uecker
2026-06-18  5:30   ` Martin Uecker
2026-06-18 11:54     ` Joseph Myers

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.