Bug 90737 - [8 Regression] inconsistent address of a local converted to intptr_t between callee and caller
Summary: [8 Regression] inconsistent address of a local converted to intptr_t between ...
Status: ASSIGNED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 9.0
: P2 normal
Target Milestone: 8.5
Assignee: Martin Sebor
URL:
Keywords: diagnostic, patch, wrong-code
Depends on:
Blocks: Wreturn-local-addr
  Show dependency treegraph
 
Reported: 2019-06-03 19:07 UTC by Martin Sebor
Modified: 2020-03-04 09:32 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Known to work: 4.9.4
Known to fail: 5.1.0, 6.4.0, 7.3.0, 8.2.0
Last reconfirmed: 2019-06-03 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2019-06-03 19:07:52 UTC
GCC issues -Wreturn-local-addr even for returning the address of a local variable converted to an integer.  In addition, it also replaces the value of the integer with a zero.  Since returning the integer representation of pointer is well-defined, as is using such an integer, this leads to inconsistencies/undefined behavior when the integer is first determined to be non-zero within the body of the returning function and then zero in its caller.

The warning should only be issued for functions that return a pointer.  Likewise, the replacement of the address with a zero should only be done for such functions and not for those returning other types.

$ cat a.c && gcc -O2 -S -Wall -Wextra -fdump-tree-optimized=/dev/stdout a.c
typedef __INTPTR_TYPE__ intptr_t;

intptr_t f (void)
{
  int i;
  if ((intptr_t)&i == 0)
    __builtin_abort ();

  return (intptr_t)&i;
}

void g (void)
{
  intptr_t i = f ();
  if (i == 0)
    __builtin_trap ();
}
a.c: In function ‘f’:
a.c:9:10: warning: function returns address of local variable [-Wreturn-local-addr]
    9 |   return (intptr_t)&i;
      |          ^~~~~~~~~~~~

;; Function f (f, funcdef_no=0, decl_uid=1907, cgraph_uid=1, symbol_order=0)

f ()
{
  <bb 2> [local count: 1073741824]:
  return 0;

}



;; Function g (g, funcdef_no=1, decl_uid=1911, cgraph_uid=2, symbol_order=1) (unlikely executed)

g ()
{
  <bb 2> [count: 0]:
  __builtin_trap ();

}
Comment 1 Martin Sebor 2019-06-03 19:12:14 UTC
The warning for this code goes at least as far back as GCC 4.1.  The inconsistency between the callee and the caller started in GCC 4.9.
Comment 2 Martin Sebor 2019-06-03 21:25:57 UTC
Patch: https://gcc.gnu.org/ml/gcc-patches/2019-06/msg00114.html
Comment 3 Martin Sebor 2019-06-06 02:53:32 UTC
Author: msebor
Date: Thu Jun  6 02:53:01 2019
New Revision: 271985

URL: https://gcc.gnu.org/viewcvs?rev=271985&root=gcc&view=rev
Log:
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.


Added:
    trunk/gcc/testsuite/c-c++-common/Wreturn-local-addr.c
    trunk/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C
Modified:
    trunk/gcc/c/ChangeLog
    trunk/gcc/c/c-typeck.c
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/typeck.c
    trunk/gcc/testsuite/ChangeLog
Comment 4 Martin Sebor 2019-06-06 02:54:08 UTC
Fixed for GCC 10 via r271985.
Comment 5 Marc Glisse 2019-06-10 20:08:17 UTC
I don't understand very well why the cast to an integer before returning is relevant. What if I return the pointer, and convert it to an integer afterwards in the caller? Or what if there is no cast, but I test if the pointer is NULL both in the caller and the callee? What if I return an integer, and then cast it to a pointer in the caller? This optimization was always a bit borderline, I am trying to understand the difference better.
Comment 6 Martin Sebor 2019-06-10 22:54:03 UTC
(In reply to Marc Glisse from comment #5)

Returning a pointer to a local and converting it to an integer in the caller is undefined because the pointer is indeterminate.
If there is no cast to an integer in the return statement in the callee but the pointer is tested for equality to NULL both in the caller and the callee, the behavior of the test in the caller is also undefined.
If a pointer is converted to an integer, returned, and then cast back to a pointer in the caller, the behavior (of the cast) is implementation defined (opinions might vary here whether that could include undefined since the result is indeterminate).

What is well-defined is storing the pointer in some integer X, and b) returning the pointer as an integer R, and the comparing X == R: the result must be true.

I see the transformation to null as helpful in avoiding accidentally accessing some other function's stack (not so much as an optimization).
Comment 7 Richard Biener 2019-08-02 09:59:28 UTC
Please consider backporting for GCC 9.2.
Comment 8 Martin Sebor 2019-08-02 17:06:05 UTC
Author: msebor
Date: Fri Aug  2 17:05:34 2019
New Revision: 274022

URL: https://gcc.gnu.org/viewcvs?rev=274022&root=gcc&view=rev
Log:
Backport from mainline

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.


Added:
    branches/gcc-9-branch/gcc/testsuite/c-c++-common/Wreturn-local-addr.c
    branches/gcc-9-branch/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C
Modified:
    branches/gcc-9-branch/gcc/c/ChangeLog
    branches/gcc-9-branch/gcc/c/c-typeck.c
    branches/gcc-9-branch/gcc/cp/ChangeLog
    branches/gcc-9-branch/gcc/cp/typeck.c
    branches/gcc-9-branch/gcc/testsuite/ChangeLog
Comment 9 Martin Sebor 2019-08-02 17:06:37 UTC
Backported to 9-branch in r274022.
Comment 10 Jakub Jelinek 2020-03-04 09:32:04 UTC
GCC 8.4.0 has been released, adjusting target milestone.