public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Nathaniel Shead <nathanieloshead@gmail.com>
To: gcc-patches@gcc.gnu.org
Cc: Jason Merrill <jason@redhat.com>, Patrick Palka <ppalka@redhat.com>
Subject: [PATCH v5 2/3] c++: Prevent dangling pointers from becoming nullptr in constexpr [PR110619]
Date: Sun, 23 Jul 2023 01:14:37 +1000	[thread overview]
Message-ID: <ZLvyXbXiHvhfAjpu@Thaum.localdomain> (raw)
In-Reply-To: <ZLvx7bNUL0KLo+Iq@Thaum.localdomain>

Currently, when typeck discovers that a return statement will refer to a
local variable it rewrites to return a null pointer. This causes the
error messages for using the return value in a constant expression to be
unhelpful, especially for reference return values, and is also a visible
change to otherwise valid code (as in the linked PR).

The transformation is nonetheless important, however, both as a safety
guard against attackers being able to gain a handle to other data on the
stack, and to prevent duplicate warnings from later null-dereference
warning passes.

As such, this patch just delays the transformation until cp_genericize,
after constexpr function definitions have been generated.

	PR c++/110619

gcc/cp/ChangeLog:

	* cp-gimplify.cc (cp_genericize_r): Transform RETURN_EXPRs to
	not return dangling pointers.
	* cp-tree.h (RETURN_EXPR_LOCAL_ADDR_P): New flag.
	(check_return_expr): Add a new parameter.
	* semantics.cc (finish_return_stmt): Set flag on RETURN_EXPR
	when referring to dangling pointer.
	* typeck.cc (check_return_expr): Disable transformation of
	dangling pointers, instead pass this information to caller.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp1y/constexpr-110619.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/cp-gimplify.cc                         | 23 ++++++++++++++++---
 gcc/cp/cp-tree.h                              |  8 ++++++-
 gcc/cp/semantics.cc                           |  4 +++-
 gcc/cp/typeck.cc                              |  9 ++++----
 gcc/testsuite/g++.dg/cpp1y/constexpr-110619.C | 10 ++++++++
 5 files changed, 45 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-110619.C

diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index f5734197774..0a5d6300aca 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -1336,9 +1336,26 @@ cp_genericize_r (tree *stmt_p, int *walk_subtrees, void *data)
       break;
 
     case RETURN_EXPR:
-      if (TREE_OPERAND (stmt, 0) && is_invisiref_parm (TREE_OPERAND (stmt, 0)))
-	/* Don't dereference an invisiref RESULT_DECL inside a RETURN_EXPR.  */
-	*walk_subtrees = 0;
+      if (TREE_OPERAND (stmt, 0))
+	{
+	  if (is_invisiref_parm (TREE_OPERAND (stmt, 0)))
+	    /* Don't dereference an invisiref RESULT_DECL inside a RETURN_EXPR.  */
+	    *walk_subtrees = 0;
+	  if (RETURN_EXPR_LOCAL_ADDR_P (stmt))
+	    {
+	      /* Don't return the address of a local variable.  */
+	      tree *p = &TREE_OPERAND (stmt, 0);
+	      while (TREE_CODE (*p) == COMPOUND_EXPR)
+		p = &TREE_OPERAND (*p, 0);
+	      if (TREE_CODE (*p) == INIT_EXPR)
+		{
+		  tree op = TREE_OPERAND (*p, 1);
+		  tree new_op = build2 (COMPOUND_EXPR, TREE_TYPE (op), op,
+					build_zero_cst (TREE_TYPE (op)));
+		  TREE_OPERAND (*p, 1) = new_op;
+		}
+	    }
+	}
       break;
 
     case OMP_CLAUSE:
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 3de0e154c12..e0c181d9aef 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -447,6 +447,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
       INIT_EXPR_NRV_P (in INIT_EXPR)
       ATOMIC_CONSTR_MAP_INSTANTIATED_P (in ATOMIC_CONSTR)
       contract_semantic (in ASSERTION_, PRECONDITION_, POSTCONDITION_STMT)
+      RETURN_EXPR_LOCAL_ADDR_P (in RETURN_EXPR)
    1: IDENTIFIER_KIND_BIT_1 (in IDENTIFIER_NODE)
       TI_PENDING_TEMPLATE_FLAG.
       TEMPLATE_PARMS_FOR_INLINE.
