Bug 84919 - [8/9 Regression] error: passing argument 1 to restrict-qualified parameter aliases with argument 5 [-Werror=restrict]
Summary: [8/9 Regression] error: passing argument 1 to restrict-qualified parameter al...
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 8.0.1
: P3 normal
Target Milestone: 9.0
Assignee: Not yet assigned to anyone
URL:
Keywords: deferred, diagnostic
Depends on:
Blocks: Wrestrict
  Show dependency treegraph
 
Reported: 2018-03-16 22:21 UTC by H.J. Lu
Modified: 2018-08-03 02:27 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2018-03-16 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description H.J. Lu 2018-03-16 22:21:09 UTC
When building Linux/x86-64 kernel 4.16-rc5, r258590 gave

gcc -Wp,-MD,/export/ssd/git/kernel.org/linux-cet/tools/objtool/.str_error_r.o.d -Wp,-MT,/export/ssd/git/kernel.org/linux-cet/tools/objtool/str_error_r.o -Wall -Werror -Wbad-function-cast -Wdeclaration-after-statement -Wformat-security -Wformat-y2k -Winit-self -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wno-system-headers -Wold-style-definition -Wpacked -Wredundant-decls -Wshadow -Wstrict-prototypes -Wswitch-default -Wswitch-enum -Wundef -Wwrite-strings -Wformat -Wstrict-aliasing=3 -Wno-switch-default -Wno-switch-enum -Wno-packed -fomit-frame-pointer -O2 -g -I/export/ssd/git/kernel.org/linux-cet/tools/include -I/export/ssd/git/kernel.org/linux-cet/tools/arch/x86/include/uapi -I/export/ssd/git/kernel.org/linux-cet/tools/objtool/arch/x86/include -I/export/ssd/git/kernel.org/linux-cet/tools/lib -D"BUILD_STR(s)=\#s" -c -o /export/ssd/git/kernel.org/linux-cet/tools/objtool/str_error_r.o ../lib/str_error_r.c
gnu-bdx-1:pts/7[63]> /usr/gcc-8.0.1-x32/bin/gcc -Wp,-MD,/export/ssd/git/kernel.org/linux-cet/tools/objtool/.str_error_r.o.d -Wp,-MT,/export/ssd/git/kernel.org/linux-cet/tools/objtool/str_error_r.o -Wall -Werror -Wbad-function-cast -Wdeclaration-after-statement -Wformat-security -Wformat-y2k -Winit-self -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wno-system-headers -Wold-style-definition -Wpacked -Wredundant-decls -Wshadow -Wstrict-prototypes -Wswitch-default -Wswitch-enum -Wundef -Wwrite-strings -Wformat -Wstrict-aliasing=3 -Wno-switch-default -Wno-switch-enum -Wno-packed -fomit-frame-pointer -O2 -g -I/export/ssd/git/kernel.org/linux-cet/tools/include -I/export/ssd/git/kernel.org/linux-cet/tools/arch/x86/include/uapi -I/export/ssd/git/kernel.org/linux-cet/tools/objtool/arch/x86/include -I/export/ssd/git/kernel.org/linux-cet/tools/lib -D"BUILD_STR(s)=\#s" -c -o /export/ssd/git/kernel.org/linux-cet/tools/objtool/str_error_r.o ../lib/str_error_r.c
../lib/str_error_r.c: In function ‘str_error_r’:
../lib/str_error_r.c:25:3: error: passing argument 1 to restrict-qualified parameter aliases with argument 5 [-Werror=restrict]
   snprintf(buf, buflen, "INTERNAL ERROR: strerror_r(%d, %p, %zd)=%d", errnum, buf, buflen, err);
   ^~~~~~~~
cc1: all warnings being treated as errors
Comment 1 H.J. Lu 2018-03-16 22:25:50 UTC
[hjl@gnu-bdx-1 tmp]$ cat x.c
#include <string.h>
#include <stdio.h>
char *str_error_r(char *buf, size_t buflen)
{
	snprintf(buf, buflen, "(%p, %zd)", buf, buflen);
	return buf;
}
[hjl@gnu-bdx-1 tmp]$ /usr/gcc-8.0.1-x32/bin/gcc -O2 -S -Wall x.c
x.c: In function ‘str_error_r’:
x.c:5:2: warning: passing argument 1 to restrict-qualified parameter aliases with argument 4 [-Wrestrict]
  snprintf(buf, buflen, "(%p, %zd)", buf, buflen);
  ^~~~~~~~
