[Bug tree-optimization/88973] [8/9 Regression] New -Wrestrict warning since r268048
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88973 --- Comment #8 from Martin Sebor --- The Asan warning is much clearer because it's based on actually observed values. This instance of the -Wrestrict warning is based on a heuristic: "we think the copy may overlap because it is within the same object and we can't prove that the offsets and the size assure it doesn't happen." There may be a way to reword the warning to make things a little bit clearer but I don't think we can match the Asan form. When the offsets and the size are completely unbounded we could just avoid printing them altogether. That would make it: 'strcpy' accessing the same array may overlap [-Werror=restrict] When the size is known it would give us: 'strcpy' accessing N bytes of the same array may overlap [-Werror=restrict] and when the offsets are known but the size isn't: 'strcpy' accessing the same array at offsets [O1, O2] and [O3, O4] may overlap [-Werror=restrict] and so on. There are many forms of the -Wrestrict warning already: singular size (1 byte) vs plural size (bytes) vs closed range (between X and Y bytes) vs open range (X or more bytes), constant offsets vs closed ranges ([X, Y]), definitely overlaps vs may overlap, and others, and because of internationalization most have to be hardcoded and can't be easily parameterized, so adding a new form into the mix isn't completely straightforward.
[Bug tree-optimization/88973] [8/9 Regression] New -Wrestrict warning since r268048
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88973 --- Comment #7 from Martin Liška --- (In reply to Martin Sebor from comment #4) > I've created a test case using the canonicalize_pathname function showing > that it does, in fact, cause an overlap to take place. The following line > in the output of the test case > Thank you very much Martin for it! Btw. ASAN prints nice error about overlapping arguments: gcc -DPATHNAME='"/home//marxin"' -Wall canonicalize_pathname.c -fsanitize=address -g && ./a.out = ==14653==ERROR: AddressSanitizer: strcpy-param-overlap: memory ranges [0x60200016,0x6020001d) and [0x60200017, 0x6020001e) overlap #0 0x772051de (/usr/lib64/libasan.so.5+0x4b1de) #1 0x40137c in canonicalize_pathname /tmp/canonicalize_pathname.c:25 #2 0x401857 in main /tmp/canonicalize_pathname.c:64 #3 0x77018b7a in __libc_start_main ../csu/libc-start.c:308 #4 0x4010e9 in _start (/tmp/a.out+0x4010e9)
[Bug tree-optimization/88973] [8/9 Regression] New -Wrestrict warning since r268048
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88973 --- Comment #6 from Martin Liška --- (In reply to Bruce Korb from comment #5) > (In reply to Martin Sebor from comment #4) > > > canonicalize_pathname.c: In function ‘canonicalize_pathname’: > > canonicalize_pathname.c:61:2: warning: ‘strcpy’ accessing 1 byte at offsets > > [0, 9223372036854775807] and [0, 9223372036854775807] may overlap 1 byte at > > offset 0 [-Wrestrict] > >61 | strcpy( result + start + 1, result + i + 2 ); > > | ^~~~ > > strcpy (0x217f265 = "//bar", 0x217f267 = "bar"): 3 > > /foo/bar > > I have a copy of this code in my project, but I haven't been writing code > for several years now. How was this fixed? The best fix would be to > determine the string length and do memmove-s, but that's enough fiddling > that I'd have to clear cobwebs and spend a mess of time. If someone's > already done that, ... :) There's an upstream bug: https://sourceforge.net/p/autogen/bugs/193/ and I'm working on patch candidate.
[Bug tree-optimization/88973] [8/9 Regression] New -Wrestrict warning since r268048
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88973 Bruce Korb changed: What|Removed |Added CC||bkorb at gnu dot org --- Comment #5 from Bruce Korb --- (In reply to Martin Sebor from comment #4) > canonicalize_pathname.c: In function ‘canonicalize_pathname’: > canonicalize_pathname.c:61:2: warning: ‘strcpy’ accessing 1 byte at offsets > [0, 9223372036854775807] and [0, 9223372036854775807] may overlap 1 byte at > offset 0 [-Wrestrict] >61 | strcpy( result + start + 1, result + i + 2 ); > | ^~~~ > strcpy (0x217f265 = "//bar", 0x217f267 = "bar"): 3 > /foo/bar I have a copy of this code in my project, but I haven't been writing code for several years now. How was this fixed? The best fix would be to determine the string length and do memmove-s, but that's enough fiddling that I'd have to clear cobwebs and spend a mess of time. If someone's already done that, ... :)
[Bug tree-optimization/88973] [8/9 Regression] New -Wrestrict warning since r268048
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88973
Martin Sebor changed:
What|Removed |Added
Status|UNCONFIRMED |RESOLVED
Resolution|--- |INVALID
--- Comment #4 from Martin Sebor ---
I've created a test case using the canonicalize_pathname function showing that
it does, in fact, cause an overlap to take place. The following line in the
output of the test case
strcpy (0x217f265 = "//bar", 0x217f267 = "bar"): 3
shows that strcpy is being called to copy the 4 bytes "bar\0" from 0x217f267 to
0x217f265, with the last two bytes of the copy overlapping its first two bytes.
$ cat canonicalize_pathname.c && gcc -DPATHNAME='"/foo///bar"' -O2 -Wall
canonicalize_pathname.c && ./a.out
char*
strcpy (char *d, const char *s)
{
unsigned n = __builtin_strlen (s);
__builtin_printf ("%s (%p = \"%s\", %p = \"%s\"): %u\n",
__func__, d, d, s, s, n);
__builtin_memcpy (d, s, n + 1);
return d;
}
char *
canonicalize_pathname (char *path)
{
int i, start;
char stub_char, *result;
result = __builtin_strdup( path );
stub_char = (*path == '/') ? '/' : '.';
i = 0;
while (result[i]) {
while (result[i] != '\0' && result[i] != '/')
i++;
start = i++;
if (!result[start])
break;
while (result[i] == '/')
i++;
if ((start + 1) != i)
{
strcpy( result + start + 1, result + i );
i = start + 1;
}
if (start > 0 && result[start - 1] == '\\')
continue;
if ((start && !result[i])
|| (result[i] == '.' && !result[i+1])) {
result[--i] = '\0';
break;
}
if (result[i] == '.') {
if (result[i + 1] == '/') {
strcpy( result + i, result + i + 1 );
i = (start < 0) ? 0 : start;
continue;
}
if (result[i + 1] == '.' &&
(result[i + 2] == '/' || !result[i + 2])) {
while (--start > -1 && result[start] != '/')
;
strcpy( result + start + 1, result + i + 2 );
i = (start < 0) ? 0 : start;
continue;
}
}
}
if (!*result) {
*result = stub_char;
result[1] = '\0';
}
return result;
}
int main (void)
{
char *p = canonicalize_pathname (PATHNAME);
__builtin_printf ("%s\n", p);
}
canonicalize_pathname.c: In function ‘canonicalize_pathname’:
canonicalize_pathname.c:61:2: warning: ‘strcpy’ accessing 1 byte at offsets [0,
9223372036854775807] and [0, 9223372036854775807] may overlap 1 byte at offset
0 [-Wrestrict]
61 | strcpy( result + start + 1, result + i + 2 );
| ^~~~
strcpy (0x217f265 = "//bar", 0x217f267 = "bar"): 3
/foo/bar
[Bug tree-optimization/88973] [8/9 Regression] New -Wrestrict warning since r268048
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88973 --- Comment #3 from Martin Sebor --- Created attachment 45497 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45497=edit canonicalize_pathname function extracted from the translation unit. Attached is the canonicalize_pathname function extracted from the translation unit that triggers the -Wrestrict warning.
[Bug tree-optimization/88973] [8/9 Regression] New -Wrestrict warning since r268048
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88973
Martin Sebor changed:
What|Removed |Added
Blocks||84774
--- Comment #2 from Martin Sebor ---
I feel like I have seen this translation unit before. The warning is by
design: for calls to strcpy with the same destination and source arrays, where
the algorithm cannot prove that an overlap doesn't occur it issues a "may
overlap" kind of a warning. The offset ranges make it clear (to me at least)
that the warning algorithm thinks the overlap is possible but not inevitable.
For example, this triggers it:
void f (char *p, int i)
{
if (i < 0)
i = 0;
char *q = p + i;
__builtin_strcpy (q, p);
}
t.c:10:3: warning: ‘__builtin_strcpy’ accessing 1 byte at offsets [0,
2147483647] and 0 may overlap 1 byte at offset 0 [-Wrestrict]
10 | __builtin_strcpy (q, p);
| ^~~
The logic behind the decision is that since using strcpy to copy within the
same array is only safe when one knows the length of the source string (in
addition to the distance between the offsets into the array), using a "bounded"
function such as memcpy or even strncpy would be more appropriate: it would
make the constraints explicit without sacrificing efficiency or readability.
GCC 8 doesn't issue the warning here because the bug fixed in r268048 on the
trunk but not yet backported to gcc-8-branch gets in the way: the offset in the
POINTER_PLUS_EXPR is in the anti-range ~[INT_MAX + 1, INT_MIN] which GCC 8
doesn't handle correctly. I haven't backported r268048 to GCC 8 yet but I
don't think this warning is a reason not to. Richard, let me know if you feel
otherwise.
Referenced Bugs:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84774
[Bug 84774] [meta-bug] bogus/missing -Wrestrict
[Bug tree-optimization/88973] [8/9 Regression] New -Wrestrict warning since r268048
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88973 Richard Biener changed: What|Removed |Added Keywords||diagnostic Priority|P3 |P2 Known to work||8.2.0 Target Milestone|--- |8.3 Summary|New -Wrestrict warning |[8/9 Regression] New |since r268048 |-Wrestrict warning since ||r268048 Known to fail||8.2.1 --- Comment #1 from Richard Biener --- I believe the change was backported as well.
