Bug 60256 - No -Wuninitialized warning for strcpy copying to self
Summary: No -Wuninitialized warning for strcpy copying to self
Status: RESOLVED WONTFIX
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 4.9.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks: Wuninitialized
  Show dependency treegraph
 
Reported: 2014-02-18 01:11 UTC by Chengnian Sun
Modified: 2019-09-29 22:44 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 4.9.3, 5.3.0, 6.1.0, 7.3.0, 8.0
Last reconfirmed: 2016-12-24 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Chengnian Sun 2014-02-18 01:11:51 UTC
The pointer "s" is uninitialized, but used in the later call to strcpy.

$: cat s.c
#include<string.h>
void f(void) {
  char* s;
  strcpy(s, s);
}
$: gcc-trunk -Wuninitialized s.c -c
$: clang-trunk -Wuninitialized s.c -c
s.c:4:10: warning: variable 's' is uninitialized when used here
      [-Wuninitialized]
  strcpy(s, s);
         ^
s.c:3:10: note: initialize the variable 's' to silence this warning
  char* s;
         ^
          = NULL
1 warning generated.
$: gcc-trunk --version
gcc-trunk (GCC) 4.9.0 20140217 (experimental)
Copyright (C) 2014 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Comment 1 Chengnian Sun 2014-02-18 01:15:08 UTC
If the call is to a user-defined function, then gcc warns. I cannot see why "strcpy" is special. 

$: cat s.c
extern void p(const char*);
void f(void) {
  char* s;
  p(s);
}
$: gcc-trunk -Wuninitialized s.c -c
s.c: In function ‘f’:
s.c:4:3: warning: ‘s’ is used uninitialized in this function [-Wuninitialized]
   p(s);
   ^
Comment 2 Andrew Pinski 2014-02-18 01:22:31 UTC
Well that is because strcpy(s,s) is optimized away as it is a nop.
Comment 3 Marek Polacek 2014-04-16 11:24:00 UTC
I think this minor issue will go away if we introduce delayed folding.
Comment 4 Manuel López-Ibáñez 2014-04-16 11:29:14 UTC
(In reply to Marek Polacek from comment #3)
> I think this minor issue will go away if we introduce delayed folding.

Is this really folded in the FE?
Comment 5 Marek Polacek 2014-04-16 18:05:39 UTC
We call c_fully_fold on strcpy (s, s);, and because this CALL_EXPR is tcc_vl_exp, we call fold () on it.  fold () then via fold_call_expr -> ...  calls fold_builtin_strcpy and that hits
  /* If SRC and DEST are the same (and not volatile), return DEST.  */
  if (operand_equal_p (src, dest, 0))
    return fold_convert_loc (loc, TREE_TYPE (TREE_TYPE (fndecl)), dest);

But on second thought, delayed folding won't help, as uninit passes are run quite late and at that point strcpy(s, s); will be already gone.
Comment 6 Manuel López-Ibáñez 2014-04-16 19:34:34 UTC
(In reply to Marek Polacek from comment #5)
> We call c_fully_fold on strcpy (s, s);, and because this CALL_EXPR is
> tcc_vl_exp, we call fold () on it.  fold () then via fold_call_expr -> ... 
> calls fold_builtin_strcpy and that hits
>   /* If SRC and DEST are the same (and not volatile), return DEST.  */
>   if (operand_equal_p (src, dest, 0))
>     return fold_convert_loc (loc, TREE_TYPE (TREE_TYPE (fndecl)), dest);
> 
> But on second thought, delayed folding won't help, as uninit passes are run
> quite late and at that point strcpy(s, s); will be already gone.

That is what I was afraid. On the other hand, this is such a particular testcase that I think it is better to focus on other more critical diagnostic issues.
Comment 7 Martin Sebor 2016-12-24 00:20:25 UTC
I'll confirm this on the grounds that a strcpy(s, s) call is invalid because it violates the strict aliasing rules (strcpy arguments are declared restrict).  The invalid strcpy(s, s) call should also be diagnosed by -Wrestrict.

The test case seems contrived but I think this type of a bug can easily be lurking in more complicated code.
Comment 8 Marek Polacek 2017-01-02 14:46:41 UTC
For strcpy(s, s) see Bug 65452, which also contains a patch for -Wsame-arguments.
Comment 9 Martin Sebor 2017-01-02 18:05:43 UTC
Thanks for the reference.  The strcmp(s, s) (and likewise memcmp(p, p, n)) case in bug 65452 is different because unlike this one, strcmp doesn't change the arrays pointed to by its arguments (which are also not declared restrict) and so calling the function with the same array is valid.
Comment 10 Martin Sebor 2018-03-08 01:22:21 UTC
GCC 8 issues a -Wrestrict warning for the test case in comment #0 but still no -Wuninitialized.  I agree that issuing -Wuninitialized will be difficult because (as noted in comment #5) the call is folded too early to tell whether the argument is valid (either initialized or assigned to) prior to the call.  The only way I can think of is to delay the folding until it is known and that has not been a popular idea in the past.

$ cat pr60256.c && gcc -O2 -S -Wall pr60256.c
#include <string.h>
void f(void) {
  char* s;
  strcpy(s, s);
}
pr60256.c: In function ‘f’:
pr60256.c:4:3: warning: ‘strcpy’ source argument is the same as destination [-Wrestrict]
   strcpy(s, s);
   ^~~~~~~~~~~~
Comment 11 Martin Sebor 2018-03-08 01:24:27 UTC
I should mention that even the -Wrestrict warning runs into problems this early -- see bug 83456.
Comment 12 Martin Sebor 2019-09-29 22:44:24 UTC
I'm going to close this as WONTFIX.  The -Wrestrict warning seems good enough for the unlikely strcpy(s, s) case, and the much more probable test case below does get diagnosed by -Wuninitialized:

$ cat pr60256.c && gcc -O2 -S -Wall pr60256.c
void f (char *d)
{
  char* s;
  strcpy (d, s);
}
pr60256.c: In function ‘f’:
pr60256.c:6:3: warning: ‘s’ is used uninitialized in this function [-Wuninitialized]
    6 |   strcpy (d, s);
      |   ^~~~~~~~~~~~~