On 04/12/2015 23:48, Jeff Law wrote:
>>
>> Why would pointer types be shifted at all (at the ubsan level,
>> which is basically the AST)?
> BTW, if you argument is that we can never get into this code with a
> shift of a pointer object, I'd like to see some kind of analysis to back
> up that assertion -- which could be as simple as pointing to FE code
> that issues an error if the user tried to do something like shift a
> pointer object.
You're right, I should have qualified that better. And actually there
is an issue with this patch, though it is not pointers.
There are only two call sites for ubsan_instrument_shift.
In c/c-typeck.c:
/* Remember whether we're doing << or >>. */
bool doing_shift = false;
/* The expression codes of the data types of the arguments tell us
whether the arguments are integers, floating, pointers, etc. */
code0 = TREE_CODE (type0);
code1 = TREE_CODE (type1);
switch (code)
{
...
case RSHIFT_EXPR:
...
else if ((code0 == INTEGER_TYPE || code0 == FIXED_POINT_TYPE)
&& code1 == INTEGER_TYPE)
{
doing_shift = true;
...
}
...
case LSHIFT_EXPR:
...
else if ((code0 == INTEGER_TYPE || code0 == FIXED_POINT_TYPE)
&& code1 == INTEGER_TYPE)
{
doing_shift = true;
...
}
...
}
...
if ((flag_sanitize & (SANITIZE_SHIFT | SANITIZE_DIVIDE
| SANITIZE_FLOAT_DIVIDE))
&& do_ubsan_in_current_function ()
&& (doing_div_or_mod || doing_shift)
&& !require_constant_value)
{
/* OP0 and/or OP1 might have side-effects. */
op0 = c_save_expr (op0);
op1 = c_save_expr (op1);
op0 = c_fully_fold (op0, false, NULL);
op1 = c_fully_fold (op1, false, NULL);
...
else if (doing_shift && (flag_sanitize & SANITIZE_SHIFT))
instrument_expr = ubsan_instrument_shift (location, code, op0, op1);
}
cp/typeck.c is the same but it doesn't handle code0 == FIXED_POINT_TYPE.
But FIXED_POINT_TYPE is not an integral type, and thus it would fail the
TYPE_OVERFLOW_WRAPS check with my patch. I'll post an updated patch that
also removes all instrumentation in the case of fixed point types, similar
to instrument_si_overflow.
Thanks for the careful review!
Paolo
