On Wed, Nov 25, 2020 at 1:45 AM Martin Sebor via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Offsets in pointer expressions are signed but GCC prefers to
> represent them as sizetype instead, and sometimes (though not
> always) crashes during GIMPLE verification when they're not.
> The sometimes-but-not-always part makes it easy for mistakes
> to slip in and go undetected for months, until someone either
> trips over it by accident, or deliberately tries to break
> things (the test case in the bug relies on declaring memchr
> with the third argument of type signed long which is what's
> apparently needed to trigger the ICE). The attached patch
> corrects a couple of such mistakes.
>
> Martin
>
> PS It would save us the time and effort dealing with these
> bugs to either detect (or even correct) the mistakes early,
> at the time the POINTER_PLUS_EXPR is built. Adding an assert
> to gimple_build_assign()) to verify that it has the expected
> type (or converting the operand to sizetype) as in the change
> below does that. I'm pretty sure I submitted a patch like it
> in the past but it was rejected. If I'm wrong or if there are
> no objections to it now I'll be happy to commit it as well.
We already verify this in verify_gimple_assign_binary after
each pass. Iff then this would argue for verifying all built
stmts immediately, assigns with verify_gimple_assign.
But I think this is overkill - your testcase is already catched
by the IL verification.
Btw, are you sure the offset returned by constant_byte_string
is never checked to be positive in callers?
The gimple-fold.c hunk and the new testcase are OK.
Richard.
> Both patches were tested on x86_64-linux.
>
> diff --git a/gcc/gimple.c b/gcc/gimple.c
> index e3e508daf2f..8e88bab9e41 100644
> --- a/gcc/gimple.c
> +++ b/gcc/gimple.c
> @@ -489,6 +489,9 @@ gassign *
> gimple_build_assign (tree lhs, enum tree_code subcode, tree op1,
> tree op2 MEM_STAT_DECL)
> {
> + if (subcode == POINTER_PLUS_EXPR)
> + gcc_checking_assert (ptrofftype_p (TREE_TYPE (op2)));
> +
> return gimple_build_assign_1 (lhs, subcode, op1, op2, NULL_TREE
> PASS_MEM_STAT);
> }
