The warning is issued because a PHI operand of the the return statement references another PHI node one of whose operands is yet another PHI node that includes the local variable stack_buf among its operands:

  <bb 6> [local count: 79230146]:
  # buffer_17 = PHI <&stack_buf(4), buffer_38(D)(5)>
  ...
  <bb 7> [local count: 1073741824]:
  # buf_20 = PHI <buffer_17(6), buf_57(25)>
  ...
  <bb 29> [local count: 79230145]:
  # _25 = PHI <0B(10), buf_20(15), 0B(24), 0B(28), b_63(14), b_61(17), 0B(9), buf_20(16), buf_20(18)>
  stack_buf ={v} {CLOBBER};
  return _25;

A simplified test case is below.  It warns for the same reason (but with fewer indirections):

  # buffer_2 = PHI <&stack_buf(2), buffer_7(D)(3)>
  ...
  # _4 = PHI <0B(4), _11(7), 0B(5), buffer_2(6)>
  return _4;


There's no way to tell from the IL in either of these test cases that one of the final PHI operands cannot point to stack_buf so the warning triggers.  Short of not considering PHI arguments that are themselves PHIs  and trading off false positives for false negatives I don't see how to avoid the warning in this case.


$ cat pr93644.c && gcc -O2 -S -Wall -fdump-tree-isolate-paths=/dev/stdout pr93644.c
typedef __SIZE_TYPE__ size_t;

char *
careadlinkat (char *buffer, size_t buffer_size)
{
  char stack_buf[1024];
  if (!buffer_size)
    {
      buffer = stack_buf;
      buffer_size = sizeof stack_buf;
    }

  char *buf = buffer;
  size_t buf_size = buffer_size;

  extern int link_length;
  if (link_length < 0)
    return 0;

  size_t link_size = link_length;
  if (link_size < buf_size)
    {
      if (buf == stack_buf)
	return __builtin_malloc (link_size);

      return buf;
    }

  return 0;
}

;; Function careadlinkat (careadlinkat, funcdef_no=0, decl_uid=1932, cgraph_uid=1, symbol_order=0)

pr93644.c: In function ‘careadlinkat’:
cc1: warning: function may return address of local variable [-Wreturn-local-addr]
pr93644.c:6:8: note: declared here
    6 |   char stack_buf[1024];
      |        ^~~~~~~~~
careadlinkat (char * buffer, size_t buffer_size)
{
  size_t link_size;
  char stack_buf[1024];
  int link_length.0_1;
  char * _4;
  char * _11;

  <bb 2> [local count: 1073741824]:
  if (buffer_size_6(D) == 0)
    goto <bb 4>; [50.00%]
  else
    goto <bb 3>; [50.00%]

  <bb 3> [local count: 536870912]:

  <bb 4> [local count: 1073741824]:
  # buffer_2 = PHI <&stack_buf(2), buffer_7(D)(3)>
  # buffer_size_3 = PHI <1024(2), buffer_size_6(D)(3)>
  link_length.0_1 = link_length;
  if (link_length.0_1 < 0)
    goto <bb 8>; [12.76%]
  else
    goto <bb 5>; [87.24%]

  <bb 5> [local count: 936732369]:
  link_size_9 = (size_t) link_length.0_1;
  if (buffer_size_3 > link_size_9)
    goto <bb 6>; [71.00%]
  else
    goto <bb 8>; [29.00%]

  <bb 6> [local count: 665079983]:
  if (&stack_buf == buffer_2)
    goto <bb 7>; [17.43%]
  else
    goto <bb 8>; [82.57%]

  <bb 7> [local count: 115923441]:
  _11 = __builtin_malloc (link_size_9);

  <bb 8> [local count: 1073741824]:
  # _4 = PHI <0B(4), _11(7), 0B(5), buffer_2(6)>
  stack_buf ={v} {CLOBBER};
  return _4;

}

  # buffer_2 = PHI <&stack_bufD.1939(3), buffer_7(D)(9)>

  buffer_18 = ASSERT_EXPR <buffer_2, buffer_2 != &stack_bufD.1939>;

Can't we deduce from this

  buffer_18 = buffer_7(D)

? Of course that's not a general solution, but it looks like a sensible optimization, and in the reduced testcase it should remove &stack_buf from the possible return values.

The most likely reason the warning isn't suppressed by #pragma GCC diagnostic is that the statement it's issued for has no source line number associated with it.  That's also why the warning doesn't point to such a statement (only the note does).  The details of this bug are in pr90735.

