On 12/18/2017 10:07 AM, Jakub Jelinek wrote:
On Mon, Dec 18, 2017 at 10:00:36AM -0700, Martin Sebor wrote:
On 12/18/2017 08:10 AM, Marek Polacek wrote:
I'm not entirely up to speed with this code, but this one seemed sufficiently
obvious: check INTEGRAL_TYPE_P before looking at a tree's min/max value.
Otherwise, go with maxobjsize.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
Thanks for the fix. I noticed the problem only comes up when
memcpy is declared without a prototype. Otherwise, the memcpy
call is eliminated early on (during gimplification?) So while
avoiding the ICE is obviously a necessary change, I wonder if
this bug also suggests a missing optimization that might still
be relevant (i.e., eliminating memcpy() with a zero size for
a memcpy without a prototype):
No, we shouldn't try to optimize if people mess up stuff.
Declaring memcpy() without a prototype isn't messing up. It may
be poor practice but there's nothing inherently wrong with it.
GCC accepts it, treats it as a built-in (i.e., doesn't complain
about the declaration being incompatible with the built-in), but
doesn't optimize it. On the other hand, as I said, when someone
really does mess up and declares memcpy like so:
void* memcpy (long, long, long);
GCC optimizes calls to it as if the function were declared with
the right prototype.
What happens in this case is that
gimple_builtin_call_types_compatible_p
will fail, thus gimple_call_builtin_p and we won't be treating it specially.
That just shows a problem in the new pass:
tree func = gimple_call_fndecl (call);
if (!func || DECL_BUILT_IN_CLASS (func) != BUILT_IN_NORMAL)
return;
is just wrong, it should have been:
if (!gimple_call_builtin_p (call, BUILT_IN_NORMAL))
return;
Then you don't need to worry about float or _Complex int third argument,
or just 2 or 27 arguments instead of 3 for memcpy, etc.
Yes, but only at the cost of not checking calls to memcpy() and
other built ins declared without a prototype. I didn't consider
those when I wrote the code but now that I have, since GCC allows
built-ins to be declared without a prototype and doesn't warn even
with -Wpedantic, it doesn't seem like a good approach to me.
Martin
