https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103255
--- Comment #6 from Andrew Macleod <amacleod at redhat dot com> ---
(In reply to Jakub Jelinek from comment #5)
> I think the bug is in:
> else if (flag_delete_null_pointer_checks
> && !TYPE_OVERFLOW_WRAPS (TREE_TYPE (expr)))
> {
> /* For -fdelete-null-pointer-checks -fno-wrapv-pointer we don't
> allow going from non-NULL pointer to NULL. */
> if(!range_includes_zero_p (&r))
> return true;
> }
>
> If !off_cst (not this case), I think before return true here we need
> r = range_nonzero (TREE_TYPE (gimple_assign_rhs1 (stmt)));
> - we are going from the base pointer which is known not to be NULL to that +
> some unknown offset, all we can say is that the result is not NULL, but we
> don't really know the exact details.
> For the off_cst case (probably only when it is actually constant, i.e.
> off_cst && off.to_constant (&offi), we could use something based on r, but
> will need to add the offi / BITS_PER_UNIT addend to the range, probably
> verify the result doesn't include zero and if it would, use that
> range_nonzero too.
>
> For the addition, not sure if that would be range_op_handler (PLUS_EXPR,
> type)->fold_range or range_op_handler (POINTER_PLUS_EXPR, type)->fold_range
> or what.
>
> Because, with the code as is, we end up with the original r for the base
> hdr_3: struct header * [4194336B, 4194336B]
> and instead of returning range unsigned int * [4194340B, 4194340B] we return
> incorrect unsigned int * [4194336B, 4194336B]
>
> Or does this code rely on pointer ranges to be always just range_zero,
> range_nonzero or varying and nothing else? If so, it should normalize
> INTEGER_CSTs used in addresses etc. into range_zero or range_nonzero and
> nothing else...
That code is an import from vr-values, and in gcc 10 it did just what we do:
if ((off_cst && known_eq (off, 0))
|| (flag_delete_null_pointer_checks
&& !TYPE_OVERFLOW_WRAPS (TREE_TYPE (expr))))
{
const value_range_equiv *vr
= get_value_range (TREE_OPERAND (base, 0));
if (!range_includes_zero_p (vr))
return true;
}
Perhaps it later sanitizes it to non-zero... Ah, indeed:
else if (vrp_stmt_computes_nonzero (stmt))
{
vr->set_nonzero (type);
vr->equiv_clear ();
}
nothing is suppose to depend on the zero/nonzero directly. Pointer tracking is
a little schizophrenic. Sometimes it just tracks zero/nonzero, other times it
uses integral calculations to come up with ranges.. especially if its gone back
or forth to an integral value via casting.
If we know its a constant, and we know the offset is a constant, then there
would be no harm in doing the math and coming up with the right value... just
as you suggest, with a check to make sure if zero has reappeared that we remove
it.
Or we could restore previous behaviour if we simply changed it to non-zero as
well before returning true.
Either would be correct/fine.
