All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <kees@kernel.org>
To: Martin Uecker <uecker@tugraz.at>
Cc: Kees Cook <kees@kernel.org>, Richard Biener <rguenther@suse.de>,
	gcc-patches@gcc.gnu.org, linux-hardening@vger.kernel.org
Subject: [PATCH] c: handle .ACCESS_WITH_SIZE in build_unary_op for !/+/- [PR123569]
Date: Wed, 17 Jun 2026 22:12:51 -0700	[thread overview]
Message-ID: <20260618051246.work.269-kees@kernel.org> (raw)

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


             reply	other threads:[~2026-06-18  5:12 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-18  5:12 Kees Cook [this message]
2026-06-18  5:21 ` [PATCH] c: handle .ACCESS_WITH_SIZE in build_unary_op for !/+/- [PR123569] Martin Uecker
2026-06-18  5:30   ` Martin Uecker
2026-06-18 11:54     ` Joseph Myers

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=20260618051246.work.269-kees@kernel.org \
    --to=kees@kernel.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=rguenther@suse.de \
    --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.