| Message ID | 1fe3da88-eb66-c8aa-87b3-4680a56e8c08@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | avoid an other ICE due to invalid built-in call (PR 89934) | expand |
On 4/3/19 1:27 PM, Martin Sebor wrote: > PR 89934 reports yet another ICE due to a call to a library > built-in function with invalid arguments. The attached patch > does the bare minimum to avoid it. I will commit it to trunk > and to GCC 8 later this week if there are no objections. > > Martin > > PS There have been many of these reports (at least three in > the wrestrict pass alone, and quite a few elsewhere). Simply > avoiding the ICEs as this patch does and as all the others > before it have done as well does nothing to help solve > the underlying problem in user code: the invalid calls will > end up with undefined behavior if they are ever made at runtime. > For example, the one in this bug report will most likely cause > data corruption or buffer overflow without ever being detected > by _FORTIFY_SOURCE. That seems far worse than failing to compile > the code (either with an ICE or with some > other hard error). > > For GCC 10 I think a more robust solution should be put in place > not just to prevent these invalid built-in calls from tripping > up the middle-end, but also to avoid the undefined behavior at > runtime. The calls could either be rejected with an error as > Clang does, or they could be replaced them with __builtin_trap > or perhaps __builtin_unreachable, depending on some command > line option. > > > gcc-89934.diff > > PR middle-end/89934 - ICE on a call with fewer arguments to strncpy declared without prototype > > gcc/ChangeLog: > > PR middle-end/89934 > * gimple-ssa-warn-restrict.c (builtin_access::builtin_access): Bail > out if the number of arguments is less than expected. > > gcc/testsuite/ChangeLog: > > PR middle-end/89934 > * gcc.dg/Wrestrict-19.c: New test. > * gcc.dg/Wrestrict-5.c: Add comment. Remove unused code. OK. WRT the larger issue. Assume we have the infrastructure bits to transform the bogus calls into a __builtin_trap or __builtin_unreachable (which I think we can easily lift from elsewhere) and a flag to allow us to determine which style we want. (we can build and prototype that and use the result in the erroneous-path-isolation pass). The question is when do we want to detect the bogus calls. There are some that we'd likely want to catch early, such as the wrong number of arguments like we see in this case. That feels like a pass over the IL right after gimplification. Other cases are more interesting. Do we try to detect when a propagation makes the call invalid as soon as it happens, or do we defer to the erroneous path isolation pass? Or do we end up doing both as there may be cases where analysis is nontrivial. Anyway, I think we can start working down this path for gcc-10. jeff
PR middle-end/89934 - ICE on a call with fewer arguments to strncpy declared without prototype gcc/ChangeLog: PR middle-end/89934 * gimple-ssa-warn-restrict.c (builtin_access::builtin_access): Bail out if the number of arguments is less than expected. gcc/testsuite/ChangeLog: PR middle-end/89934 * gcc.dg/Wrestrict-19.c: New test. * gcc.dg/Wrestrict-5.c: Add comment. Remove unused code. Index: gcc/gimple-ssa-warn-restrict.c =================================================================== --- gcc/gimple-ssa-warn-restrict.c (revision 270061) +++ gcc/gimple-ssa-warn-restrict.c (working copy) @@ -730,6 +730,10 @@ builtin_access::builtin_access (gimple *call, buil offset_int bounds[2] = { maxobjsize, maxobjsize }; if (dstref->strbounded_p) { + unsigned nargs = gimple_call_num_args (call); + if (nargs <= sizeargno) + return; + tree size = gimple_call_arg (call, sizeargno); tree range[2]; if (get_size_range (size, range, true)) Index: gcc/testsuite/gcc.dg/Wrestrict-19.c =================================================================== --- gcc/testsuite/gcc.dg/Wrestrict-19.c (nonexistent) +++ gcc/testsuite/gcc.dg/Wrestrict-19.c (working copy) @@ -0,0 +1,33 @@ +/* PR middle-end/89934 - ICE on a call with fewer arguments to strncpy + declared without prototype + { dg-do compile } + { dg-options "-O2 -Wall" } */ + +typedef __SIZE_TYPE__ size_t; + +char *strncpy (); + +char* f0 (char *s) +{ + return strncpy (); +} + +char* f1 (char *s) +{ + return strncpy (s); +} + +char* f2 (char *s) +{ + return strncpy (s, s + 1); /* ICE here. */ +} + +void f3 (char *s, size_t n, const char *t) +{ + strncpy (s, n, t); + strncpy (n, s, t); +} + +/* { dg-prune-output "\\\[-Wbuiltin-declaration-mismatch]" } + { dg-prune-output "\\\[-Wint-conversion]" } */ + Index: gcc/testsuite/gcc.dg/Wrestrict-5.c =================================================================== --- gcc/testsuite/gcc.dg/Wrestrict-5.c (revision 270061) +++ gcc/testsuite/gcc.dg/Wrestrict-5.c (working copy) @@ -1,20 +1,11 @@ -/* Test to verify that valid calls to common restrict-qualified built-in +/* PR tree-optimization/83655 - ICE on an invalid call to memcpy declared + with no prototype + Test to verify that valid calls to common restrict-qualified built-in functions declared with no prototype are checked for overlap, and that invalid calls are ignored. { dg-do compile } - { dg-prune-output "conflicting types for built-in" } { dg-options "-O2 -Wrestrict" } */ -typedef __SIZE_TYPE__ size_t; - -#if __cplusplus -extern "C" { - -#define NO_PROTO ... -#else -#define NO_PROTO /* empty */ -#endif - void* memcpy (); char* strncpy ();