This patch to the Go compiler and runtime adds explicit checks for division by zero and for division overflow (INT_MIN / -1).
In Go, a division by zero is supposed to produce a panic; that did not happen on processors like PPC that do not issue a SIGFPE signal on division by zero. This patch fixes that problem. In Go, a division overflow is supposed to wrap. On x86 processors it would raise a SIGFPE signal leading to a panic. This patch fixes that problem as well. This patch introduces new two command line options to control this behaviour. Currently the compiler will always generate these checks. It would be better to only generate the checks for processors where they are needed. That would require somehow communicating this fact to the driver or the frontend. That is something for the future. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu and powerpc64-unknown-linux-gnu. Committed to mainline and 4.7 branch. Ian 2012-04-20 Ian Lance Taylor <i...@google.com> * lang.opt: Add -fgo-check-divide-zero and -fgo-check-divide-overflow. * gccgo.texi (Invoking gccgo): Document new options.
Index: gcc/go/lang.opt =================================================================== --- gcc/go/lang.opt (revision 184521) +++ gcc/go/lang.opt (working copy) @@ -1,6 +1,6 @@ ; lang.opt -- Options for the gcc Go front end. -; Copyright (C) 2009, 2010, 2011 Free Software Foundation, Inc. +; Copyright (C) 2009, 2010, 2011, 2012 Free Software Foundation, Inc. ; ; This file is part of GCC. ; @@ -37,6 +37,14 @@ Wall Go ; Documented in c.opt +fgo-check-divide-zero +Go Var(go_check_divide_zero) Init(1) +Add explicit checks for division by zero + +fgo-check-divide-overflow +Go Var(go_check_divide_overflow) Init(1) +Add explicit checks for division overflow in INT_MIN / -1 + fgo-dump- Go Joined RejectNegative -fgo-dump-<type> Dump Go frontend internal information Index: gcc/go/gccgo.texi =================================================================== --- gcc/go/gccgo.texi (revision 184521) +++ gcc/go/gccgo.texi (working copy) @@ -12,7 +12,7 @@ @include gcc-common.texi @c Copyright years for this manual. -@set copyrights-go 2010 +@set copyrights-go 2010, 2011, 2012 @copying @c man begin COPYRIGHT @@ -174,6 +174,31 @@ By default @command{gccgo} will warn abo more return parameters but lack an explicit @code{return} statement. This warning may be disabled using @option{-fno-require-return-statement}. + +@item -fgo-check-divide-zero +@cindex @option{-fgo-check-divide-zero} +@cindex @option{-fno-go-check-divide-zero} +Add explicit checks for division by zero. In Go a division (or +modulos) by zero causes a panic. On Unix systems this is detected in +the runtime by catching the @code{SIGFPE} signal. Some processors, +such as PowerPC, do not generate a SIGFPE on division by zero. Some +runtimes do not generate a signal that can be caught. On those +systems, this option may be used. Or the checks may be removed via +@option{-fno-go-check-divide-zero}. This option is currently on by +default, but in the future may be off by default on systems that do +not require it. + +@item -fgo-check-divide-overflow +@cindex @option{-fgo-check-divide-overflow} +@cindex @option{-fno-go-check-divide-overflow} +Add explicit checks for division overflow. For example, division +overflow occurs when computing @code{INT_MIN / -1}. In Go this should +be wrapped, to produce @code{INT_MIN}. Some processors, such as x86, +generate a trap on division overflow. On those systems, this option +may be used. Or the checks may be removed via +@option{-fno-go-check-divide-overflow}. This option is currently on +by default, but in the future may be off by default on systems that do +not require it. @end table @c man end Index: gcc/go/gofrontend/gogo.h =================================================================== --- gcc/go/gofrontend/gogo.h (revision 185080) +++ gcc/go/gofrontend/gogo.h (working copy) @@ -2791,6 +2791,9 @@ static const int RUNTIME_ERROR_MAKE_MAP_ // Channel capacity out of bounds in make: negative or overflow. static const int RUNTIME_ERROR_MAKE_CHAN_OUT_OF_BOUNDS = 9; +// Division by zero. +static const int RUNTIME_ERROR_DIVISION_BY_ZERO = 10; + // This is used by some of the langhooks. extern Gogo* go_get_gogo(); Index: gcc/go/gofrontend/expressions.cc =================================================================== --- gcc/go/gofrontend/expressions.cc (revision 186511) +++ gcc/go/gofrontend/expressions.cc (working copy) @@ -5647,6 +5647,7 @@ Binary_expression::do_get_tree(Translate enum tree_code code; bool use_left_type = true; bool is_shift_op = false; + bool is_idiv_op = false; switch (this->op_) { case OPERATOR_EQEQ: @@ -5689,11 +5690,15 @@ Binary_expression::do_get_tree(Translate if (t->float_type() != NULL || t->complex_type() != NULL) code = RDIV_EXPR; else - code = TRUNC_DIV_EXPR; + { + code = TRUNC_DIV_EXPR; + is_idiv_op = true; + } } break; case OPERATOR_MOD: code = TRUNC_MOD_EXPR; + is_idiv_op = true; break; case OPERATOR_LSHIFT: code = LSHIFT_EXPR; @@ -5714,6 +5719,7 @@ Binary_expression::do_get_tree(Translate go_unreachable(); } + location_t gccloc = this->location().gcc_location(); tree type = use_left_type ? TREE_TYPE(left) : TREE_TYPE(right); if (this->left_->type()->is_string_type()) @@ -5741,28 +5747,27 @@ Binary_expression::do_get_tree(Translate } tree eval_saved = NULL_TREE; - if (is_shift_op) + if (is_shift_op + || (is_idiv_op && (go_check_divide_zero || go_check_divide_overflow))) { // Make sure the values are evaluated. - if (!DECL_P(left) && TREE_SIDE_EFFECTS(left)) + if (!DECL_P(left)) { left = save_expr(left); eval_saved = left; } - if (!DECL_P(right) && TREE_SIDE_EFFECTS(right)) + if (!DECL_P(right)) { right = save_expr(right); if (eval_saved == NULL_TREE) eval_saved = right; else - eval_saved = fold_build2_loc(this->location().gcc_location(), - COMPOUND_EXPR, + eval_saved = fold_build2_loc(gccloc, COMPOUND_EXPR, void_type_node, eval_saved, right); } } - tree ret = fold_build2_loc(this->location().gcc_location(), - code, + tree ret = fold_build2_loc(gccloc, code, compute_type != NULL_TREE ? compute_type : type, left, right); @@ -5780,39 +5785,116 @@ Binary_expression::do_get_tree(Translate tree compare = fold_build2(LT_EXPR, boolean_type_node, right, build_int_cst_type(TREE_TYPE(right), bits)); - tree overflow_result = fold_convert_loc(this->location().gcc_location(), - TREE_TYPE(left), + tree overflow_result = fold_convert_loc(gccloc, TREE_TYPE(left), integer_zero_node); if (this->op_ == OPERATOR_RSHIFT && !this->left_->type()->integer_type()->is_unsigned()) { tree neg = - fold_build2_loc(this->location().gcc_location(), LT_EXPR, - boolean_type_node, left, - fold_convert_loc(this->location().gcc_location(), - TREE_TYPE(left), + fold_build2_loc(gccloc, LT_EXPR, boolean_type_node, + left, + fold_convert_loc(gccloc, TREE_TYPE(left), integer_zero_node)); tree neg_one = - fold_build2_loc(this->location().gcc_location(), - MINUS_EXPR, TREE_TYPE(left), - fold_convert_loc(this->location().gcc_location(), - TREE_TYPE(left), + fold_build2_loc(gccloc, MINUS_EXPR, TREE_TYPE(left), + fold_convert_loc(gccloc, TREE_TYPE(left), integer_zero_node), - fold_convert_loc(this->location().gcc_location(), - TREE_TYPE(left), + fold_convert_loc(gccloc, TREE_TYPE(left), integer_one_node)); overflow_result = - fold_build3_loc(this->location().gcc_location(), COND_EXPR, - TREE_TYPE(left), neg, neg_one, - overflow_result); + fold_build3_loc(gccloc, COND_EXPR, TREE_TYPE(left), + neg, neg_one, overflow_result); } - ret = fold_build3_loc(this->location().gcc_location(), COND_EXPR, - TREE_TYPE(left), compare, ret, overflow_result); + ret = fold_build3_loc(gccloc, COND_EXPR, TREE_TYPE(left), + compare, ret, overflow_result); + + if (eval_saved != NULL_TREE) + ret = fold_build2_loc(gccloc, COMPOUND_EXPR, TREE_TYPE(ret), + eval_saved, ret); + } + + // Add checks for division by zero and division overflow as needed. + if (is_idiv_op) + { + if (go_check_divide_zero) + { + // right == 0 + tree check = fold_build2_loc(gccloc, EQ_EXPR, boolean_type_node, + right, + fold_convert_loc(gccloc, + TREE_TYPE(right), + integer_zero_node)); + + // __go_runtime_error(RUNTIME_ERROR_DIVISION_BY_ZERO), 0 + int errcode = RUNTIME_ERROR_DIVISION_BY_ZERO; + tree panic = fold_build2_loc(gccloc, COMPOUND_EXPR, TREE_TYPE(ret), + Gogo::runtime_error(errcode, + this->location()), + fold_convert_loc(gccloc, TREE_TYPE(ret), + integer_zero_node)); + + // right == 0 ? (__go_runtime_error(...), 0) : ret + ret = fold_build3_loc(gccloc, COND_EXPR, TREE_TYPE(ret), + check, panic, ret); + } + + if (go_check_divide_overflow) + { + // right == -1 + // FIXME: It would be nice to say that this test is expected + // to return false. + tree m1 = integer_minus_one_node; + tree check = fold_build2_loc(gccloc, EQ_EXPR, boolean_type_node, + right, + fold_convert_loc(gccloc, + TREE_TYPE(right), + m1)); + + tree overflow; + if (TYPE_UNSIGNED(TREE_TYPE(ret))) + { + // An unsigned -1 is the largest possible number, so + // dividing is always 1 or 0. + tree cmp = fold_build2_loc(gccloc, EQ_EXPR, boolean_type_node, + left, right); + if (this->op_ == OPERATOR_DIV) + overflow = fold_build3_loc(gccloc, COND_EXPR, TREE_TYPE(ret), + cmp, + fold_convert_loc(gccloc, + TREE_TYPE(ret), + integer_one_node), + fold_convert_loc(gccloc, + TREE_TYPE(ret), + integer_zero_node)); + else + overflow = fold_build3_loc(gccloc, COND_EXPR, TREE_TYPE(ret), + cmp, + fold_convert_loc(gccloc, + TREE_TYPE(ret), + integer_zero_node), + left); + } + else + { + // Computing left / -1 is the same as computing - left, + // which does not overflow since Go sets -fwrapv. + if (this->op_ == OPERATOR_DIV) + overflow = fold_build1_loc(gccloc, NEGATE_EXPR, TREE_TYPE(left), + left); + else + overflow = integer_zero_node; + } + overflow = fold_convert_loc(gccloc, TREE_TYPE(ret), overflow); + + // right == -1 ? - left : ret + ret = fold_build3_loc(gccloc, COND_EXPR, TREE_TYPE(ret), + check, overflow, ret); + } if (eval_saved != NULL_TREE) - ret = fold_build2_loc(this->location().gcc_location(), COMPOUND_EXPR, - TREE_TYPE(ret), eval_saved, ret); + ret = fold_build2_loc(gccloc, COMPOUND_EXPR, TREE_TYPE(ret), + eval_saved, ret); } return ret; Index: libgo/runtime/go-runtime-error.c =================================================================== --- libgo/runtime/go-runtime-error.c (revision 184521) +++ libgo/runtime/go-runtime-error.c (working copy) @@ -46,7 +46,10 @@ enum MAKE_MAP_OUT_OF_BOUNDS = 8, /* Channel capacity out of bounds in make: negative or overflow. */ - MAKE_CHAN_OUT_OF_BOUNDS = 9 + MAKE_CHAN_OUT_OF_BOUNDS = 9, + + /* Integer division by zero. */ + DIVISION_BY_ZERO = 10 }; extern void __go_runtime_error () __attribute__ ((noreturn)); @@ -78,6 +81,9 @@ __go_runtime_error (int i) case MAKE_CHAN_OUT_OF_BOUNDS: runtime_panicstring ("make chan len out of range"); + case DIVISION_BY_ZERO: + runtime_panicstring ("integer divide by zero"); + default: runtime_panicstring ("unknown runtime error"); }