On 11/21/18 3:04 AM, Jakub Jelinek wrote:
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
Done.
* 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.
Thanks for the test case! I didn't know about error_operand_p
or that this could happen. I wonder how many other bugs there
are lurking in there because of it.
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.
Given how easy it is to trip over the error type I have to agree.
I made the change to error_operand_p, moved the test and committed
r266594.
Martin