pr90735 suggests using the location of the closing curly brace of the function instead.  Another alternative might be to use the location associated with the note.

WRT c#2, yes it's a reasonable deduction, but the warning and the point where we have the asserts are in two different passes.  ie, the info isn't really available when we need it.



What we might be able to do is see the assert:

  buffer_16 = ASSERT_EXPR <buffer_3, buffer_3 != &stack_buf>;

Then look at the DEF point for buffer_3:

  # buffer_3 = PHI <&stack_buf(3), buffer_8(D)(9)>

Which means at the point of the assertion that buffer_16 can only have the value buffer_8 and replace the assert with

buffer_16 = buffer_8

That would in turn allow the use of buffer_16 in the problematic PHI to be replaced with buffer_8 and avoid the problem

We have to be careful and ensure that doesn't break the SSA form (it's safe in this example because buffer_8 is the default def).

I think we might be able to do this in remove_range_assertions

So optimizing things in remove_range_assertions works for the reduced testcase, but not for the original testcase.  There's a couple of deeper issues that have to be figured out for the original testcase. 

The trick we were trying to utilize is if we could safely copy-propagate a value which we knew not to be a stack address for an object which might have been a stack address, then we avoid the false positive.  This has a nice property that it likely improves optimization.

But it's insufficient for the full testcase.  First, loops get in the way. 

We might have something like this at the top of a loop:


x = phi (a, b)
b = <something>


If we have an subsequent assert that x != b, we can't then replace uses of x with a.  Why?  Because the assert refers to the value of b on the current iteration while the PHI refers to the value of b on the previous iteration.  This issue doesn't show up in the reduced testcase, but does in the full testcase.

Second, we have to deal with the cascading nature of the asserts.  If we go back to the original testcase the PHI in question looks like this just before removal of the ASSERT_EXPRs in VRP1:

  # _25 = PHI <0B(10), buf_79(16), 0B(26), 0B(30), b_94(15), b_92(19), 0B(9), buf_80(17), buf_80(39)>

Just looking at chain for buf_79 we have:

  # buffer_17 = PHI <&stack_buf(5), buffer_38(D)(34)>

  # buf_20 = PHI <buffer_17(6), buf_90(32)>

  # buf_29 = PHI <buf_20(37)>

  buf_79 = ASSERT_EXPR <buf_29, buf_29 != &stack_buf>;


We can see that buf_79 has 3 possible values:

&stack_buf, buffer_38 and buf_90 (buf_90 is loop carried)

The ASSERT only excludes &stack_buf so we can't copy propagate buffer_38 or buf_90 for the uses of buf_79.  And we (of course) lose the knowledge that buf_79 can't be &stack_buf when we drop the ASSERT_EXPRs.


We may still want to use the trick Marc noted in c#2.  It applies something like 20k times during a bootstrap and fixes Wreturn-local-addr-9 in the testsuite.  But it doesn't seem appropriate for stage4.

I find myself wondering if we could tackle this on the alias analysis side by looking at the PTA solution and see if any of the objects point into the stack.  I also wonder if PTA would benefit from the VRP analysis which excluded the stack object for certain addresses.  All blue sky stuff though.

*** Bug 95044 has been marked as a duplicate of this bug. ***

We now have a generic workaround to this bug:

If the bug occurs in a function foo:
1. Rename foo to foo_internal, mark it as '__attribute__ ((__noinline__)) static' and add a 'char stack_buf[1024]' parameter.
2. In the function foo_internal, drop the stack-allocated buffer and use the new parameter instead.
3. Create a new function foo, with the same signature as before, that merely allocates a 'char stack_buf[1024]' on the stack and passes it to foo_internal.

For an example, see https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=2a3468c9f263596815a3383c0157ba9a81cf2d24

The generic workaround that Bruno describes ran into problems in Gnulib, as it's enabled only when compiled with -DGCC_LINT, and some users don't compile it that way. So we now have a more elaborate workaround:

https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=5a8a1598e1243599feb302f0f75d947553f2918f

that causes GCC to issue warnings like the following when the file is not compiled with -DGCC_LINT:

careadlinkat.c:58:4: warning: #warning "GCC might issue a bogus -Wreturn-local-addr warning here." [-Wcpp]
   58 | #  warning "GCC might issue a bogus -Wreturn-local-addr warning here."
      |    ^~~~~~~
careadlinkat.c:59:4: warning: #warning "See <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93644>." [-Wcpp]
   59 | #  warning "See <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93644>."
      |    ^~~~~~~
