On 11/27/13 01:13, Marek Polacek wrote:
On Fri, Nov 22, 2013 at 10:54:16AM +0100, Marek Polacek wrote:
Hi!
Working virtually out of Pago Pago.
The following is the implementation of the signed integer overflow
checking for the UndefinedBehaviorSanitizer. I wrote some of the
generic bits; Jakub did the i?86 handlind/optabs as well as VRP/fold
bits.
I'd like to ping this patch. Here's a rebased version that contains
a fix for miscompilation with -Os -m32.
2013-11-27 Jakub Jelinek <ja...@redhat.com>
Marek Polacek <pola...@redhat.com>
* opts.c (common_handle_option): Handle
-fsanitize=signed-integer-overflow.
* config/i386/i386.md (addv<mode>4, subv<mode>4, mulv<mode>4,
negv<mode>3, negv<mode>3_1): Define expands.
(*addv<mode>4, *subv<mode>4, *mulv<mode>4, *negv<mode>3): Define
insns.
* sanitizer.def (BUILT_IN_UBSAN_HANDLE_ADD_OVERFLOW,
BUILT_IN_UBSAN_HANDLE_SUB_OVERFLOW,
BUILT_IN_UBSAN_HANDLE_MUL_OVERFLOW,
BUILT_IN_UBSAN_HANDLE_NEGATE_OVERFLOW): Define.
* ubsan.h (PROB_VERY_UNLIKELY, PROB_EVEN, PROB_VERY_LIKELY,
PROB_ALWAYS): Define.
(ubsan_build_overflow_builtin): Declare.
* gimple-fold.c (gimple_fold_stmt_to_constant_1): Add folding of
internal functions.
* ubsan.c (PROB_VERY_UNLIKELY): Don't define here.
(ubsan_build_overflow_builtin): New function.
(instrument_si_overflow): Likewise.
(ubsan_pass): Add signed integer overflow checking.
(gate_ubsan): Enable the pass also when SANITIZE_SI_OVERFLOW.
* flag-types.h (enum sanitize_code): Add SANITIZE_SI_OVERFLOW.
* internal-fn.c: Include ubsan.h and target.h.
(ubsan_expand_si_overflow_addsub_check): New function.
(ubsan_expand_si_overflow_neg_check): Likewise.
(ubsan_expand_si_overflow_mul_check): Likewise.
(expand_UBSAN_CHECK_ADD): Likewise.
(expand_UBSAN_CHECK_SUB): Likewise.
(expand_UBSAN_CHECK_MUL): Likewise.
* fold-const.c (fold_binary_loc): Don't fold A + (-B) -> A - B and
(-A) + B -> B - A when doing the signed integer overflow checking.
* internal-fn.def (UBSAN_CHECK_ADD, UBSAN_CHECK_SUB, UBSAN_CHECK_MUL):
Define.
* tree-vrp.c (extract_range_basic): Handle internal calls.
* optabs.def (addv4_optab, subv4_optab, mulv4_optab, negv4_optab): New
optabs.
c-family/
* c-gimplify.c (c_gimplify_expr): If doing the integer-overflow
sanitization, call unsigned_type_for only when !TYPE_OVERFLOW_WRAPS.
testsuite/
* c-c++-common/ubsan/overflow-mul-2.c: New test.
* c-c++-common/ubsan/overflow-add-1.c: New test.
* c-c++-common/ubsan/overflow-add-2.c: New test.
* c-c++-common/ubsan/overflow-mul-1.c: New test.
* c-c++-common/ubsan/overflow-sub-1.c: New test.
* c-c++-common/ubsan/overflow-sub-2.c: New test.
* c-c++-common/ubsan/overflow-negate-1.c: New test.
Perhaps split this patch into two parts which can be reviewed
independently, but go into the tree at the same time. The obvious hope
would be that Uros or one of the other x86 backend folks could chime in
on that part.
--- gcc/ubsan.h.mp 2013-11-27 08:46:28.046629473 +0100
+++ gcc/ubsan.h 2013-11-27 08:46:57.578753342 +0100
@@ -21,6 +21,12 @@ along with GCC; see the file COPYING3.
#ifndef GCC_UBSAN_H
#define GCC_UBSAN_H
+/* From predict.c. */
+#define PROB_VERY_UNLIKELY (REG_BR_PROB_BASE / 2000 - 1)
+#define PROB_EVEN (REG_BR_PROB_BASE / 2)
+#define PROB_VERY_LIKELY (REG_BR_PROB_BASE - PROB_VERY_UNLIKELY)
+#define PROB_ALWAYS (REG_BR_PROB_BASE)
Seems like this should factor out rather than get duplicated.
--- gcc/gimple-fold.c.mp 2013-11-27 08:46:27.979629191 +0100
+++ gcc/gimple-fold.c 2013-11-27 08:46:57.556753251 +0100
@@ -2660,8 +2660,30 @@ gimple_fold_stmt_to_constant_1 (gimple s
tree fn;
if (gimple_call_internal_p (stmt))
- /* No folding yet for these functions. */
- return NULL_TREE;
+ {
+ enum tree_code subcode = ERROR_MARK;
+ switch (gimple_call_internal_fn (stmt))
+ {
+ case IFN_UBSAN_CHECK_ADD: subcode = PLUS_EXPR; break;
+ case IFN_UBSAN_CHECK_SUB: subcode = MINUS_EXPR; break;
+ case IFN_UBSAN_CHECK_MUL: subcode = MULT_EXPR; break;
Minor detail, put the case value and associated codes on separate lines.
case FU:
code;
more code
break;
case BAR
blah;
break;
--- gcc/internal-fn.c.mp 2013-11-27 08:46:28.014629338 +0100
+++ gcc/internal-fn.c 2013-11-27 08:46:57.559753263 +0100
@@ -31,6 +31,8 @@ along with GCC; see the file COPYING3.
#include "gimple-expr.h"
#include "is-a.h"
#include "gimple.h"
+#include "ubsan.h"
+#include "target.h"
/* The names of each internal function, indexed by function number. */
const char *const internal_fn_name_array[] = {
@@ -153,6 +155,306 @@ expand_UBSAN_NULL (gimple stmt ATTRIBUTE
gcc_unreachable ();
}
+/* Add sub/add overflow checking to the statement STMT.
+ CODE says whether the operation is +, or -. */
+
+void
+ubsan_expand_si_overflow_addsub_check (tree_code code, gimple stmt)
Does this need to use Jakub's recent changes for pr58864 (pending stack
adjustment stuff). The code seems to have the same kind of structure
Jakub was cleaning up already.
+
+/* Add negate overflow checking to the statement STMT. */
+
+void
+ubsan_expand_si_overflow_neg_check (gimple stmt)
Similarly.
+/* Add mul overflow checking to the statement STMT. */
+
+void
+ubsan_expand_si_overflow_mul_check (gimple stmt)
Similarly.
--- gcc/fold-const.c.mp 2013-11-27 08:46:27.941629031 +0100
+++ gcc/fold-const.c 2013-11-27 08:46:57.548753215 +0100
@@ -10335,14 +10335,16 @@ fold_binary_loc (location_t loc,
case PLUS_EXPR:
/* A + (-B) -> A - B */
- if (TREE_CODE (arg1) == NEGATE_EXPR)
+ if (TREE_CODE (arg1) == NEGATE_EXPR
+ && (flag_sanitize & SANITIZE_SI_OVERFLOW) == 0)
return fold_build2_loc (loc, MINUS_EXPR, type,
fold_convert_loc (loc, type, arg0),
fold_convert_loc (loc, type,
TREE_OPERAND (arg1, 0)));
/* (-A) + B -> B - A */
if (TREE_CODE (arg0) == NEGATE_EXPR
- && reorder_operands_p (TREE_OPERAND (arg0, 0), arg1))
+ && reorder_operands_p (TREE_OPERAND (arg0, 0), arg1)
+ && (flag_sanitize & SANITIZE_SI_OVERFLOW) == 0)
return fold_build2_loc (loc, MINUS_EXPR, type,
fold_convert_loc (loc, type, arg1),
fold_convert_loc (loc, type,
Presumably because you want to see the original arithmetic to instrument
it?
--- gcc/tree-vrp.c.mp 2013-11-27 08:46:28.043629459 +0100
+++ gcc/tree-vrp.c 2013-11-27 08:46:57.570753307 +0100
@@ -3757,6 +3757,40 @@ extract_range_basic (value_range_t *vr,
break;
}
}
+ else if (is_gimple_call (stmt)
+ && gimple_call_internal_p (stmt))
+ {
+ enum tree_code subcode = ERROR_MARK;
+ switch (gimple_call_internal_fn (stmt))
+ {
+ case IFN_UBSAN_CHECK_ADD: subcode = PLUS_EXPR; break;
+ case IFN_UBSAN_CHECK_SUB: subcode = MINUS_EXPR; break;
+ case IFN_UBSAN_CHECK_MUL: subcode = MULT_EXPR; break;
+ default: break;
Formatting again.
Overall the stuff outside the i386 directory looks pretty good, though
it needs some minor updates. I'd suggest extracting the i386 bits and
pinging them as a separate patch in the hope that we'll get Uros's
attention.
Jeff
