Hello,
thanks for working on this. Just a few questions inline:
On Sat, 8 Jun 2013, Marek Polacek wrote:
+/* Instrument division by zero and INT_MIN / -1. */
+
+tree
+ubsan_instrument_division (location_t loc, enum tree_code code,
+ tree op0, tree op1)
+{
+ tree t, tt;
+ tree orig = build2 (code, TREE_TYPE (op0), op0, op1);
+
+ if (TREE_CODE (TREE_TYPE (op0)) != INTEGER_TYPE
+ || TREE_CODE (TREE_TYPE (op1)) != INTEGER_TYPE)
+ return orig;
Type promotion means that INTEGRAL_TYPE_P wouldn't catch anything this
doesn't?
+ /* 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 orig;
This is just to speed up compilation a bit, I assume, since fold would
remove the instrumentation anyway.
+ tt = fold_build2 (EQ_EXPR, boolean_type_node, op1,
+ integer_minus_one_node);
Don't we usually try to have both operands of a comparison of the same
type?
+ t = fold_build2 (EQ_EXPR, boolean_type_node, op0,
+ TYPE_MIN_VALUE (TREE_TYPE (op0)));
I didn't see where this test was restricted to the signed case (0u/-1
is well defined)?
+ t = fold_build2 (TRUTH_AND_EXPR, boolean_type_node, t, tt);
+ tt = build2 (EQ_EXPR, boolean_type_node,
+ op1, integer_zero_node);
Why not fold this one?
+ t = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, tt, t);
+ 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);
+ t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (orig), t, orig);
+
+ return t;
+}
+
+/* Instrument left and right shifts. */
+
+tree
+ubsan_instrument_shift (location_t loc, enum tree_code code,
+ tree op0, tree op1)
+{
+ tree t, tt = NULL_TREE;
+ tree orig = build2 (code, TREE_TYPE (op0), op0, op1);
+ tree uprecm1 = build_int_cst (unsigned_type_for (TREE_TYPE (op1)),
+ TYPE_PRECISION (TREE_TYPE (op0)) - 1);
+ tree precm1 = build_int_cst (TREE_TYPE (op1),
+ TYPE_PRECISION (TREE_TYPE (op0)) - 1);
(if we later want to extend this to vector-scalar shifts,
element_precision will be better than TYPE_PRECISION)
Name unsigned_type_for (TREE_TYPE (op1)) and TYPE_PRECISION (TREE_TYPE
(op0)) that are used several times?
+ t = fold_convert_loc (loc, unsigned_type_for (TREE_TYPE (op1)), op1);
+ t = fold_build2 (GT_EXPR, boolean_type_node, t, uprecm1);
[...]
--- gcc/cp/typeck.c.mp 2013-06-07 23:01:16.779536165 +0200
+++ gcc/cp/typeck.c 2013-06-07 23:04:47.625161351 +0200
@@ -37,6 +37,7 @@ along with GCC; see the file COPYING3.
#include "convert.h"
#include "c-family/c-common.h"
#include "c-family/c-objc.h"
+#include "c-family/c-ubsan.h"
#include "params.h"
static tree pfn_from_ptrmemfunc (tree);
@@ -3891,6 +3892,12 @@ cp_build_binary_op (location_t location,
op0 = orig_op0;
op1 = orig_op1;
+ /* Remember whether we're doing / or %. */
+ bool doing_div_or_mod = false;
+
+ /* Remember whether we're doing << or >>. */
+ bool doing_shift = false;
+
if (code == TRUTH_AND_EXPR || code == TRUTH_ANDIF_EXPR
|| code == TRUTH_OR_EXPR || code == TRUTH_ORIF_EXPR
|| code == TRUTH_XOR_EXPR)
@@ -4070,8 +4077,15 @@ cp_build_binary_op (location_t location,
{
enum tree_code tcode0 = code0, tcode1 = code1;
tree cop1 = fold_non_dependent_expr_sfinae (op1, tf_none);
+ cop1 = maybe_constant_value (cop1);
- warn_for_div_by_zero (location, maybe_constant_value (cop1));
+ if (!processing_template_decl && tcode0 == INTEGER_TYPE
+ && (TREE_CODE (cop1) != INTEGER_CST
+ || integer_zerop (cop1)
+ || integer_minus_onep (cop1)))
+ doing_div_or_mod = true;
Aren't you already doing this test in ubsan_instrument_division?
--
Marc Glisse
