From 6131b3716322d60cf26f7aad1a654dc6c4414051 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 6 May 2016 19:23:21 -0700 Subject: [PATCH] fix eval integer wrapping and add tests See #46 --- src/analyze.cpp | 24 +++-- src/bignum.cpp | 17 +++- src/eval.cpp | 205 ++++++++++++++++++++++++++++++++----------- test/run_tests.cpp | 49 +++++++++++ test/self_hosted.zig | 72 ++++++++------- 5 files changed, 269 insertions(+), 98 deletions(-) diff --git a/src/analyze.cpp b/src/analyze.cpp index 62938ad0fc..7b4aefc0fc 100644 --- a/src/analyze.cpp +++ b/src/analyze.cpp @@ -105,11 +105,25 @@ static AstNode *first_executing_node(AstNode *node) { zig_unreachable(); } +static void mark_impure_fn(BlockContext *context) { + if (context->fn_entry) { + context->fn_entry->is_pure = false; + } +} + ErrorMsg *add_node_error(CodeGen *g, AstNode *node, Buf *msg) { // if this assert fails, then parseh generated code that // failed semantic analysis, which isn't supposed to happen assert(!node->owner->c_import_node); + // if an error occurs in a function then it becomes impure + if (node->block_context) { + FnTableEntry *fn_entry = node->block_context->fn_entry; + if (fn_entry) { + fn_entry->is_pure = false; + } + } + ErrorMsg *err = err_msg_create_with_line(node->owner->path, node->line, node->column, node->owner->source_code, node->owner->line_offsets, msg); @@ -2620,12 +2634,6 @@ static TypeTableEntry *analyze_slice_expr(CodeGen *g, ImportTableEntry *import, return return_type; } -static void mark_impure_fn(BlockContext *context) { - if (context->fn_entry) { - context->fn_entry->is_pure = false; - } -} - static TypeTableEntry *analyze_array_access_expr(CodeGen *g, ImportTableEntry *import, BlockContext *context, AstNode *node) { @@ -5149,7 +5157,7 @@ static TypeTableEntry *analyze_prefix_op_expr(CodeGen *g, ImportTableEntry *impo } case PrefixOpNegation: { - TypeTableEntry *expr_type = analyze_expression(g, import, context, expected_type, *expr_node); + TypeTableEntry *expr_type = analyze_expression(g, import, context, nullptr, *expr_node); if (expr_type->id == TypeTableEntryIdInvalid) { return expr_type; } else if ((expr_type->id == TypeTableEntryIdInt && @@ -5762,6 +5770,7 @@ static TypeTableEntry *analyze_expression_pointer_only(CodeGen *g, ImportTableEn { assert(!expected_type || expected_type->id != TypeTableEntryIdInvalid); TypeTableEntry *return_type = nullptr; + node->block_context = context; switch (node->type) { case NodeTypeBlock: return_type = analyze_block_expr(g, import, context, expected_type, node); @@ -5889,7 +5898,6 @@ static TypeTableEntry *analyze_expression_pointer_only(CodeGen *g, ImportTableEn Expr *expr = get_resolved_expr(node); expr->type_entry = return_type; - node->block_context = context; add_global_const_expr(g, node); diff --git a/src/bignum.cpp b/src/bignum.cpp index 7046ff4874..1e90b201b0 100644 --- a/src/bignum.cpp +++ b/src/bignum.cpp @@ -45,11 +45,21 @@ bool bignum_fits_in_bits(BigNum *bn, int bit_count, bool is_signed) { assert(bn->kind == BigNumKindInt); if (is_signed) { - if (bn->data.x_uint <= ((uint64_t)(INT8_MAX)) + 1) { + if (bn->is_negative) { + if (bn->data.x_uint <= ((uint64_t)INT8_MAX) + 1) { + return bit_count >= 8; + } else if (bn->data.x_uint <= ((uint64_t)INT16_MAX) + 1) { + return bit_count >= 16; + } else if (bn->data.x_uint <= ((uint64_t)INT32_MAX) + 1) { + return bit_count >= 32; + } else { + return bit_count >= 64; + } + } else if (bn->data.x_uint <= (uint64_t)INT8_MAX) { return bit_count >= 8; - } else if (bn->data.x_uint <= ((uint64_t)(INT16_MAX)) + 1) { + } else if (bn->data.x_uint <= (uint64_t)INT16_MAX) { return bit_count >= 16; - } else if (bn->data.x_uint <= ((uint64_t)(INT32_MAX)) + 1) { + } else if (bn->data.x_uint <= (uint64_t)INT32_MAX) { return bit_count >= 32; } else { return bit_count >= 64; @@ -98,6 +108,7 @@ bool bignum_add(BigNum *dest, BigNum *op1, BigNum *op2) { } if (op1->is_negative == op2->is_negative) { + dest->is_negative = op1->is_negative; return __builtin_uaddll_overflow(op1->data.x_uint, op2->data.x_uint, &dest->data.x_uint); } else if (!op1->is_negative && op2->is_negative) { if (__builtin_usubll_overflow(op1->data.x_uint, op2->data.x_uint, &dest->data.x_uint)) { diff --git a/src/eval.cpp b/src/eval.cpp index a2bea04ab7..3befeb2d83 100644 --- a/src/eval.cpp +++ b/src/eval.cpp @@ -99,14 +99,78 @@ static bool eval_bool_bin_op_bool(bool a, BinOpType bin_op, bool b) { } } +static uint64_t max_unsigned_val(TypeTableEntry *type_entry) { + assert(type_entry->id == TypeTableEntryIdInt); + if (type_entry->data.integral.bit_count == 64) { + return UINT64_MAX; + } else if (type_entry->data.integral.bit_count == 32) { + return UINT32_MAX; + } else if (type_entry->data.integral.bit_count == 16) { + return UINT16_MAX; + } else if (type_entry->data.integral.bit_count == 8) { + return UINT8_MAX; + } else { + zig_unreachable(); + } +} + +static int64_t max_signed_val(TypeTableEntry *type_entry) { + assert(type_entry->id == TypeTableEntryIdInt); + if (type_entry->data.integral.bit_count == 64) { + return INT64_MAX; + } else if (type_entry->data.integral.bit_count == 32) { + return INT32_MAX; + } else if (type_entry->data.integral.bit_count == 16) { + return INT16_MAX; + } else if (type_entry->data.integral.bit_count == 8) { + return INT8_MAX; + } else { + zig_unreachable(); + } +} + +static int64_t min_signed_val(TypeTableEntry *type_entry) { + assert(type_entry->id == TypeTableEntryIdInt); + if (type_entry->data.integral.bit_count == 64) { + return INT64_MIN; + } else if (type_entry->data.integral.bit_count == 32) { + return INT32_MIN; + } else if (type_entry->data.integral.bit_count == 16) { + return INT16_MIN; + } else if (type_entry->data.integral.bit_count == 8) { + return INT8_MIN; + } else { + zig_unreachable(); + } +} + static int eval_const_expr_bin_op_bignum(ConstExprValue *op1_val, ConstExprValue *op2_val, - ConstExprValue *out_val, bool (*bignum_fn)(BigNum *, BigNum *, BigNum *)) + ConstExprValue *out_val, bool (*bignum_fn)(BigNum *, BigNum *, BigNum *), + TypeTableEntry *type) { bool overflow = bignum_fn(&out_val->data.x_bignum, &op1_val->data.x_bignum, &op2_val->data.x_bignum); if (overflow) { return ErrorOverflow; } + if (type->id == TypeTableEntryIdInt && !bignum_fits_in_bits(&out_val->data.x_bignum, + type->data.integral.bit_count, type->data.integral.is_signed)) + { + if (type->data.integral.is_wrapping) { + if (type->data.integral.is_signed) { + out_val->data.x_bignum.data.x_uint = max_unsigned_val(type) - out_val->data.x_bignum.data.x_uint + 1; + out_val->data.x_bignum.is_negative = !out_val->data.x_bignum.is_negative; + } else if (out_val->data.x_bignum.is_negative) { + out_val->data.x_bignum.data.x_uint = max_unsigned_val(type) - out_val->data.x_bignum.data.x_uint + 1; + out_val->data.x_bignum.is_negative = false; + } else { + bignum_truncate(&out_val->data.x_bignum, type->data.integral.bit_count); + } + } else { + return ErrorOverflow; + } + } + out_val->ok = true; out_val->depends_on_compile_var = op1_val->depends_on_compile_var || op2_val->depends_on_compile_var; return 0; @@ -117,6 +181,8 @@ int eval_const_expr_bin_op(ConstExprValue *op1_val, TypeTableEntry *op1_type, { assert(op1_val->ok); assert(op2_val->ok); + assert(op1_type->id != TypeTableEntryIdInvalid); + assert(op2_type->id != TypeTableEntryIdInvalid); switch (bin_op) { case BinOpTypeAssign: @@ -132,8 +198,7 @@ int eval_const_expr_bin_op(ConstExprValue *op1_val, TypeTableEntry *op1_type, case BinOpTypeAssignBitOr: case BinOpTypeAssignBoolAnd: case BinOpTypeAssignBoolOr: - out_val->ok = true; - return 0; + zig_unreachable(); case BinOpTypeBoolOr: case BinOpTypeBoolAnd: assert(op1_type->id == TypeTableEntryIdBool); @@ -191,21 +256,21 @@ int eval_const_expr_bin_op(ConstExprValue *op1_val, TypeTableEntry *op1_type, return 0; } case BinOpTypeAdd: - return eval_const_expr_bin_op_bignum(op1_val, op2_val, out_val, bignum_add); + return eval_const_expr_bin_op_bignum(op1_val, op2_val, out_val, bignum_add, op1_type); case BinOpTypeBinOr: - return eval_const_expr_bin_op_bignum(op1_val, op2_val, out_val, bignum_or); + return eval_const_expr_bin_op_bignum(op1_val, op2_val, out_val, bignum_or, op1_type); case BinOpTypeBinXor: - return eval_const_expr_bin_op_bignum(op1_val, op2_val, out_val, bignum_xor); + return eval_const_expr_bin_op_bignum(op1_val, op2_val, out_val, bignum_xor, op1_type); case BinOpTypeBinAnd: - return eval_const_expr_bin_op_bignum(op1_val, op2_val, out_val, bignum_and); + return eval_const_expr_bin_op_bignum(op1_val, op2_val, out_val, bignum_and, op1_type); case BinOpTypeBitShiftLeft: - return eval_const_expr_bin_op_bignum(op1_val, op2_val, out_val, bignum_shl); + return eval_const_expr_bin_op_bignum(op1_val, op2_val, out_val, bignum_shl, op1_type); case BinOpTypeBitShiftRight: - return eval_const_expr_bin_op_bignum(op1_val, op2_val, out_val, bignum_shr); + return eval_const_expr_bin_op_bignum(op1_val, op2_val, out_val, bignum_shr, op1_type); case BinOpTypeSub: - return eval_const_expr_bin_op_bignum(op1_val, op2_val, out_val, bignum_sub); + return eval_const_expr_bin_op_bignum(op1_val, op2_val, out_val, bignum_sub, op1_type); case BinOpTypeMult: - return eval_const_expr_bin_op_bignum(op1_val, op2_val, out_val, bignum_mul); + return eval_const_expr_bin_op_bignum(op1_val, op2_val, out_val, bignum_mul, op1_type); case BinOpTypeDiv: { bool is_int = false; @@ -224,11 +289,11 @@ int eval_const_expr_bin_op(ConstExprValue *op1_val, TypeTableEntry *op1_type, { return ErrorDivByZero; } else { - return eval_const_expr_bin_op_bignum(op1_val, op2_val, out_val, bignum_div); + return eval_const_expr_bin_op_bignum(op1_val, op2_val, out_val, bignum_div, op1_type); } } case BinOpTypeMod: - return eval_const_expr_bin_op_bignum(op1_val, op2_val, out_val, bignum_mod); + return eval_const_expr_bin_op_bignum(op1_val, op2_val, out_val, bignum_mod, op1_type); case BinOpTypeUnwrapMaybe: zig_panic("TODO"); case BinOpTypeStrCat: @@ -244,18 +309,61 @@ static bool eval_bin_op_expr(EvalFn *ef, AstNode *node, ConstExprValue *out_val) AstNode *op1 = node->data.bin_op_expr.op1; AstNode *op2 = node->data.bin_op_expr.op2; + BinOpType bin_op = node->data.bin_op_expr.bin_op; + + switch (bin_op) { + case BinOpTypeAssign: + case BinOpTypeAssignTimes: + case BinOpTypeAssignDiv: + case BinOpTypeAssignMod: + case BinOpTypeAssignPlus: + case BinOpTypeAssignMinus: + case BinOpTypeAssignBitShiftLeft: + case BinOpTypeAssignBitShiftRight: + case BinOpTypeAssignBitAnd: + case BinOpTypeAssignBitXor: + case BinOpTypeAssignBitOr: + case BinOpTypeAssignBoolAnd: + case BinOpTypeAssignBoolOr: + zig_panic("TODO"); + case BinOpTypeBoolOr: + case BinOpTypeBoolAnd: + case BinOpTypeCmpEq: + case BinOpTypeCmpNotEq: + case BinOpTypeCmpLessThan: + case BinOpTypeCmpGreaterThan: + case BinOpTypeCmpLessOrEq: + case BinOpTypeCmpGreaterOrEq: + case BinOpTypeBinOr: + case BinOpTypeBinXor: + case BinOpTypeBinAnd: + case BinOpTypeBitShiftLeft: + case BinOpTypeBitShiftRight: + case BinOpTypeAdd: + case BinOpTypeSub: + case BinOpTypeMult: + case BinOpTypeDiv: + case BinOpTypeMod: + case BinOpTypeUnwrapMaybe: + case BinOpTypeStrCat: + case BinOpTypeArrayMult: + break; + case BinOpTypeInvalid: + zig_unreachable(); + } TypeTableEntry *op1_type = get_resolved_expr(op1)->type_entry; TypeTableEntry *op2_type = get_resolved_expr(op2)->type_entry; + assert(op1_type); + assert(op2_type); + ConstExprValue op1_val = {0}; if (eval_expr(ef, op1, &op1_val)) return true; ConstExprValue op2_val = {0}; if (eval_expr(ef, op2, &op2_val)) return true; - BinOpType bin_op = node->data.bin_op_expr.bin_op; - int err; if ((err = eval_const_expr_bin_op(&op1_val, op1_type, bin_op, &op2_val, op2_type, out_val))) { ef->root->abort = true; @@ -568,48 +676,15 @@ void eval_min_max_value(CodeGen *g, TypeTableEntry *type_entry, ConstExprValue * const_val->depends_on_compile_var = int_type_depends_on_compile_var(g, type_entry); if (is_max) { if (type_entry->data.integral.is_signed) { - int64_t val; - if (type_entry->data.integral.bit_count == 64) { - val = INT64_MAX; - } else if (type_entry->data.integral.bit_count == 32) { - val = INT32_MAX; - } else if (type_entry->data.integral.bit_count == 16) { - val = INT16_MAX; - } else if (type_entry->data.integral.bit_count == 8) { - val = INT8_MAX; - } else { - zig_unreachable(); - } + int64_t val = max_signed_val(type_entry); bignum_init_signed(&const_val->data.x_bignum, val); } else { - uint64_t val; - if (type_entry->data.integral.bit_count == 64) { - val = UINT64_MAX; - } else if (type_entry->data.integral.bit_count == 32) { - val = UINT32_MAX; - } else if (type_entry->data.integral.bit_count == 16) { - val = UINT16_MAX; - } else if (type_entry->data.integral.bit_count == 8) { - val = UINT8_MAX; - } else { - zig_unreachable(); - } + uint64_t val = max_unsigned_val(type_entry); bignum_init_unsigned(&const_val->data.x_bignum, val); } } else { if (type_entry->data.integral.is_signed) { - int64_t val; - if (type_entry->data.integral.bit_count == 64) { - val = INT64_MIN; - } else if (type_entry->data.integral.bit_count == 32) { - val = INT32_MIN; - } else if (type_entry->data.integral.bit_count == 16) { - val = INT16_MIN; - } else if (type_entry->data.integral.bit_count == 8) { - val = INT8_MIN; - } else { - zig_unreachable(); - } + int64_t val = min_signed_val(type_entry); bignum_init_signed(&const_val->data.x_bignum, val); } else { bignum_init_unsigned(&const_val->data.x_bignum, 0); @@ -687,6 +762,8 @@ static bool eval_fn_call_builtin(EvalFn *ef, AstNode *node, ConstExprValue *out_ return eval_fn_with_overflow(ef, node, out_val, bignum_add); case BuiltinFnIdSubWithOverflow: return eval_fn_with_overflow(ef, node, out_val, bignum_sub); + case BuiltinFnIdShlWithOverflow: + return eval_fn_with_overflow(ef, node, out_val, bignum_shl); case BuiltinFnIdFence: return false; case BuiltinFnIdMemcpy: @@ -707,7 +784,6 @@ static bool eval_fn_call_builtin(EvalFn *ef, AstNode *node, ConstExprValue *out_ case BuiltinFnIdErrName: case BuiltinFnIdEmbedFile: case BuiltinFnIdCmpExchange: - case BuiltinFnIdShlWithOverflow: zig_panic("TODO"); case BuiltinFnIdBreakpoint: case BuiltinFnIdInvalid: @@ -962,8 +1038,31 @@ static bool eval_prefix_op_expr(EvalFn *ef, AstNode *node, ConstExprValue *out_v out_val->ok = true; break; } - case PrefixOpBinNot: case PrefixOpNegation: + if (expr_type->id == TypeTableEntryIdInt) { + assert(expr_type->data.integral.is_signed); + bignum_negate(&out_val->data.x_bignum, &expr_val.data.x_bignum); + out_val->ok = true; + bool overflow = !bignum_fits_in_bits(&out_val->data.x_bignum, + expr_type->data.integral.bit_count, expr_type->data.integral.is_signed); + if (expr_type->data.integral.is_wrapping) { + if (overflow) { + out_val->data.x_bignum.is_negative = true; + } + } else if (overflow) { + ErrorMsg *msg = add_node_error(ef->root->codegen, ef->root->fn->fn_def_node, + buf_sprintf("function evaluation caused overflow")); + add_error_note(ef->root->codegen, msg, ef->root->call_node, buf_sprintf("called from here")); + add_error_note(ef->root->codegen, msg, node, buf_sprintf("overflow occurred here")); + return true; + } + } else if (expr_type->id == TypeTableEntryIdFloat) { + zig_panic("TODO"); + } else { + zig_unreachable(); + } + break; + case PrefixOpBinNot: case PrefixOpMaybe: case PrefixOpError: case PrefixOpUnwrapError: diff --git a/test/run_tests.cpp b/test/run_tests.cpp index 81911671b6..f183b648e2 100644 --- a/test/run_tests.cpp +++ b/test/run_tests.cpp @@ -1305,6 +1305,55 @@ fn f() { )SOURCE", 2, ".tmp_source.zig:4:72: error: failure atomic ordering must be no stricter than success", ".tmp_source.zig:5:49: error: success atomic ordering must be Monotonic or stricter"); + + add_compile_fail_case("negation overflow in function evaluation", R"SOURCE( +fn f() { + const x = neg(-128); +} +fn neg(x: i8) -> i8 { + -x +} + )SOURCE", 3, + ".tmp_source.zig:5:1: error: function evaluation caused overflow", + ".tmp_source.zig:3:18: note: called from here", + ".tmp_source.zig:6:5: note: overflow occurred here"); + + add_compile_fail_case("add overflow in function evaluation", R"SOURCE( +fn f() { + const x = add(65530, 10); +} +fn add(a: u16, b: u16) -> u16 { + a + b +} + )SOURCE", 3, + ".tmp_source.zig:5:1: error: function evaluation caused overflow", + ".tmp_source.zig:3:18: note: called from here", + ".tmp_source.zig:6:7: note: overflow occurred here"); + + + add_compile_fail_case("sub overflow in function evaluation", R"SOURCE( +fn f() { + const x = sub(10, 20); +} +fn sub(a: u16, b: u16) -> u16 { + a - b +} + )SOURCE", 3, + ".tmp_source.zig:5:1: error: function evaluation caused overflow", + ".tmp_source.zig:3:18: note: called from here", + ".tmp_source.zig:6:7: note: overflow occurred here"); + + add_compile_fail_case("mul overflow in function evaluation", R"SOURCE( +fn f() { + const x = mul(300, 6000); +} +fn mul(a: u16, b: u16) -> u16 { + a * b +} + )SOURCE", 3, + ".tmp_source.zig:5:1: error: function evaluation caused overflow", + ".tmp_source.zig:3:18: note: called from here", + ".tmp_source.zig:6:7: note: overflow occurred here"); } ////////////////////////////////////////////////////////////////////////////// diff --git a/test/self_hosted.zig b/test/self_hosted.zig index 2e6cc6a3e5..7d2d5ff144 100644 --- a/test/self_hosted.zig +++ b/test/self_hosted.zig @@ -1459,68 +1459,72 @@ fn fence() { #attribute("test") fn unsigned_wrapping() { - var x_u32: u32w = @max_value(u32); - x_u32 += 1; - assert(x_u32 == 0); - x_u32 -= 1; - assert(x_u32 == @max_value(u32)); + test_unsigned_wrapping_eval(@max_value(u32)); test_unsigned_wrapping_noeval(@max_value(u32)); } +fn test_unsigned_wrapping_eval(x: u32w) { + const zero = x + 1; + assert(zero == 0); + const orig = zero - 1; + assert(orig == @max_value(u32)); +} #static_eval_enable(false) fn test_unsigned_wrapping_noeval(x: u32w) { - var x_u32 = x; - x_u32 += 1; - assert(x_u32 == 0); - x_u32 -= 1; - assert(x_u32 == @max_value(u32)); + const zero = x + 1; + assert(zero == 0); + const orig = zero - 1; + assert(orig == @max_value(u32)); } #attribute("test") fn signed_wrapping() { - var x_i32: i32w = @max_value(i32); - x_i32 += 1; - assert(x_i32 == @min_value(i32)); - x_i32 -= 1; - assert(x_i32 == @max_value(i32)); + test_signed_wrapping_eval(@max_value(i32)); test_signed_wrapping_noeval(@max_value(i32)); } +fn test_signed_wrapping_eval(x: i32w) { + const min_val = x + 1; + assert(min_val == @min_value(i32)); + const max_val = min_val - 1; + assert(max_val == @max_value(i32)); +} #static_eval_enable(false) fn test_signed_wrapping_noeval(x: i32w) { - var x_i32 = x; - x_i32 += 1; - assert(x_i32 == @min_value(i32)); - x_i32 -= 1; - assert(x_i32 == @max_value(i32)); + const min_val = x + 1; + assert(min_val == @min_value(i32)); + const max_val = min_val - 1; + assert(max_val == @max_value(i32)); } #attribute("test") fn negation_wrapping() { - var x_i16 = @min_value(i16w); - assert(x_i16 == -32768); - x_i16 = -x_i16; - assert(x_i16 == -32768); + test_negation_wrapping_eval(@min_value(i16)); test_negation_wrapping_noeval(@min_value(i16)); } +fn test_negation_wrapping_eval(x: i16w) { + assert(x == -32768); + const neg = -x; + assert(neg == -32768); +} #static_eval_enable(false) fn test_negation_wrapping_noeval(x: i16w) { - var x_i16 = x; - assert(x_i16 == -32768); - x_i16 = -x_i16; - assert(x_i16 == -32768); + assert(x == -32768); + const neg = -x; + assert(neg == -32768); } #attribute("test") fn shl_wrapping() { - var x_u16 = @max_value(u16w); - x_u16 <<= 1; - assert(x_u16 == 65534); + test_shl_wrapping_eval(@max_value(u16)); test_shl_wrapping_noeval(@max_value(u16)); } +fn test_shl_wrapping_eval(x: u16w) { + const shifted = x << 1; + assert(shifted == 65534); +} #static_eval_enable(false) fn test_shl_wrapping_noeval(x: u16w) { - var x_u16 = x; - x_u16 <<= 1; - assert(x_u16 == 65534); + const shifted = x << 1; + assert(shifted == 65534); } #attribute("test")