On 12/18/2017 10:45 AM, Jakub Jelinek wrote:
On Mon, Dec 18, 2017 at 10:37:19AM -0700, Martin Sebor wrote:
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.

There is.  Because the last argument is size_t, so calling it with 0
is UB (on LP64, on ILP32 I think it will be optimized normally).

It isn't optimized either way.  In fact, the only indication
of a problem in the code below is the new -Wrestrict warning:

  void* memcpy ();

  void p ()
  {
    memcpy (0, 0, 0);
  }

  warning: ‘memcpy’ source argument is the same as destination [-Wrestrict]
     memcpy (0, 0, 0);
     ^~~~~~~~~~~~~~~~

;; Function p (p, funcdef_no=0, decl_uid=1892, cgraph_uid=0, symbol_order=0)

  p ()
  {
    <bb 2> [local count: 1073741825]:
    memcpy (0, 0, 0); [tail call]
    return;
  }

But you are conflating the ability to call such a built-in with
arguments of the wrong types with declaring it.  There really is
nothing wrong with declaring it as long as GCC checks the calls.

To ameliorate the undefined behavior you point out, GCC could do
one of three things: a) complain about declaring a built-in with
no prototype, or b) complain about calls to such built-ins that
pass arguments of the wrong type (or the wrong number of
arguments), or c) convert the arguments to the expected type
and still warn on mismatches).

In all cases all the information necessary to detect and diagnose
or even avoid the problem is available.  In fact, one might argue
that optimizing such calls (expanding them inline) would be
preferable to doing nothing and allowing the undefined behavior
to cause a bug at runtime.

Martin

Reply via email to