diff mbox series

preserve integer value of local addresses returned from functions (PR 90737)

Message ID 421adf0e-440c-d505-4408-9e418c5bb250@gmail.com
State New
Headers show
Series preserve integer value of local addresses returned from functions (PR 90737) | expand

Commit Message

Martin Sebor June 3, 2019, 9:24 p.m. UTC 
While testing a different -Wreturn-local-addr bug fix/enhancement
I noticed that in functions that return integers as opposed to
pointers such as:

   intptr_t f (int i) { return (intptr_t)&i; }

the converted address is folded to zero.  This can be detected
by strictly conforming programs so it's not really correct.
Such statements also trigger the warning.

The attached patch adjusts the C and C++ front-ends to avoid
the folding.

The patch also avoids the warning but I'm on the fence about that.
There is some value in diagnosing it since it could be masking
a bug.  Would anyone like to argue in favor of keeping it?

Martin

Comments

Jeff Law June 5, 2019, 10:39 p.m. UTC | #1 
On 6/3/19 3:24 PM, Martin Sebor wrote:
> While testing a different -Wreturn-local-addr bug fix/enhancement
> I noticed that in functions that return integers as opposed to
> pointers such as:
> 
>   intptr_t f (int i) { return (intptr_t)&i; }
> 
> the converted address is folded to zero.  This can be detected
> by strictly conforming programs so it's not really correct.
> Such statements also trigger the warning.
> 
> The attached patch adjusts the C and C++ front-ends to avoid
> the folding.
> 
> The patch also avoids the warning but I'm on the fence about that.
> There is some value in diagnosing it since it could be masking
> a bug.  Would anyone like to argue in favor of keeping it?
> 
> Martin
> 
> gcc-90737.diff
> 
> PR c/90737 - [8/9/10 Regression] inconsistent address of a local converted to intptr_t between callee and caller
> 
> gcc/c/ChangeLog:
> 
> 	PR c/90737
> 	* c-typeck.c (c_finish_return): Only consider functions returning
> 	pointers as candidates for -Wreturn-local-addr.
> 
> gcc/cp/ChangeLog:
> 
> 	PR c/90737
> 	* typeck.c (maybe_warn_about_returning_address_of_local): Only
> 	consider functions returning pointers as candidates for
> 	-Wreturn-local-addr.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c/90737
> 	* c-c++-common/Wreturn-local-addr.c: New test.
> 	* g++.dg/warn/Wreturn-local-addr-6.C: New test.
OK
jeff
diff mbox series

Patch

PR c/90737 - [8/9/10 Regression] inconsistent address of a local converted to intptr_t between callee and caller

gcc/c/ChangeLog:

	PR c/90737
	* c-typeck.c (c_finish_return): Only consider functions returning
	pointers as candidates for -Wreturn-local-addr.

gcc/cp/ChangeLog:

	PR c/90737
	* typeck.c (maybe_warn_about_returning_address_of_local): Only
	consider functions returning pointers as candidates for
	-Wreturn-local-addr.

gcc/testsuite/ChangeLog:

	PR c/90737
	* c-c++-common/Wreturn-local-addr.c: New test.
	* g++.dg/warn/Wreturn-local-addr-6.C: New test.

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 5eff040e85d..0dd86f074b9 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -10628,7 +10628,8 @@  c_finish_return (location_t loc, tree retval, tree origtype)
 	      if (DECL_P (inner)
 		  && !DECL_EXTERNAL (inner)
 		  && !TREE_STATIC (inner)
-		  && DECL_CONTEXT (inner) == current_function_decl)
+		  && DECL_CONTEXT (inner) == current_function_decl
+		  && POINTER_TYPE_P (TREE_TYPE (TREE_TYPE (current_function_decl))))
 		{
 		  if (TREE_CODE (inner) == LABEL_DECL)
 		    warning_at (loc, OPT_Wreturn_local_addr,
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 7289f2c49fc..585b38e1cd9 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -9305,11 +9305,12 @@  maybe_warn_about_returning_address_of_local (tree retval)
 			"returning local %<initializer_list%> variable %qD "
 			"does not extend the lifetime of the underlying array",
 			whats_returned);
-      else if (TREE_CODE (whats_returned) == LABEL_DECL)
+      else if (POINTER_TYPE_P (valtype)
+	       && TREE_CODE (whats_returned) == LABEL_DECL)
 	w = warning_at (loc, OPT_Wreturn_local_addr,
 			"address of label %qD returned",
 			whats_returned);
-      else
+      else if (POINTER_TYPE_P (valtype))
 	w = warning_at (loc, OPT_Wreturn_local_addr,
 			"address of local variable %qD returned",
 			whats_returned);
diff --git a/gcc/testsuite/c-c++-common/Wreturn-local-addr.c b/gcc/testsuite/c-c++-common/Wreturn-local-addr.c
new file mode 100644
index 00000000000..c8c3b900291
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wreturn-local-addr.c
@@ -0,0 +1,47 @@ 
+/* PR c/90737 - inconsistent address of a local converted to intptr_t
+   between callee and caller
+   { dg-do compile }
+   { dg-options "-O1 -Wall -Wreturn-local-addr -fdump-tree-optimized" } */
+
+typedef __INTPTR_TYPE__ intptr_t;
+
+static inline intptr_t
+return_addr_local_as_int (void)
+{
+  int i;
+  if ((intptr_t)&i == 0)
+    __builtin_abort ();
+
+  return (intptr_t)&i;
+}
+
+void get_addr_local_as_int (void)
+{
+  intptr_t i = return_addr_local_as_int ();
+  if (i == 0)
+    __builtin_abort ();
+}
+
+
+static inline intptr_t
+return_addr_label_as_int (void)
+{
+ label:
+  if ((intptr_t)&&label == 0)
+    __builtin_abort ();
+
+  return (intptr_t)&&label;
+}
+
+void get_addr_label_as_int (void)
+{
+  intptr_t i = return_addr_label_as_int ();
+  if (i == 0)
+    __builtin_abort ();
+}
+
+/* Verify that the functions that return the address of the label
+   or local variable have been optimized away and so have the calls
+   to abort.
+  { dg-final { scan-tree-dump-not "return_addr_" "optimized" } }
+  { dg-final { scan-tree-dump-not "abort" "optimized" } } */
diff --git a/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C b/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C
new file mode 100644
index 00000000000..bfe14457547
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C
@@ -0,0 +1,29 @@ 
+/* PR c/90737 - inconsistent address of a local converted to intptr_t
+   between callee and caller
+   { dg-do compile }
+   { dg-options "-O1 -Wall -Wreturn-local-addr -fdump-tree-optimized" } */
+
+typedef __INTPTR_TYPE__ intptr_t;
+
+const intptr_t&
+return_addr_label_as_intref (void)
+{
+ label:
+  if ((const intptr_t*)&&label == 0)
+    __builtin_exit (1);
+
+  return *(const intptr_t*)&&label;   // { dg-warning "\\\[-Wreturn-local-addr]" } */
+}
+
+const intptr_t&
+return_addr_local_as_intref (void)
+{
+  int a[1];
+  if ((const intptr_t*)a == 0)
+    __builtin_exit (1);
+
+  return (const intptr_t&)a;   // { dg-warning "\\\[-Wreturn-local-addr]" } */
+}
+
+/* Verify that the return value has been replaced with zero:
+  { dg-final { scan-tree-dump-times "return 0;" 2 "optimized" } } */