On 11/19/2018 04:10 PM, Jeff Law wrote:
On 11/17/18 3:45 PM, Martin Sebor wrote:
-Wsizeof-pointer-memaccess fails with an ICE when one of
the arguments is ill-formed (error_mark_node). To avoid
the error the attached patch has the function bail in this
case.
Martin
gcc-88065.diff
PR c/88065 - ICE in -Wsizeof-pointer-memaccess on an invalid strncpy
gcc/c-family/ChangeLog:
PR c/88065
* c-warn.c (sizeof_pointer_memaccess_warning): Bail if source
or destination is an error.
gcc/testsuite/ChangeLog:
PR c/88065
* gcc.dg/Wsizeof-pointer-memaccess2.c: New test.
This is probably OK. But before final ACK, is there a point earlier
where we could/should have bailed out?
ie, when does the ERROR_MARK get created and if you were to look at the
flow from that point to the offending call to
sizeof_pointer_memaccess_warning is there a better place to bail?
sizeof_pointer_memaccess_warning() has a big switch statement
with the built-in code as a controlling expression that it
uses to determine which argument is the source, which one is
the destination, and which one is the size/bound. To do
the checking anywhere else up the stack the caller would have
to replicate the same switch statement, so I think this is
the right spot to do it.
I can move the test if it's important.
FWIW, I tend to only add rests to c-c++-common if they exercise
different code between the two front-ends. Otherwise it seems
like unnecessarily increasing the already excessive testsuite
runtimes (each of the C++ tests runs once for each C++ revision,
i.e., C++ 98, C++ 11, C++ 14, and C++ 17). I do realize that
if the implementation were to diverge between the two front-ends
having test coverage for both would be a good thing. So it's
a trade-off. While griping about c-c++-common: sometimes (though
not in the case of ICEs), because of subtle differences between
the two front-ends (like missing or inaccurate location
information) it can also be a pain to get the tests to produce
the same output even for a shared implementation of a warning,
let alone for distinct ones.
Martin
