On Tue, Nov 20, 2018 at 12:39:44AM +0100, Jakub Jelinek wrote:
> On Mon, Nov 19, 2018 at 04:10:09PM -0700, Jeff Law wrote:
> > > PR c/88065 - ICE in -Wsizeof-pointer-memaccess on an invalid strncpy
> > >
> > > gcc/c-family/ChangeLog:
> > >
> > > PR c/88065
Please also add
PR c/87297
> > > * 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?
>
> IMHO it is a good point, but it should use error_operand_p predicate instead
> of == error_mark_node checks to also catch the case where the argument is
> not error_mark_node, but has error_mark_node type. And, the testcase
> shouldn't be in gcc.dg, but in c-c++-common and cover also C++ testing.
Testcase proving that error_operand_p is really necessary:
/* PR c/87297 */
/* { dg-do compile } */
/* { dg-options "-Wsizeof-pointer-memaccess" } */
struct S { char a[4]; };
void
foo (struct S *p, const char *s)
{
struct T x; /* { dg-error "storage size|incomplete type" } */
__builtin_strncpy (x, s, sizeof p->a);
}
Works in C, still ICEs in C++ even with the patch you've posted.
819 tree dstsz = TYPE_SIZE_UNIT (TREE_TYPE (d));
debug_tree (d)
<var_decl 0x7ffff7ff6c60 x type <error_mark 0x7fffefc7fdf8>
used decl_5 VOID huvaa.c:9:12
align:8 warn_if_not_align:0 context <function_decl 0x7fffefdda200 foo>
chain <type_decl 0x7fffefda1850 T>>
And, I think it is important to have these tests in c-c++-common, as the
above test shows, it behaves differently between C and C++ (C will present
error_mark_node itself rather than VAR_DECL with error_mark_node type) and
the code in question is just a helper for the FEs.
Jakub
