Bug 81811 - missing -Wreturn-local-addr returning strcpy result
Summary: missing -Wreturn-local-addr returning strcpy result
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 8.0
: P3 normal
Target Milestone: 10.0
Assignee: Martin Sebor
URL:
Keywords: diagnostic, missed-optimization
Depends on:
Blocks: Wreturn-local-addr
  Show dependency treegraph
 
Reported: 2017-08-10 20:34 UTC by Martin Sebor
Modified: 2020-02-09 21:55 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-08-10 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2017-08-10 20:34:17 UTC
Modifying the test case from bug 81810 to have each of the test functions return the address of the local array by returning each of the built-in's return value exposes a defect in the implementation of -Wreturn-local-addr.  The warning successfully detects the bug in the case of memcpy but fails to detect the same problem involving strcpy or strncpy.

$ cat a.c && gcc -O2 -S -Wall  -fdump-tree-optimized=/dev/stdout a.c
void* f (const void *p, unsigned n)
{
  char a[8];
  return __builtin_memcpy (a, p, n);   // -Wreturn-local-addr (good)
}

char* g (const char *s)
{
  char a[8];
  return __builtin_strcpy (a, s);   // missing -Wreturn-local-addr
}

char* h (const char *s)
{
  char a[8];
  return __builtin_strncpy (a, s, sizeof a);   // missing -Wreturn-local-addr
}

a.c: In function ‘f’:
a.c:4:10: warning: function returns address of local variable [-Wreturn-local-addr]
   return __builtin_memcpy (a, p, n);   // -Wreturn-local-addr (good)
          ^~~~~~~~~~~~~~~~~~~~~~~~~~
a.c:3:8: note: declared here
   char a[8];
        ^

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

f (const void * p, unsigned int n)
{
  <bb 2> [100.00%] [count: INV]:
  return 0B;

}



;; Function g (g, funcdef_no=1, decl_uid=1820, cgraph_uid=1, symbol_order=1)

g (const char * s)
{
  char a[8];
  char * _4;

  <bb 2> [100.00%] [count: INV]:
  _4 = __builtin_strcpy (&a, s_2(D));
  a ={v} {CLOBBER};
  return _4;

}



;; Function h (h, funcdef_no=2, decl_uid=1824, cgraph_uid=2, symbol_order=2)

h (const char * s)
{
  char a[8];
  char * _4;

  <bb 2> [100.00%] [count: INV]:
  _4 = __builtin_strncpy (&a, s_2(D), 8);
  a ={v} {CLOBBER};
  return _4;

}
Comment 1 Eric Gallager 2017-08-10 21:47:06 UTC
Confirmed.
Comment 2 Richard Biener 2017-08-15 10:15:45 UTC
Wonder whether the memcpy case is because we fold the memcpy away as both
memcpy and strcpy are marked RET1 (returns arg1).
Comment 3 Martin Sebor 2017-08-15 14:29:16 UTC
(In reply to Richard Biener from comment #2)
> Wonder whether the memcpy case is because we fold the memcpy away as both
> memcpy and strcpy are marked RET1 (returns arg1).

The memcpy is eliminated in DSE and turned into a plain 'return a' statement that  the find_explicit_erroneous_behavior() in tree-ssa-isolate-paths.c that implements the warning knows how to handle.

DSE doesn't handle strcpy or strncpy so the calls are not eliminated and because find_explicit_erroneous_behavior() doesn't handle calls, the 'return strcpy(a, s)' statement isn't detected.

This suggests an opportunity to improve not just the warning but also DSE.
Comment 4 Eric Gallager 2018-12-12 04:26:04 UTC
(In reply to Martin Sebor from comment #3)
> (In reply to Richard Biener from comment #2)
> > Wonder whether the memcpy case is because we fold the memcpy away as both
> > memcpy and strcpy are marked RET1 (returns arg1).
> 
> The memcpy is eliminated in DSE and turned into a plain 'return a' statement
> that  the find_explicit_erroneous_behavior() in tree-ssa-isolate-paths.c
> that implements the warning knows how to handle.
> 
> DSE doesn't handle strcpy or strncpy so the calls are not eliminated and
> because find_explicit_erroneous_behavior() doesn't handle calls, the 'return
> strcpy(a, s)' statement isn't detected.
> 
> This suggests an opportunity to improve not just the warning but also DSE.

so, sounds like a missed-optimization too, then
Comment 5 Martin Sebor 2020-02-09 21:55:02 UTC
Fixed for GCC 10 (most likely by r273261).  Tested in gcc.dg/Wreturn-local-addr-6.c.