* [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.