@@ -4071,6 +4072,11 @@ struct GTY(()) lang_decl {
   (LANG_DECL_FN_CHECK (FUNCTION_DECL_CHECK (NODE))	\
    ->u.saved_auto_return_type)
 
+/* In a RETURN_EXPR, whether the expression refers to the address
+   of a local variable.  */
+#define RETURN_EXPR_LOCAL_ADDR_P(NODE) \
+  TREE_LANG_FLAG_0 (RETURN_EXPR_CHECK (NODE))
+
 /* True if NODE is an implicit INDIRECT_REF from convert_from_reference.  */
 #define REFERENCE_REF_P(NODE)				\
   (INDIRECT_REF_P (NODE)				\
@@ -8139,7 +8145,7 @@ extern tree composite_pointer_type		(const op_location_t &,
 						 tsubst_flags_t);
 extern tree merge_types				(tree, tree);
 extern tree strip_array_domain			(tree);
-extern tree check_return_expr			(tree, bool *);
+extern tree check_return_expr			(tree, bool *, bool *);
 extern tree spaceship_type			(tree, tsubst_flags_t = tf_warning_or_error);
 extern tree genericize_spaceship		(location_t, tree, tree, tree);
 extern tree cp_build_binary_op                  (const op_location_t &,
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 8fb47fd179e..720521b7f1a 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -1240,8 +1240,9 @@ finish_return_stmt (tree expr)
 {
   tree r;
   bool no_warning;
+  bool dangling;
 
-  expr = check_return_expr (expr, &no_warning);
+  expr = check_return_expr (expr, &no_warning, &dangling);
 
   if (error_operand_p (expr)
       || (flag_openmp && !check_omp_return ()))
@@ -1259,6 +1260,7 @@ finish_return_stmt (tree expr)
     }
 
   r = build_stmt (input_location, RETURN_EXPR, expr);
+  RETURN_EXPR_LOCAL_ADDR_P (r) = dangling;
   if (no_warning)
     suppress_warning (r, OPT_Wreturn_type);
   r = maybe_cleanup_point_expr_void (r);
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 859b133a18d..d5c0c85ed51 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -10920,10 +10920,11 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p)
    change RETVAL into the function return type, and to assign it to
    the DECL_RESULT for the function.  Set *NO_WARNING to true if
    code reaches end of non-void function warning shouldn't be issued
-   on this RETURN_EXPR.  */
+   on this RETURN_EXPR.  Set *DANGLING to true if code returns the
+   address of a local variable.  */
 
 tree
-check_return_expr (tree retval, bool *no_warning)
+check_return_expr (tree retval, bool *no_warning, bool *dangling)
 {
   tree result;
   /* The type actually returned by the function.  */
@@ -10935,6 +10936,7 @@ check_return_expr (tree retval, bool *no_warning)
   location_t loc = cp_expr_loc_or_input_loc (retval);
 
   *no_warning = false;
+  *dangling = false;
 
   /* A `volatile' function is one that isn't supposed to return, ever.
      (This is a G++ extension, used to get better code for functions
@@ -11273,8 +11275,7 @@ check_return_expr (tree retval, bool *no_warning)
       else if (!processing_template_decl
 	       && maybe_warn_about_returning_address_of_local (retval, loc)
 	       && INDIRECT_TYPE_P (valtype))
-	retval = build2 (COMPOUND_EXPR, TREE_TYPE (retval), retval,
-			 build_zero_cst (TREE_TYPE (retval)));
+	*dangling = true;
     }
 
   /* A naive attempt to reduce the number of -Wdangling-reference false
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-110619.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-110619.C
new file mode 100644
index 00000000000..cca13302238
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-110619.C
@@ -0,0 +1,10 @@
+// { dg-do compile { target c++14 } }
+// { dg-options "-Wno-return-local-addr" }
+// PR c++/110619
+
+constexpr auto f() {
+    int i = 0;
+    return &i;
+};
+
+static_assert( f() != nullptr );
-- 
2.41.0


  parent reply	other threads:[~2023-07-22 15:14 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-22 15:12 [PATCH v5 0/3] c++: Track lifetimes in constant evaluation [PR70331, ...] Nathaniel Shead
2023-07-22 15:13 ` [PATCH v5 1/3] c++: Improve location information in constant evaluation Nathaniel Shead
2023-07-22 15:14 ` Nathaniel Shead [this message]
2023-07-22 15:15 ` [PATCH v5 3/3] c++: Track lifetimes in constant evaluation [PR70331,PR96630,PR98675] Nathaniel Shead
2023-07-26  1:53 ` [PATCH v5 0/3] c++: Track lifetimes in constant evaluation [PR70331, ...] Jason Merrill

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=ZLvyXbXiHvhfAjpu@Thaum.localdomain \
    --to=nathanieloshead@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    --cc=ppalka@redhat.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).