From 093ffd7b5784745f23c0bd4f601d91c467defd08 Mon Sep 17 00:00:00 2001 From: zerbina <100542850+zerbina@users.noreply.github.com> Date: Sun, 4 Feb 2024 17:56:36 +0100 Subject: [PATCH] mir: add unchecked signed-integer arithmetic ops (#1164) ## Summary Which signed integer arithmetic operation are checked for overflow is now made explicit within the MIR. This: * makes the exceptional control-flow arising from overflow checks visible to data-flow analysis, fixing destructors sometimes not being run * moves decision-making regarding overflow checks out of the code generators and into a central place (the AST -> MIR translation) ## Details In order to give the code generators more freedom over how they implement the overflow checks, no separate magics that implement the checks are introduced. ### MIR additions * add built-in signed integer arithmetic unary operators (negation) * add built-in signed integer arithmetic binary operators (addition, subtraction, multiplication, division, and remainder) * the behaviour of the built-in operators regarding over- and underflow, as well as division-by-zero, is undefined ### General architecture * when overflow checks are enabled, signed integer arithmetic operations are kept as magic calls (using `mnkCheckedCall` if panics are disabled) * when overflow checks are disabled, signed integer arithmetic operations are translated to use the MIR built-in operators * signed integer arithmetic operators kept as magic calls are unconditionally translated to checked operations by the code generators * the built-in arithmetic operators are unconditionally translated to unchecked operations (if available) by the code generators * the overflow check behaviour of `abs` is still controlled individually by each code generator ### AST-to-MIR translation * `mSucc` (successor) and `mPred` (predecessor) are lowered into normal addition and subtraction operators, respectively. This was previously left to the code generators ### MIR-to-CGIR translation * built-in operations equivalent to the ones added to the MIR are added to the `CgNode` IR * the MIR's arithmetic operations have a one-to-one correspondence with the `CgNode` ones ### Code generation * `vmgen` generates `Addu`, `Subu`, and `Mulu` for unchecked signed additions, subtractions, and multiplications. For a lack of unchecked versions, negation, division, and remainder still use the checked opcodes * `cgen` and `jsgen` generate the same code for the unchecked built-in operators as they previously did for unchecked arithmetic magics * all three code generators ignore `mSucc` and `mPred` ### Other * `mirexec` treats the built-in arithmetic operations operands as usages * the MIR built-in arithmetic operators are rendered as how they would look like in the high-level language (`+`, `*`, `div`, etc.) --------- Co-authored-by: Saem Ghani --- compiler/backend/ccgexprs.nim | 69 +++-- compiler/backend/cgir.nim | 8 + compiler/backend/cgirgen.nim | 9 + compiler/backend/jsgen.nim | 266 ++++++++++-------- compiler/mir/mirgen.nim | 55 +++- compiler/mir/mirtrees.nim | 20 +- compiler/mir/utils.nim | 13 + compiler/sem/injectdestructors.nim | 3 +- compiler/sem/mirexec.nim | 5 + compiler/vm/vmgen.nim | 25 +- doc/mir.rst | 14 +- tests/arc/topt_no_cursor.nim | 4 +- tests/arc/topt_wasmoved_destroy_pairs.nim | 2 +- .../tdestruction_when_checks_failed.nim | 10 +- tests/lang_objects/destructor/tv2_cast.nim | 2 +- tests/overflw/toverflow_abs.nim | 24 ++ 16 files changed, 359 insertions(+), 170 deletions(-) create mode 100644 tests/overflw/toverflow_abs.nim diff --git a/compiler/backend/ccgexprs.nim b/compiler/backend/ccgexprs.nim index 207f434593c..fad306744b3 100644 --- a/compiler/backend/ccgexprs.nim +++ b/compiler/backend/ccgexprs.nim @@ -303,17 +303,14 @@ template binaryArithOverflowRaw(p: BProc, t: PType, a, b: TLoc; proc binaryArithOverflow(p: BProc, e: CgNode, d: var TLoc, m: TMagic) = const - prc: array[mAddI..mPred, string] = [ + prc: array[mAddI..mModI, string] = [ "nimAddInt", "nimSubInt", - "nimMulInt", "nimDivInt", "nimModInt", - "nimAddInt", "nimSubInt" + "nimMulInt", "nimDivInt", "nimModInt" ] - prc64: array[mAddI..mPred, string] = [ + prc64: array[mAddI..mModI, string] = [ "nimAddInt64", "nimSubInt64", - "nimMulInt64", "nimDivInt64", "nimModInt64", - "nimAddInt64", "nimSubInt64" + "nimMulInt64", "nimDivInt64", "nimModInt64" ] - opr: array[mAddI..mPred, string] = ["+", "-", "*", "/", "%", "+", "-"] var a, b: TLoc assert(e[1].typ != nil) assert(e[2].typ != nil) @@ -322,10 +319,7 @@ proc binaryArithOverflow(p: BProc, e: CgNode, d: var TLoc, m: TMagic) = # skipping 'range' is correct here as we'll generate a proper range check # later via 'chckRange' let t = e.typ.skipTypes(abstractRange) - if optOverflowCheck notin p.options: - let res = "($1)($2 $3 $4)" % [getTypeDesc(p.module, e.typ), rdLoc(a), rope(opr[m]), rdLoc(b)] - putIntoDest(p, d, e, res) - else: + if true: # we handle div by zero here so that we know that the compilerproc's # result is only for overflows. if m in {mDivI, mModI}: @@ -343,9 +337,8 @@ proc unaryArithOverflow(p: BProc, e: CgNode, d: var TLoc, m: TMagic) = assert(e[1].typ != nil) initLocExpr(p, e[1], a) t = skipTypes(e.typ, abstractRange) - if optOverflowCheck in p.options: - linefmt(p, cpsStmts, "if ($1 == $2){ #raiseOverflow(); $3}$n", - [rdLoc(a), intLiteral(firstOrd(p.config, t)), raiseInstr(p)]) + linefmt(p, cpsStmts, "if ($1 == $2){ #raiseOverflow(); $3}$n", + [rdLoc(a), intLiteral(firstOrd(p.config, t)), raiseInstr(p)]) case m of mUnaryMinusI: putIntoDest(p, d, e, "((NI$2)-($1))" % [rdLoc(a), rope(getSize(p.config, t) * 8)]) @@ -356,14 +349,14 @@ proc unaryArithOverflow(p: BProc, e: CgNode, d: var TLoc, m: TMagic) = else: assert(false, $m) -proc binaryArith(p: BProc, e: CgNode, d: var TLoc, op: TMagic) = +proc binaryArith(p: BProc, e, x, y: CgNode, d: var TLoc, op: TMagic) = var a, b: TLoc s, k: BiggestInt - assert(e[1].typ != nil) - assert(e[2].typ != nil) - initLocExpr(p, e[1], a) - initLocExpr(p, e[2], b) + assert(x.typ != nil) + assert(y.typ != nil) + initLocExpr(p, x, a) + initLocExpr(p, y, b) # BUGFIX: cannot use result-type here, as it may be a boolean s = max(getSize(p.config, a.t), getSize(p.config, b.t)) * 8 k = getSize(p.config, a.t) * 8 @@ -375,6 +368,11 @@ proc binaryArith(p: BProc, e: CgNode, d: var TLoc, op: TMagic) = ) case op + of mAddI: applyFormat("($4)($1 + $2)") + of mSubI: applyFormat("($4)($1 - $2)") + of mMulI: applyFormat("($4)($1 * $2)") + of mDivI: applyFormat("($4)($1 / $2)") + of mModI: applyFormat("($4)($1 % $2)") of mAddF64: applyFormat("(($4)($1) + ($4)($2))") of mSubF64: applyFormat("(($4)($1) - ($4)($2))") of mMulF64: applyFormat("(($4)($1) * ($4)($2))") @@ -435,12 +433,12 @@ proc genIsNil(p: BProc, e: CgNode, d: var TLoc) = else: unaryExpr(p, e, d, "($1 == 0)") -proc unaryArith(p: BProc, e: CgNode, d: var TLoc, op: TMagic) = +proc unaryArith(p: BProc, e, x: CgNode, d: var TLoc, op: TMagic) = var a: TLoc t: PType - assert(e[1].typ != nil) - initLocExpr(p, e[1], a) + assert(x.typ != nil) + initLocExpr(p, x, a) t = skipTypes(e.typ, abstractRange) template applyFormat(frmt: untyped) = @@ -455,8 +453,12 @@ proc unaryArith(p: BProc, e: CgNode, d: var TLoc, op: TMagic) = applyFormat("($3)((NU$2) ~($1))") of mUnaryPlusF64: applyFormat("$1") - of mUnaryMinusF64: + of mUnaryMinusF64, mUnaryMinusI64: applyFormat("-($1)") + of mUnaryMinusI: + applyFormat("((NI$2)-($1))") + of mAbsI: + applyFormat("($1 > 0? ($1) : -($1))") else: assert false, $op @@ -1571,7 +1573,7 @@ proc binaryFloatArith(p: BProc, e: CgNode, d: var TLoc, m: TMagic) = if optInfCheck in p.options: linefmt(p, cpsStmts, "if ($1 != 0.0 && $1*0.5 == $1) { #raiseFloatOverflow($1); $2}$n", [rdLoc(d), raiseInstr(p)]) else: - binaryArith(p, e, d, m) + binaryArith(p, e, e[1], e[2], d, m) proc skipAddr(n: CgNode): CgNode = if n.kind == cnkHiddenAddr: n.operand @@ -1649,10 +1651,16 @@ proc genBreakState(p: BProc, n: CgNode, d: var TLoc) = proc genMagicExpr(p: BProc, e: CgNode, d: var TLoc, op: TMagic) = case op - of mNot..mUnaryMinusF64: unaryArith(p, e, d, op) - of mUnaryMinusI..mAbsI: unaryArithOverflow(p, e, d, op) + of mNot..mUnaryMinusF64: unaryArith(p, e, e[1], d, op) + of mUnaryMinusI, mUnaryMinusI64: unaryArithOverflow(p, e, d, op) + of mAbsI: + # TODO: lower the unchecked ``abs`` variant earlier + if optOverflowCheck in p.options: + unaryArithOverflow(p, e, d, op) + else: + unaryArith(p, e, e[1], d, op) of mAddF64..mDivF64: binaryFloatArith(p, e, d, op) - of mShrI..mXor: binaryArith(p, e, d, op) + of mShrI..mXor: binaryArith(p, e, e[1], e[2], d, op) of mEqProc: genEqProc(p, e, d) of mAddI..mPred: binaryArithOverflow(p, e, d, op) of mGetTypeInfo: genGetTypeInfo(p, e, d) @@ -2049,6 +2057,13 @@ proc expr(p: BProc, n: CgNode, d: var TLoc) = genMagicExpr(p, n, d, m) else: genCall(p, n, d) + # unchecked arithmetic operations: + of cnkNegI: unaryArith(p, n, n[0], d, mUnaryMinusI) + of cnkAddI: binaryArith(p, n, n[0], n[1], d, mAddI) + of cnkSubI: binaryArith(p, n, n[0], n[1], d, mSubI) + of cnkMulI: binaryArith(p, n, n[0], n[1], d, mMulI) + of cnkDivI: binaryArith(p, n, n[0], n[1], d, mDivI) + of cnkModI: binaryArith(p, n, n[0], n[1], d, mModI) of cnkSetConstr: genSetConstr(p, n, d) of cnkArrayConstr: diff --git a/compiler/backend/cgir.nim b/compiler/backend/cgir.nim index 976a70fa42d..7eb88f86f71 100644 --- a/compiler/backend/cgir.nim +++ b/compiler/backend/cgir.nim @@ -48,6 +48,14 @@ type ## the following operands the arguments cnkCheckedCall ## like ``cnkCall``, but the call might raise an exception + # arithmetic operations: + cnkNegI + cnkAddI + cnkSubI + cnkMulI + cnkDivI + cnkModI + # constructors: cnkTupleConstr ## tuple constructor cnkObjConstr ## object constructor diff --git a/compiler/backend/cgirgen.nim b/compiler/backend/cgirgen.nim index 25104e6928e..1d0ca2663c8 100644 --- a/compiler/backend/cgirgen.nim +++ b/compiler/backend/cgirgen.nim @@ -801,6 +801,15 @@ proc exprToIr(tree: MirBody, cl: var TranslateCl, res.add argToIr(tree, cl, cr)[1] of mnkCall, mnkCheckedCall: callToIr(tree, cl, n, cr) + of UnaryOps: + const Map = [mnkNegI: cnkNegI] + treeOp Map[n.kind]: + res.add valueToIr(tree, cl, cr) + of BinaryOps: + const Map = [mnkAddI: cnkAddI, mnkSubI: cnkSubI, + mnkMulI: cnkMulI, mnkDivI: cnkDivI, mnkModI: cnkModI] + treeOp Map[n.kind]: + res.kids = @[valueToIr(tree, cl, cr), valueToIr(tree, cl, cr)] of AllNodeKinds - ExprKinds - {mnkNone}: unreachable(n.kind) diff --git a/compiler/backend/jsgen.nim b/compiler/backend/jsgen.nim index bf563ae98d2..5b8b68e58a3 100644 --- a/compiler/backend/jsgen.nim +++ b/compiler/backend/jsgen.nim @@ -422,66 +422,65 @@ proc getTemp(p: PProc, defineInLocals: bool = true): Rope = p.defs.add(p.indentLine("var $1;$n" % [result])) type - TMagicFrmt = array[0..1, string] - TMagicOps = array[mAddI..mUnaryMinusF64, TMagicFrmt] + TMagicOps = array[mAddI..mUnaryMinusF64, string] -const # magic checked op; magic unchecked op; +const # magic checked op jsMagics: TMagicOps = [ - mAddI: ["addInt", ""], - mSubI: ["subInt", ""], - mMulI: ["mulInt", ""], - mDivI: ["divInt", ""], - mModI: ["modInt", ""], - mSucc: ["addInt", ""], - mPred: ["subInt", ""], - mAddF64: ["", ""], - mSubF64: ["", ""], - mMulF64: ["", ""], - mDivF64: ["", ""], - mShrI: ["", ""], - mShlI: ["", ""], - mAshrI: ["", ""], - mBitandI: ["", ""], - mBitorI: ["", ""], - mBitxorI: ["", ""], - mMinI: ["nimMin", "nimMin"], - mMaxI: ["nimMax", "nimMax"], - mAddU: ["", ""], - mSubU: ["", ""], - mMulU: ["", ""], - mDivU: ["", ""], - mModU: ["", ""], - mEqI: ["", ""], - mLeI: ["", ""], - mLtI: ["", ""], - mEqF64: ["", ""], - mLeF64: ["", ""], - mLtF64: ["", ""], - mLeU: ["", ""], - mLtU: ["", ""], - mEqEnum: ["", ""], - mLeEnum: ["", ""], - mLtEnum: ["", ""], - mEqCh: ["", ""], - mLeCh: ["", ""], - mLtCh: ["", ""], - mEqB: ["", ""], - mLeB: ["", ""], - mLtB: ["", ""], - mEqRef: ["", ""], - mLePtr: ["", ""], - mLtPtr: ["", ""], - mXor: ["", ""], - mEqCString: ["", ""], - mEqProc: ["", ""], - mUnaryMinusI: ["negInt", ""], - mUnaryMinusI64: ["negInt64", ""], - mAbsI: ["absInt", ""], - mNot: ["", ""], - mUnaryPlusI: ["", ""], - mBitnotI: ["", ""], - mUnaryPlusF64: ["", ""], - mUnaryMinusF64: ["", ""]] + mAddI: "addInt", + mSubI: "subInt", + mMulI: "mulInt", + mDivI: "divInt", + mModI: "modInt", + mSucc: "addInt", + mPred: "subInt", + mAddF64: "", + mSubF64: "", + mMulF64: "", + mDivF64: "", + mShrI: "", + mShlI: "", + mAshrI: "", + mBitandI: "", + mBitorI: "", + mBitxorI: "", + mMinI: "nimMin", + mMaxI: "nimMax", + mAddU: "", + mSubU: "", + mMulU: "", + mDivU: "", + mModU: "", + mEqI: "", + mLeI: "", + mLtI: "", + mEqF64: "", + mLeF64: "", + mLtF64: "", + mLeU: "", + mLtU: "", + mEqEnum: "", + mLeEnum: "", + mLtEnum: "", + mEqCh: "", + mLeCh: "", + mLtCh: "", + mEqB: "", + mLeB: "", + mLtB: "", + mEqRef: "", + mLePtr: "", + mLtPtr: "", + mXor: "", + mEqCString: "", + mEqProc: "", + mUnaryMinusI: "negInt", + mUnaryMinusI64: "negInt64", + mAbsI: "absInt", + mNot: "", + mUnaryPlusI: "", + mBitnotI: "", + mUnaryPlusF64: "", + mUnaryMinusF64: ""] template binaryExpr(p: PProc, n: CgNode, r: var TCompRes, magic, frmt: string) = # $1 and $2 in the `frmt` string bind to lhs and rhs of the expr, @@ -498,6 +497,12 @@ template binaryExpr(p: PProc, n: CgNode, r: var TCompRes, magic, frmt: string) = r.res = frmt % [a, b, a, b] r.kind = resExpr +template binaryExpr(p: PProc, a, b: CgNode, r: var TCompRes, frmt: string) = + var x, y: TCompRes + gen(p, a, x) + gen(p, b, y) + r.res = frmt % [x.rdLoc, y.rdLoc] + proc unsignedTrimmerJS(size: BiggestInt): Rope = case size of 1: rope"& 0xff" @@ -532,8 +537,8 @@ proc arithAux(p: PProc, n: CgNode, r: var TCompRes, op: TMagic) = var x, y: TCompRes xLoc,yLoc: Rope - let i = ord(optOverflowCheck notin p.options) - useMagic(p, jsMagics[op][i]) + + useMagic(p, jsMagics[op]) if n.len > 2: gen(p, n[1], x) gen(p, n[2], y) @@ -545,75 +550,73 @@ proc arithAux(p: PProc, n: CgNode, r: var TCompRes, op: TMagic) = template applyFormat(frmt) = r.res = frmt % [xLoc, yLoc] - template applyFormat(frmtA, frmtB) = - if i == 0: applyFormat(frmtA) else: applyFormat(frmtB) case op: - of mAddI: applyFormat("addInt($1, $2)", "($1 + $2)") - of mSubI: applyFormat("subInt($1, $2)", "($1 - $2)") - of mMulI: applyFormat("mulInt($1, $2)", "($1 * $2)") - of mDivI: applyFormat("divInt($1, $2)", "Math.trunc($1 / $2)") - of mModI: applyFormat("modInt($1, $2)", "Math.trunc($1 % $2)") - of mSucc: applyFormat("addInt($1, $2)", "($1 + $2)") - of mPred: applyFormat("subInt($1, $2)", "($1 - $2)") - of mAddF64: applyFormat("($1 + $2)", "($1 + $2)") - of mSubF64: applyFormat("($1 - $2)", "($1 - $2)") - of mMulF64: applyFormat("($1 * $2)", "($1 * $2)") - of mDivF64: applyFormat("($1 / $2)", "($1 / $2)") - of mShrI: applyFormat("", "") + of mAddI: applyFormat("addInt($1, $2)") + of mSubI: applyFormat("subInt($1, $2)") + of mMulI: applyFormat("mulInt($1, $2)") + of mDivI: applyFormat("divInt($1, $2)") + of mModI: applyFormat("modInt($1, $2)") + of mSucc: applyFormat("addInt($1, $2)") + of mPred: applyFormat("subInt($1, $2)") + of mAddF64: applyFormat("($1 + $2)") + of mSubF64: applyFormat("($1 - $2)") + of mMulF64: applyFormat("($1 * $2)") + of mDivF64: applyFormat("($1 / $2)") + of mShrI: applyFormat("") of mShlI: if n[1].typ.size <= 4: - applyFormat("($1 << $2)", "($1 << $2)") + applyFormat("($1 << $2)") else: - applyFormat("($1 * Math.pow(2, $2))", "($1 * Math.pow(2, $2))") + applyFormat("($1 * Math.pow(2, $2))") of mAshrI: - if n[1].typ.size <= 4: - applyFormat("($1 >> $2)", "($1 >> $2)") + if n[2].typ.size <= 4: + applyFormat("($1 >> $2)") else: - applyFormat("Math.floor($1 / Math.pow(2, $2))", "Math.floor($1 / Math.pow(2, $2))") - of mBitandI: applyFormat("($1 & $2)", "($1 & $2)") - of mBitorI: applyFormat("($1 | $2)", "($1 | $2)") - of mBitxorI: applyFormat("($1 ^ $2)", "($1 ^ $2)") - of mMinI: applyFormat("nimMin($1, $2)", "nimMin($1, $2)") - of mMaxI: applyFormat("nimMax($1, $2)", "nimMax($1, $2)") - of mAddU: applyFormat("", "") - of mSubU: applyFormat("", "") - of mMulU: applyFormat("", "") - of mDivU: applyFormat("", "") - of mModU: applyFormat("($1 % $2)", "($1 % $2)") - of mEqI: applyFormat("($1 == $2)", "($1 == $2)") - of mLeI: applyFormat("($1 <= $2)", "($1 <= $2)") - of mLtI: applyFormat("($1 < $2)", "($1 < $2)") - of mEqF64: applyFormat("($1 == $2)", "($1 == $2)") - of mLeF64: applyFormat("($1 <= $2)", "($1 <= $2)") - of mLtF64: applyFormat("($1 < $2)", "($1 < $2)") - of mLeU: applyFormat("($1 <= $2)", "($1 <= $2)") - of mLtU: applyFormat("($1 < $2)", "($1 < $2)") - of mEqEnum: applyFormat("($1 == $2)", "($1 == $2)") - of mLeEnum: applyFormat("($1 <= $2)", "($1 <= $2)") - of mLtEnum: applyFormat("($1 < $2)", "($1 < $2)") - of mEqCh: applyFormat("($1 == $2)", "($1 == $2)") - of mLeCh: applyFormat("($1 <= $2)", "($1 <= $2)") - of mLtCh: applyFormat("($1 < $2)", "($1 < $2)") - of mEqB: applyFormat("($1 == $2)", "($1 == $2)") - of mLeB: applyFormat("($1 <= $2)", "($1 <= $2)") - of mLtB: applyFormat("($1 < $2)", "($1 < $2)") - of mEqRef: applyFormat("($1 == $2)", "($1 == $2)") - of mLePtr: applyFormat("($1 <= $2)", "($1 <= $2)") - of mLtPtr: applyFormat("($1 < $2)", "($1 < $2)") - of mXor: applyFormat("($1 != $2)", "($1 != $2)") - of mEqCString: applyFormat("($1 == $2)", "($1 == $2)") - of mEqProc: applyFormat("($1 == $2)", "($1 == $2)") - of mUnaryMinusI: applyFormat("negInt($1)", "-($1)") - of mUnaryMinusI64: applyFormat("negInt64($1)", "-($1)") - of mAbsI: applyFormat("absInt($1)", "Math.abs($1)") - of mNot: applyFormat("!($1)", "!($1)") - of mUnaryPlusI: applyFormat("+($1)", "+($1)") - of mBitnotI: applyFormat("~($1)", "~($1)") - of mUnaryPlusF64: applyFormat("+($1)", "+($1)") - of mUnaryMinusF64: applyFormat("-($1)", "-($1)") + applyFormat("Math.floor($1 / Math.pow(2, $2))") + of mBitandI: applyFormat("($1 & $2)") + of mBitorI: applyFormat("($1 | $2)") + of mBitxorI: applyFormat("($1 ^ $2)") + of mMinI: applyFormat("nimMin($1, $2)") + of mMaxI: applyFormat("nimMax($1, $2)") + of mAddU: applyFormat("") + of mSubU: applyFormat("") + of mMulU: applyFormat("") + of mDivU: applyFormat("") + of mModU: applyFormat("($1 % $2)") + of mEqI: applyFormat("($1 == $2)") + of mLeI: applyFormat("($1 <= $2)") + of mLtI: applyFormat("($1 < $2)") + of mEqF64: applyFormat("($1 == $2)") + of mLeF64: applyFormat("($1 <= $2)") + of mLtF64: applyFormat("($1 < $2)") + of mLeU: applyFormat("($1 <= $2)") + of mLtU: applyFormat("($1 < $2)") + of mEqEnum: applyFormat("($1 == $2)") + of mLeEnum: applyFormat("($1 <= $2)") + of mLtEnum: applyFormat("($1 < $2)") + of mEqCh: applyFormat("($1 == $2)") + of mLeCh: applyFormat("($1 <= $2)") + of mLtCh: applyFormat("($1 < $2)") + of mEqB: applyFormat("($1 == $2)") + of mLeB: applyFormat("($1 <= $2)") + of mLtB: applyFormat("($1 < $2)") + of mEqRef: applyFormat("($1 == $2)") + of mLePtr: applyFormat("($1 <= $2)") + of mLtPtr: applyFormat("($1 < $2)") + of mXor: applyFormat("($1 != $2)") + of mEqCString: applyFormat("($1 == $2)") + of mEqProc: applyFormat("($1 == $2)") + of mUnaryMinusI: applyFormat("negInt($1)") + of mUnaryMinusI64: applyFormat("negInt64($1)") + of mAbsI: applyFormat("absInt($1)") + of mNot: applyFormat("!($1)") + of mUnaryPlusI: applyFormat("+($1)") + of mBitnotI: applyFormat("~($1)") + of mUnaryPlusF64: applyFormat("+($1)") + of mUnaryMinusF64: applyFormat("-($1)") else: - assert false, $op + unreachable(op) proc arith(p: PProc, n: CgNode, r: var TCompRes, op: TMagic) = case op @@ -629,6 +632,13 @@ proc arith(p: PProc, n: CgNode, r: var TCompRes, op: TMagic) = gen(p, n[1], x) gen(p, n[2], y) r.res = "($1 >>> $2)" % [x.rdLoc, y.rdLoc] + of mAbsI: + # TODO: lower the unchecked ``abs`` variant earlier + if optOverflowCheck in p.options: + arithAux(p, n, r, op) + else: + let x = gen(p, n[1]) + r.res = "Math.abs($1)" % rdLoc(x) of mEqRef: if mapType(n[1].typ) != etyBaseIndex: arithAux(p, n, r, op) @@ -907,8 +917,8 @@ proc generateHeader(params: openArray[Loc]): string = const nodeKindsNeedNoCopy = cnkLiterals + { - cnkObjConstr, cnkTupleConstr, cnkArrayConstr, - cnkCall, cnkCheckedCall} + cnkObjConstr, cnkTupleConstr, cnkArrayConstr, cnkCall, cnkCheckedCall, + cnkNegI, cnkAddI, cnkSubI, cnkMulI, cnkDivI, cnkModI } proc needsNoCopy(p: PProc; y: CgNode): bool = return y.kind in nodeKindsNeedNoCopy or @@ -2354,6 +2364,16 @@ proc gen(p: PProc, n: CgNode, r: var TCompRes) = genInfixCall(p, n, r) else: genCall(p, n, r) + of cnkNegI: + let x = gen(p, n[0]) + r.res = "(-$1)" % rdLoc(x) + r.typ = mapType(n.typ) + r.kind = resExpr + of cnkAddI: binaryExpr(p, n[0], n[1], r, "($1 + $2)") + of cnkSubI: binaryExpr(p, n[0], n[1], r, "($1 - $2)") + of cnkMulI: binaryExpr(p, n[0], n[1], r, "($1 * $2)") + of cnkDivI: binaryExpr(p, n[0], n[1], r, "Math.trunc($1 / $2)") + of cnkModI: binaryExpr(p, n[0], n[1], r, "Math.trunc($1 % $2)") of cnkClosureConstr: useMagic(p, "makeClosure") var tmp1, tmp2: TCompRes diff --git a/compiler/mir/mirgen.nim b/compiler/mir/mirgen.nim index 4bc57481fbf..c4c859905f0 100644 --- a/compiler/mir/mirgen.nim +++ b/compiler/mir/mirgen.nim @@ -435,7 +435,8 @@ func detectKind(tree: MirTree, n: NodePosition, sink: bool): ExprKind = OwnedRvalue else: Rvalue - of mnkConv, mnkStdConv, mnkCast, mnkAddr, mnkView, mnkToSlice: + of mnkConv, mnkStdConv, mnkCast, mnkAddr, mnkView, mnkToSlice, UnaryOps, + BinaryOps: Rvalue of mnkObjConstr: if tree[n].typ.skipTypes(abstractInst).kind == tyRef or @@ -1201,6 +1202,32 @@ proc genMagic(c: var TCtx, n: PNode; m: TMagic) = genArgExpression(c, n[3], sink=false) # arithmetic operations: + of mAddI, mSubI, mMulI, mDivI, mModI, mPred, mSucc: + # the `pred` and `succ` magic are lowered to a normal subtraction and + # addition, respectively. Depending on whether overflow checks are + # enabled, either magics or the dedicated MIR operators are used + if optOverflowCheck in c.userOptions: + const Map = [mAddI: mAddI, mSubI, mMulI, mDivI, mModI, + mSucc: mAddI, mPred: mSubI] + c.buildCheckedMagicCall Map[m], n.typ: + arg n[1] + arg n[2] + else: + const Map = [mAddI: mnkAddI, mSubI: mnkSubI, + mMulI: mnkMulI, mDivI: mnkDivI, mModI: mnkModI, + mSucc: mnkAddI, mPred: mnkSubI] + c.buildTree Map[m], n.typ: + genArgExpression(c, n[1], sink=false) + genArgExpression(c, n[2], sink=false) + + of mUnaryMinusI, mUnaryMinusI64: + # negation can cause overflows too + if optOverflowCheck in c.userOptions: + c.buildDefectMagicCall m, n.typ: + arg n[1] + else: + c.genOp(mnkNegI, n.typ, n[1]) + of mInc, mDec: # ``inc a, b`` -> ``a = a + b`` let @@ -1214,24 +1241,32 @@ proc genMagic(c: var TCtx, n: PNode; m: TMagic) = c.emitByVal dest arg n[2] else: - const magic = [mInc: mAddI, mDec: mSubI] + proc op(c: var TCtx, dest: Value, n: PNode, m: TMagic) = + if optOverflowCheck in c.userOptions: + const magic = [mInc: mAddI, mDec: mSubI] + # use a magic call that can potentially raise + c.buildDefectMagicCall magic[m], dest.typ: + c.emitByVal dest + arg n[2] + else: + const kind = [mInc: mnkAddI, mDec: mnkSubI] + # the unchecked arithmetic operators can be used directly + c.buildTree kind[m], dest.typ: + c.use dest + genArgExpression(c, n[2], sink=false) + if optRangeCheck in c.userOptions and typ.skipTypes(abstractInst).kind in {tyRange, tyEnum}: # needs an additional range check in order to ensure that the value # is in range - let val = c.wrapTemp(typ): - c.buildMagicCall magic[m], typ: - c.emitByVal dest - arg n[2] + let val = c.wrapTemp(typ): op(c, dest, n, m) c.buildDefectMagicCall mChckRange, typ: c.emitByVal val c.emitByVal intLiteral(firstOrd(c.graph.config, typ), typ) c.emitByVal intLiteral(lastOrd(c.graph.config, typ), typ) else: # no range check is needed - c.buildMagicCall magic[m], n[1].typ: - c.emitByVal dest - arg n[2] + op(c, dest, n, m) # magics that use incomplete symbols (most of them are generated by # ``liftdestructors``): @@ -1256,7 +1291,7 @@ proc genMagic(c: var TCtx, n: PNode; m: TMagic) = arg n[2] else: genCall(c, n) - of mNot, mLtI, mSubI, mLengthSeq, mLengthStr, mSamePayload: + of mNot, mLtI, mLengthSeq, mLengthStr, mSamePayload: if n[0].typ == nil: # simple translation. None of the arguments need to be passed by lvalue c.buildMagicCall m, n.typ: diff --git a/compiler/mir/mirtrees.nim b/compiler/mir/mirtrees.nim index 81d987422d7..40d539e2251 100644 --- a/compiler/mir/mirtrees.nim +++ b/compiler/mir/mirtrees.nim @@ -154,6 +154,18 @@ type ## Used for both static and dynamic calls mnkCheckedCall ## invoke a magic procedure and pass along the provided arguments + # unary arithmetic operations: + mnkNegI ## signed integer negation (overflow is UB) + # binary arithmetic operations: + mnkAddI ## signed integer addition (overflow is UB) + mnkSubI ## signed integer substraction (overflow is UB) + mnkMulI ## signed integer multiplication (overflow is UB) + mnkDivI ## signed integer division (division by zero is UB) + mnkModI ## compute the remainder of an integer division (division by zero + ## is UB) + # future direction: the arithmetic operations should be generalized to also + # apply to unsigned integers and floats + mnkRaise ## if the operand is an ``mnkNone`` node, reraises the ## currently active exception. Otherwise, set the operand value ## as the active exception (via a move). Control-flow is @@ -339,11 +351,17 @@ const mnkAsgn, mnkSwitch, mnkFastAsgn, mnkVoid, mnkRaise, mnkEmit, mnkAsm} + DefNodes + UnaryOps* = {mnkNegI} + ## All unary operators + BinaryOps* = {mnkAddI, mnkSubI, mnkMulI, mnkDivI, mnkModI} + ## All binary operators + LvalueExprKinds* = {mnkPathPos, mnkPathNamed, mnkPathArray, mnkPathVariant, mnkPathConv, mnkDeref, mnkDerefView, mnkTemp, mnkAlias, mnkLocal, mnkParam, mnkConst, mnkGlobal} RvalueExprKinds* = {mnkLiteral, mnkType, mnkProc, mnkConv, mnkStdConv, - mnkCast, mnkAddr, mnkView, mnkToSlice} + mnkCast, mnkAddr, mnkView, mnkToSlice} + UnaryOps + + BinaryOps ExprKinds* = {mnkCall, mnkCheckedCall, mnkConstr, mnkObjConstr} + LvalueExprKinds + RvalueExprKinds diff --git a/compiler/mir/utils.nim b/compiler/mir/utils.nim index 1c7cd943854..4300acde7f5 100644 --- a/compiler/mir/utils.nim +++ b/compiler/mir/utils.nim @@ -319,6 +319,19 @@ proc exprToStr(nodes: MirTree, i: var int, result: var string, env: EnvPtr) = commaSeparated: argToStr() result.add ") (raises)" + of UnaryOps: + const Map = [mnkNegI: "-"] + let kind = nodes[i].kind + tree Map[kind]: + valueToStr() + of BinaryOps: + let kind = nodes[i].kind + tree "": + valueToStr() # first operand + const Map = [mnkAddI: " + ", mnkSubI: " - ", + mnkMulI: " * ", mnkDivI: " div ", mnkModI: " mod "] + result.add Map[kind] + valueToStr() # second operand else: # TODO: make this branch exhaustive result.add "" diff --git a/compiler/sem/injectdestructors.nim b/compiler/sem/injectdestructors.nim index 5c23d06cdbe..049d340db1d 100644 --- a/compiler/sem/injectdestructors.nim +++ b/compiler/sem/injectdestructors.nim @@ -555,7 +555,8 @@ func isMoveable(tree: MirTree, v: Values, n: NodePosition): bool = false of mnkLiteral, mnkProc, mnkType: true - of mnkConv, mnkStdConv, mnkCast, mnkAddr, mnkView, mnkToSlice: + of mnkConv, mnkStdConv, mnkCast, mnkAddr, mnkView, mnkToSlice, UnaryOps, + BinaryOps: # the result of these operations is not an owned value false of mnkCall, mnkCheckedCall, mnkObjConstr, mnkConstr: diff --git a/compiler/sem/mirexec.nim b/compiler/sem/mirexec.nim index eea4e61aadd..02309eda105 100644 --- a/compiler/sem/mirexec.nim +++ b/compiler/sem/mirexec.nim @@ -273,6 +273,11 @@ func emitForExpr(env: var ClosureEnv, tree: MirTree, at, source: NodePosition, if tree[source].typ.kind == tyVar: opMutate else: opUse emitLvalueOp(env, opc, tree, at, tree.operand(source, 0)) + of UnaryOps: + emitLvalueOp(env, opUse, tree, at, tree.operand(source, 0)) + of BinaryOps: + emitLvalueOp(env, opUse, tree, at, tree.operand(source, 0)) + emitLvalueOp(env, opUse, tree, at, tree.operand(source, 1)) of LvalueExprKinds: # a read or consume is performed on an lvalue let opc = diff --git a/compiler/vm/vmgen.nim b/compiler/vm/vmgen.nim index f33d3ceb920..2d62f92a68e 100644 --- a/compiler/vm/vmgen.nim +++ b/compiler/vm/vmgen.nim @@ -1602,9 +1602,9 @@ proc genNoLoad(c: var TCtx, n: CgNode): tuple[reg: TRegister, isDirect: bool] = proc genMagic(c: var TCtx; n: CgNode; dest: var TDest; m: TMagic) = case m - of mPred, mSubI: + of mSubI: c.genAddSubInt(n, dest, opcSubInt) - of mSucc, mAddI: + of mAddI: c.genAddSubInt(n, dest, opcAddInt) of mOrd, mChr: c.gen(n[1], dest) of mArrToSeq: @@ -2843,6 +2843,18 @@ proc genClosureConstr(c: var TCtx, n: CgNode, dest: TRegister) = c.freeTemp(tmp2) c.freeTemp(envTmp) +proc binaryArith(c: var TCtx, e, x, y: CgNode, dest: var TDest, op: TOpcode) = + ## Emits the instruction sequence for the binary operation `e` with opcode + ## `op`. `x` and `y` are the operand expressions. + prepare(c, dest, e.typ) + let + a = c.genx(x) + b = c.genx(y) + c.gABC(e, op, dest, a, b) + c.genNarrow(x, dest) + c.freeTemp(a) + c.freeTemp(b) + proc gen(c: var TCtx; n: CgNode; dest: var TDest) = when defined(nimCompilerStacktraceHints): frameMsg c.config, n @@ -2863,6 +2875,15 @@ proc gen(c: var TCtx; n: CgNode; dest: var TDest) = else: genCall(c, n, dest) clearDest(c, n, dest) + of cnkNegI: + let a = c.genx(n[0]) + c.gABC(n, opcUnaryMinusInt, dest, a) + c.genNarrow(n[0], dest) + of cnkAddI: binaryArith(c, n, n[0], n[1], dest, opcAddu) + of cnkSubI: binaryArith(c, n, n[0], n[1], dest, opcSubu) + of cnkMulI: binaryArith(c, n, n[0], n[1], dest, opcMulu) + of cnkDivI: binaryArith(c, n, n[0], n[1], dest, opcDivInt) + of cnkModI: binaryArith(c, n, n[0], n[1], dest, opcModInt) of cnkIntLit, cnkUIntLit: prepare(c, dest, n.typ) c.loadInt(n, dest, getInt(n)) diff --git a/doc/mir.rst b/doc/mir.rst index 8a57d5f8dfb..c090f877aca 100644 --- a/doc/mir.rst +++ b/doc/mir.rst @@ -48,6 +48,14 @@ Semantics | | LVALUE + UNARY_OP = NegI VALUE + + BINARY_OP = AddI VALUE, VALUE + | SubI VALUE, VALUE + | MulI VALUE, VALUE + | DivI VALUE, VALUE + | ModI VALUE, VALUE + CALL_ARG = Arg VALUE # pass-by-value argument | Arg # argument that's going to be omitted # later @@ -76,7 +84,11 @@ Semantics | CheckedCall LVALUE CALL_ARG ... | CheckedCall CALL_ARG ... - RVALUE = CALL_EXPR + + RVALUE = UNARY_OP + | BINARY_OP + | CALL_EXPR + | CHECKED_CALL_EXPR | Constr CONSTR_ARG ... # construct a tuple, closure, set, or | ObjConstr ( CONSTR_ARG) ... # construct an `object` or # `ref object` diff --git a/tests/arc/topt_no_cursor.nim b/tests/arc/topt_no_cursor.nim index 33a20a952b2..9499d20a989 100644 --- a/tests/arc/topt_no_cursor.nim +++ b/tests/arc/topt_no_cursor.nim @@ -126,7 +126,7 @@ scope: echo(arg type(array[0..0, string]), arg _9) (raises) finally: =destroy(name splitted) - i = addI(arg i, arg 1) + i = addI(arg i, arg 1) (raises) finally: =destroy(name lan_ip) --expandArc: mergeShadowScope @@ -161,7 +161,7 @@ scope: def _8: Symbol = sym[] =copy(name _6, arg _8) addInterfaceDecl(arg c, consume _6) (raises) - i = addI(arg i, arg 1) + i = addI(arg i, arg 1) (raises) finally: =destroy(name shadowScope) (raises) -- end of expandArc ------------------------ diff --git a/tests/arc/topt_wasmoved_destroy_pairs.nim b/tests/arc/topt_wasmoved_destroy_pairs.nim index 89703513dee..c549515e9b8 100644 --- a/tests/arc/topt_wasmoved_destroy_pairs.nim +++ b/tests/arc/topt_wasmoved_destroy_pairs.nim @@ -50,7 +50,7 @@ scope: def _4: seq[int] =copy(name _4, arg x) add(name a, consume _4) - i = addI(arg i, arg 1) + i = addI(arg i, arg 1) (raises) block L1: if cond: scope: diff --git a/tests/lang_objects/destructor/tdestruction_when_checks_failed.nim b/tests/lang_objects/destructor/tdestruction_when_checks_failed.nim index a552344ccc6..29f5f8166cf 100644 --- a/tests/lang_objects/destructor/tdestruction_when_checks_failed.nim +++ b/tests/lang_objects/destructor/tdestruction_when_checks_failed.nim @@ -76,4 +76,12 @@ block object_conversion_checks: discard A(a) runTest: - test(RootRef()) # provoke a object-conversion-check failure \ No newline at end of file + test(RootRef()) # provoke a object-conversion-check failure + +block signed_integer_overflow_check: + proc test(a: int32) = + var obj = Object() # obj stores a value that needs to be destroyed + discard 1'i32 + a + + runTest: + test(high(int32)) # provoke an overflow-check failure diff --git a/tests/lang_objects/destructor/tv2_cast.nim b/tests/lang_objects/destructor/tv2_cast.nim index e9e2d9ff6dc..dbb16c26ada 100644 --- a/tests/lang_objects/destructor/tv2_cast.nim +++ b/tests/lang_objects/destructor/tv2_cast.nim @@ -25,7 +25,7 @@ scope: def s: string = newString(arg 100) def_cursor _0: string = s def_cursor _1: int = lengthStr(arg _0) - def_cursor _2: int = subI(arg _1, arg 1) + def_cursor _2: int = subI(arg _1, arg 1) (raises) chckBounds(arg s, arg 0, arg _2) (raises) def_cursor _3: openArray[byte] = toOpenArray s, 0, _2 def _4: seq[byte] = encode(arg _3) (raises) diff --git a/tests/overflw/toverflow_abs.nim b/tests/overflw/toverflow_abs.nim new file mode 100644 index 00000000000..1c4d168c58a --- /dev/null +++ b/tests/overflw/toverflow_abs.nim @@ -0,0 +1,24 @@ +discard """ + description: "Ensure that overflow checks for `abs` work" + targets: "c js vm" + matrix: "--overflowChecks:on" + exitcode: 1 + outputsub: "Error: over- or underflow" + knownIssue.c js: "`abs` uses the local overflow check state" +""" + +# instantiate the magic procedure in a context where overflow checks are +# enabled: +discard abs(-1) + +{.push overflowChecks: off.} + +proc test(a: int64) = + # the overflow behaviour of the ``abs`` call is not affected by the current + # overflow-check state, so the below fails with an overflow defect + discard abs(a) + +# negating the lowest possible 32-bit integer would overflow +test(low(int64)) + +{.pop.} \ No newline at end of file