All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] c: handle .ACCESS_WITH_SIZE in build_unary_op for !/+/- [PR125604]
@ 2026-06-18 19:56 Kees Cook
  2026-06-18 20:29 ` Kees Cook
  0 siblings, 1 reply; 2+ messages in thread
From: Kees Cook @ 2026-06-18 19:56 UTC (permalink / raw)
  To: Joseph Myers
  Cc: Kees Cook, Richard Biener, Martin Uecker, 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 load-bearing here.  The assertion is retained: after
the unwrap it documents the post-condition the rest of the function
relies on.

	PR c/125604

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..65ede4c0dfd8
--- /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; }
+__INTPTR_TYPE__ test_neg (struct s *o)   { return -(__INTPTR_TYPE__)o->p; }
+__INTPTR_TYPE__ test_plus (struct s *o)  { return +(__INTPTR_TYPE__)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] 2+ messages in thread

* Re: [PATCH v2] c: handle .ACCESS_WITH_SIZE in build_unary_op for !/+/- [PR125604]
  2026-06-18 19:56 [PATCH v2] c: handle .ACCESS_WITH_SIZE in build_unary_op for !/+/- [PR125604] Kees Cook
@ 2026-06-18 20:29 ` Kees Cook
  0 siblings, 0 replies; 2+ messages in thread
From: Kees Cook @ 2026-06-18 20:29 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Richard Biener, Martin Uecker, gcc-patches, linux-hardening

On Thu, Jun 18, 2026 at 12:56:51PM -0700, Kees Cook wrote:
> 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 load-bearing here.  The assertion is retained: after
> the unwrap it documents the post-condition the rest of the function
> relies on.

BTW, I don't have commit access, so if this patch looks good, I still
need someone to commit for me. :)

-- 
Kees Cook

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

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

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-18 19:56 [PATCH v2] c: handle .ACCESS_WITH_SIZE in build_unary_op for !/+/- [PR125604] Kees Cook
2026-06-18 20:29 ` Kees Cook

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.