| Message ID | 6ee328c6-788a-8f87-a7a5-c6412e4223ea@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 01/05/2017 02:53 PM, Martin Sebor wrote: > When the size passed to a call to a function like memcpy is a signed > integer whose range has a negative lower bound and a positive upper > bound the lower bound of the range of the argument after conversion > to size_t may be in excess of the maximum object size (PTRDIFF_MAX > by default). This results in -Wstringop-overflow false positives. Is this really a false positive though? ISTM that if the testcase were compiled for a 32 bit target, then all hell would break loose if g::n was 0xffffffff (unsigned 32bit). > > The attached patch detects this case and avoids the problem. > > Btw., I had a heck of a time creating a test case for this. The > large translation unit submitted with the bug is from a file in > the Linux kernel of some complexity. There, the range of the int > variable (before conversion to size_t) is [INT_MIN, INT_MAX]. It > seems very difficult to create a VR_RANGE for a signed int that > matches it. The test case I came up with that still reproduces > the false positive crates an anti-range for the signed int argument > by converting an unsigned int in one range to a signed int and > constraining it to another range. The false positive is avoided > because the code doesn't (yet) handle anti-ranges. I'd think to create [INT_MIN, INT_MAX] you'd probably need a meet at a PHI node that wasn't trivially representable and thus would get dropped to [INT_MIN, INT_MAX]. A meet of 3 values with 2 holes for example might do what you wanted. > > Martin > > PS This seems lie a bug or gotcha in the get_range_info() function. > In the regression test added by the patch the VRP dump shows the > following: Note that the ranges in VRP can be more precise than the ranges seen outside VRP. The warning is being emitted at the gimple->rtl phase, so you may be stumbling over one of the numerous problems with losing good range information. Jeff
On 01/06/2017 01:55 PM, Jeff Law wrote: > On 01/05/2017 02:53 PM, Martin Sebor wrote: >> When the size passed to a call to a function like memcpy is a signed >> integer whose range has a negative lower bound and a positive upper >> bound the lower bound of the range of the argument after conversion >> to size_t may be in excess of the maximum object size (PTRDIFF_MAX >> by default). This results in -Wstringop-overflow false positives. > Is this really a false positive though? ISTM that if the testcase > were compiled for a 32 bit target, then all hell would break loose if > g::n was 0xffffffff (unsigned 32bit). You're right! In the test case I added the warning is indeed correct. And after spending more time going through the submitted translation unit I think it's correct there too because of the unsigned to signed (to unsigned) conversion. I think I was initially looking the ranges before the function got inlined into the caller where the problem occurs. I may have also misread the VRP dump (or looked at the wrong one too). It also doesn't help is that the warning doesn't show the inlining stack. Let me fix that. > I'd think to create [INT_MIN, INT_MAX] you'd probably need a meet at a > PHI node that wasn't trivially representable and thus would get dropped > to [INT_MIN, INT_MAX]. A meet of 3 values with 2 holes for example > might do what you wanted. It would be nice to have a helper in the test suite for creating ranges. (Or perhaps a built-in.) > Note that the ranges in VRP can be more precise than the ranges seen > outside VRP. The warning is being emitted at the gimple->rtl phase, so > you may be stumbling over one of the numerous problems with losing good > range information. I think I had simply misread the ranges. Thanks for the careful review! Martin
PR c/78973 - [7.0 regression] warning: ‘memcpy’: specified size between 18446744071562067968 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=] gcc/ChangeLog: PR c/78973 * builtins.c (get_size_range): Handle signed to unsigned conversion. gcc/testsuite/ChangeLog: PR c/78973 * gcc.dg/pr78973.c: New test. diff --git a/gcc/builtins.c b/gcc/builtins.c index 5b76dfd..883d25c 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -3051,9 +3051,26 @@ get_size_range (tree exp, tree range[2]) if (range_type == VR_RANGE) { + /* The range can be the result of a conversion of a signed + variable to size_t with the lower bound after conversion + corresponding to a negative lower bound of the original + variable. Avoid the false positive for this case. */ + gimple *def = SSA_NAME_DEF_STMT (exp); + if (is_gimple_assign (def)) + { + tree_code code = gimple_assign_rhs_code (def); + if (code == NOP_EXPR) + { + tree rhs = gimple_assign_rhs1 (def); + if (!TYPE_UNSIGNED (TREE_TYPE (rhs))) + return get_size_range (rhs, range); + } + } + /* Interpret the bound in the variable's type. */ range[0] = wide_int_to_tree (TREE_TYPE (exp), min); range[1] = wide_int_to_tree (TREE_TYPE (exp), max); + return true; } else if (range_type == VR_ANTI_RANGE) diff --git a/gcc/testsuite/gcc.dg/pr78973.c b/gcc/testsuite/gcc.dg/pr78973.c new file mode 100644 index 0000000..ef212cf --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr78973.c @@ -0,0 +1,18 @@ +/* PR c/78973 - [7.0 regression] warning: ‘memcpy’: specified size between + 18446744071562067968 and 18446744073709551615 exceeds maximum object + size 9223372036854775807 [-Wstringop-overflow=] + { dg-do compile } + { dg-options "-O2 -Wall" } */ + +void f (void *p, int n) +{ + if (n <= 4) + __builtin_memset (p, 0, n); /* { dg-bogus "exceeds maximum object size" } */ +} + +void g (void *d, unsigned n) +{ + if (n < 5) + n = 5; + f (d, n); +}