careadlinkat.c: In function ‘careadlinkat’:
careadlinkat.c:193:10: warning: function may return address of local variable [-Wreturn-local-addr]
  193 |   return readlink_stk (fd, filename, buffer, buffer_size, alloc,
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  194 |                        preadlinkat, stack_buf);
      |                        ~~~~~~~~~~~~~~~~~~~~~~~
careadlinkat.c:192:8: note: declared here
  192 |   char stack_buf[STACK_BUF_SIZE];
      |        ^~~~~~~~~


but obviously this is awkward and it would be better if the bug were fixed.

Created attachment 49783 [details]
another instance of a -Wreturn-local-addr false alarm

I ran into a different instance of the bug today, while working on another Gnulib source file lib/canonicalize.c. A stripped-down test case attached. To reproduce the problem:

$ gcc -O2 -S return-local-addr.i 
return-local-addr.i: In function ‘canonicalize_filename_mode’:
cc1: warning: function may return address of local variable [-Wreturn-local-addr]
return-local-addr.i:28:25: note: declared here
   28 |   struct scratch_buffer rname_buffer;
      |                         ^~~~~~~~~~~~

This is with GCC 10.2.1 20201125 (Red Hat 10.2.1-9) on x86-64.

There are really two bugs here:

(A) GCC emits the false alarm.

(B) there's no way to shut off the false alarm, not even with '# pragma GCC diagnostic ignored "-Wreturn-local-addr"'.

Although this bug report's replies have been about (A), even fixing (B) would be a real help with Gnulib.

Should I file a separate bug report for (B)? I assume (B)'s easier to fix.

On Wed, Dec 16, 2020 at 9:05 PM eggert at cs dot ucla.edu
<gcc-bugzilla@gcc.gnu.org> wrote:
> ...
> (B) there's no way to shut off the false alarm, not even with '# pragma GCC
> diagnostic ignored "-Wreturn-local-addr"'.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53431

I looks like this might be another opportunity to use the predicate analysis from tree-ssa-uninit.c (once it's generalized).

GCC 11.1 has been released, retargeting bugs to GCC 11.2.

Created attachment 50739 [details]
another minimal example to demonstrate the false alarm

This is another instance of the test case. A senseless, yet perfectly valid piece of C code. It creates the false positive warning if compiled like this:

gcc -Wreturn-local-addr -O1 -fisolate-erroneous-paths-dereference -c pr93644_2.c

pr93644_2.c: In function ‘buildVname’:
pr93644_2.c:28:12: warning: function may return address of local variable [-Wreturn-local-addr]
   28 |     return vname;
      |            ^~~~~
pr93644_2.c:18:17: note: declared here
   18 |     char        buf[256];
      |                 ^~~


I'm using gcc version 11.1.1 20210428 (Red Hat 11.1.1-1) (GCC) 
However, I can silence the warning using

# pragma GCC diagnostic ignored "-Wreturn-local-addr"

GCC 11.2 is being released, retargeting bugs to GCC 11.3

This affects musl's getcwd implementation, btw: https://git.musl-libc.org/cgit/musl/tree/src/unistd/getcwd.c?id=v1.2.2

GCC 12 has changed to point the warning at the closing curly as suggested in pr90735 so its output now looks like this:

pr93644.c: In function ‘careadlinkat’:
pr93644.c:30:1: warning: function may return address of local variable [-Wreturn-local-addr]
   30 | }
      | ^
pr93644.c:6:8: note: declared here
    6 |   char stack_buf[1024];
      |        ^~~~~~~~~

With that change the warning can also be suppressed by #pragma GCC diagnostic placed before the closing curly.

GCC 12 also has separated the uninitialized predicate analyzer into a standalone module but no warning except -Wmaybe-uninitialized makes use of it yet.  This bug might be an opportunity to try it out.

Hi,

I'm seeing this message from the "current" findutils.

What is the solution?

Best regards,

George...

GCC 11.3 is being released, retargeting bugs to GCC 11.4.

(In reply to George R. Goffe from comment #20)
> Hi,
> 
> I'm seeing this message from the "current" findutils.
>

Likewise with libiconv.

> What is the solution?

One hasn't been figured out yet; just use -Wno-cpp and -Wno-return-local-addr for now, I guess...

GCC 11.4 is being released, retargeting bugs to GCC 11.5.

*** Bug 110267 has been marked as a duplicate of this bug. ***

*** Bug 105868 has been marked as a duplicate of this bug. ***