Hello, couple comments (not a true review)
On Tue, 11 Jun 2013, Marek Polacek wrote:
+tree +ubsan_instrument_division (location_t loc, tree op0, tree op1) +{ + tree t, tt; + tree type0 = TREE_TYPE (op0); + tree type1 = TREE_TYPE (op1);
Can the 2 types be different? I thought divisions had homogeneous arguments, and the instrumentation was done late enough to avoid any potential issue, but maybe not...
+ tree type1_zero_cst = build_int_cst (type1, 0);
It is a bit funny to do that before the following test ;-)
+ if (TREE_CODE (type0) != INTEGER_TYPE + || TREE_CODE (type1) != INTEGER_TYPE) + return NULL_TREE; + + /* If we *know* that the divisor is not -1 or 0, we don't have to + instrument this expression. + ??? We could use decl_constant_value to cover up more cases. */ + if (TREE_CODE (op1) == INTEGER_CST + && integer_nonzerop (op1) + && !integer_minus_onep (op1)) + return NULL_TREE; + + /* We check INT_MIN / -1 only for signed types. */ + if (!TYPE_UNSIGNED (type0) && !TYPE_UNSIGNED (type1)) + { + tt = fold_build2 (EQ_EXPR, boolean_type_node, op1, + build_int_cst (type1, -1)); + t = fold_build2 (EQ_EXPR, boolean_type_node, op0, + TYPE_MIN_VALUE (type0)); + t = fold_build2 (TRUTH_AND_EXPR, boolean_type_node, t, tt); + } + else + t = type1_zero_cst; + tt = fold_build2 (EQ_EXPR, boolean_type_node, + op1, type1_zero_cst); + t = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, tt, t);
If you wrote the comparison with 0 first, you could put the OR in the signed branch instead of relying on folding |0, no?
+ tt = builtin_decl_explicit (BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW); + tt = build_call_expr_loc (loc, tt, 0); + t = fold_build3 (COND_EXPR, void_type_node, t, tt, void_zero_node); + + return t; +}
-- Marc Glisse