[hjl@gnu-bdx-1 tmp]$
Comment 2 H.J. Lu 2018-03-17 00:12:04 UTC
This is caused by r258455.
Comment 3 Jeffrey A. Law 2018-03-17 00:39:34 UTC
Umm, isn't that invalid code?  The whole point of the warning is to catch precisely this kind of broken idiom.  Right?
Comment 4 H.J. Lu 2018-03-17 01:57:04 UTC
(In reply to Jeffrey A. Law from comment #3)
> Umm, isn't that invalid code?  The whole point of the warning is to catch
> precisely this kind of broken idiom.  Right?

Why is it invalid?  Argument 4 is for %p. It looks valid to me.
Comment 5 Jakub Jelinek 2018-03-17 12:29:39 UTC
Yeah, only arguments corresponding to %s/%ls/%S/%n or whatever else can actually dereference the pointer (for *scanf of course all the pointers).
Comment 6 Jakub Jelinek 2018-03-17 12:39:57 UTC
On the other side, the warning matches the documented behavior and it might be too difficult to add all the exceptions when we know that the argument will not be really dereferenced.
Comment 7 H.J. Lu 2018-03-17 12:49:24 UTC
(In reply to Jakub Jelinek from comment #6)
> On the other side, the warning matches the documented behavior and it might
> be too difficult to add all the exceptions when we know that the argument
> will not be really dereferenced.

How many exceptions are there, beside %p?
Comment 8 Jakub Jelinek 2018-03-19 14:53:15 UTC
Not aware of any right now, but 1) if the format string is not compile time known, we don't know if it is %p or not 2) the format string parsing is done in other passes (-Wformat code in FEs, and gimple-ssa-sprintf.c) compared to the restrict warning.
Comment 9 H.J. Lu 2018-03-19 14:57:48 UTC
(In reply to Jakub Jelinek from comment #8)
> Not aware of any right now, but 1) if the format string is not compile time
> known, we don't know if it is %p or not 2) the format string parsing is done

We only need to handle the known format string case.

> in other passes (-Wformat code in FEs, and gimple-ssa-sprintf.c) compared to
> the restrict warning.

This shouldn't be the reason not to do it.
Comment 10 H.J. Lu 2018-03-19 15:14:18 UTC
(In reply to H.J. Lu from comment #9)
> (In reply to Jakub Jelinek from comment #8)
> > Not aware of any right now, but 1) if the format string is not compile time
> > known, we don't know if it is %p or not 2) the format string parsing is done
> 
> We only need to handle the known format string case.
> 
> > in other passes (-Wformat code in FEs, and gimple-ssa-sprintf.c) compared to
> > the restrict warning.
> 
> This shouldn't be the reason not to do it.

If there is no way to fix it, we should warn (char *) cases under -Wextra,
not under -Wall.
Comment 11 Martin Sebor 2018-03-19 15:41:20 UTC
The warning will go away once -Wrestrict is implemented for sprintf, hopefully in GCC 9 (the patch I submitted in July didn't make it to GCC 8: https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00036.html), and the handling for the built-in functions removed from the front-end.  This is being tracked in bug 83688.

Until then, the only way to avoid the warning that I can think of is to exclude from checking arguments passed through the ellipsis, but that will result in some false negatives for sprintf et al.  So the question is: do the benefits of the warning outweigh the cost of the false positives.

The potential for false positives here also isn't limited to sprintf.  It's a general problem that affects user-defined functions as well.  For example:

$ cat z.c && gcc -S -Wall z.c
void f (char* restrict, const char* restrict, const void*);

void g (char *s)
{
  f (s, "", s);
}

z.c: In function ‘g’:
z.c:5:3: warning: passing argument 1 to restrict-qualified parameter aliases with argument 3 [-Wrestrict]
   f (s, "", s);
   ^

The only way to avoid these false positives (and negatives) is to add some annotation to let users indicate how each argument is used (e.g., such as the attributes read_only and write_only that I've been working on).  This is being tracked in bug 83859.
Comment 12 H.J. Lu 2018-03-19 16:02:05 UTC
(In reply to Martin Sebor from comment #11)
> 
> Until then, the only way to avoid the warning that I can think of is to
> exclude from checking arguments passed through the ellipsis, but that will
> result in some false negatives for sprintf et al.  So the question is: do
> the benefits of the warning outweigh the cost of the false positives.
> 

It is a regression that GCC 8 can't compile lib/str_error_r.c in Linux kernel
without modifications.
Comment 13 Jeffrey A. Law 2018-03-20 22:25:02 UTC
I think this needs to defer to gcc-9.
Comment 14 Eric Gallager 2018-08-03 02:27:07 UTC
(In reply to Jeffrey A. Law from comment #13)
> I think this needs to defer to gcc-9.

It's gcc-9 now.