lower mChckBounds magic with MIR pass

Summary
=======

Lower the bound checks for `toOpenArray` with a MIR pass when using
the C target.

Details
=======

* implement a bound check lowering pass
  * the lowering is integrated into the existing `lowerChecks`
  * bound checks are turned into calls to the `chckBounds` runtime
    procedure
* remove the `mChckBounds` translation and associated logic from `cgen`

Author: zerbina

compiler/backend/ccgexprs.nim

@@ -515,32 +515,6 @@ proc genCStringElem(p: BProc, n, x, y: CgNode, d: var TLoc) =
   putIntoDest(p, d, n,
               ropecg(p.module, "$1[$2]", [rdLoc(a), rdCharLoc(b)]), a.storage)
 
-proc genBoundsCheck(p: BProc; arr, a, b: TLoc, exit: CgNode) =
-  # types that map to C pointers need to be skipped here too, since no
-  # dereference is generated for ``ptr array`` and the like
-  let ty = skipTypes(arr.t, abstractVarRange + {tyPtr, tyRef, tyLent})
-  case ty.kind
-  of tyOpenArray, tyVarargs:
-    if reifiedOpenArray(p, arr.lode):
-      linefmt(p, cpsStmts,
-        "if ($2-$1 != -1 && " &
-        "((NU)($1) >= (NU)($3.Field1) || (NU)($2) >= (NU)($3.Field1))){ #raiseIndexError(); $4}$n",
-        [rdLoc(a), rdLoc(b), rdLoc(arr), raiseInstr(p, exit)])
-    else:
-      linefmt(p, cpsStmts,
-        "if ($2-$1 != -1 && " &
-        "((NU)($1) >= (NU)($3Len_0) || (NU)($2) >= (NU)($3Len_0))){ #raiseIndexError(); $4}$n",
-        [rdLoc(a), rdLoc(b), rdLoc(arr), raiseInstr(p, exit)])
-  of tySequence, tyString:
-    linefmt(p, cpsStmts,
-      "if ($2-$1 != -1 && " &
-      "((NU)($1) >= (NU)$3 || (NU)($2) >= (NU)$3)){ #raiseIndexError(); $4}$n",
-      [rdLoc(a), rdLoc(b), lenExpr(p, arr), raiseInstr(p, exit)])
-  of tyUncheckedArray, tyCstring:
-    discard "no checks are used"
-  else:
-    unreachable(ty.kind)
-
 proc genOpenArrayElem(p: BProc, n, x, y: CgNode, d: var TLoc) =
   var a, b: TLoc
   initLocExpr(p, x, a)
@@ -1416,12 +1390,6 @@ proc genMagicExpr(p: BProc, e: CgNode, d: var TLoc, op: TMagic) =
       typ.add "*"
 
     linefmt(p, cpsStmts, "$1 = ($2)($3);$n", [a.r, typ, rdLoc(b)])
-  of mChckBounds:
-    var arr, a, b: TLoc
-    initLocExpr(p, e[1], arr)
-    initLocExpr(p, e[2], a)
-    initLocExpr(p, e[3], b)
-    genBoundsCheck(p, arr, a, b, e.exit)
   of mSamePayload:
     var a, b: TLoc
     initLocExpr(p, e[1], a)

compiler/backend/ccgstmts.nim

@@ -95,14 +95,6 @@ proc genIf(p: BProc, n: CgNode) =
   lineF(p, cpsStmts, "if ($1)$n", [rdLoc(a)])
   startBlock(p)
 
-proc exit(n: CgNode): CgNode =
-  # XXX: exists as a convenience for overflow check, index check, etc.
-  #      code gen. Should be removed once those are fully lowered prior
-  #      to code generation
-  case n.kind
-  of cnkCheckedCall: n[^1]
-  else:              nil
-
 proc raiseInstr(p: BProc, n: CgNode): Rope =
   if n != nil:
     case n.kind

compiler/mir/rtchecks.nim

@@ -564,6 +564,35 @@ proc emitCheckedFloatOp(tree; call; graph; env; bu): Value =
       bu.emitCall(tree, call, env.addCompilerProc(graph, "raiseFloatOverflow")):
         bu.emitByVal result
 
+proc emitBoundCheck(tree; call; graph; env; bu) =
+  ## Emits the lowered version of a bound check.
+  case env.types.headerFor(tree[tree.argument(call, 0)].typ, Canonical).kind
+  of tkSeq, tkString, tkOpenArray:
+    let len = bu.wrapTemp env.types.sizeType:
+      # note: lengthOpenArray works for all containers
+      bu.buildMagicCall mLengthOpenArray, env.types.sizeType:
+        bu.subTree mnkArg:
+          bu.emitFrom(tree, NodePosition tree.argument(call, 0))
+
+    bu.emitCall(tree, call, env.addCompilerProc(graph, "chckBounds")):
+      bu.emitByVal len
+      bu.subTree mnkArg:
+        bu.emitFrom(tree, NodePosition tree.argument(call, 1))
+      bu.subTree mnkArg:
+        bu.emitFrom(tree, NodePosition tree.argument(call, 2))
+  of tkCstring:
+    # TODO: don't emit a ``mChckBounds`` call for cstrings in the first place
+    if tree[call].kind == mnkCheckedCall and
+       tree[tree.last(call)].kind != mnkUnwind:
+      # emit ``if false: raise``, so that the target label isn't unused
+      bu.buildIf (bu.use env.makeLiteral(mnkUIntLit, Zero, BoolType)):
+        bu.subTree mnkRaise:
+          bu.emitFrom(tree, tree.last(call))
+    else:
+      discard "no local handler -> nothing to do"
+  else:
+    unreachable()
+
 proc lowerChecks*(body; graph; env; changes: var Changeset) =
   ## Lowers all magic calls implementing the run-time checks.
   template tree: MirTree = body.code
@@ -599,6 +628,10 @@ proc lowerChecks*(body; graph; env; changes: var Changeset) =
         let call = tree.parent(i)
         changes.replaceMulti(tree, tree.parent(call), bu):
           emitObjectCheck(tree, call, graph, env, bu)
+      of mChckBounds:
+        let call = tree.parent(i)
+        changes.replaceMulti(tree, tree.parent(call), bu):
+          emitBoundCheck(tree, call, graph, env, bu)
 
       of mAddI, mSubI, mMulI, mModI, mDivI:
         let call = tree.parent(i)

lib/system/chcks.nim

@@ -122,3 +122,9 @@ proc chckNil(p: pointer) =
 when defined(nimV2):
   proc raiseObjectCaseTransition() {.compilerproc.} =
     sysFatal(FieldDefect, "assignment to discriminant changes object branch")
+
+proc chckBounds(len: int, lo, hi: int) {.compilerproc, inline.} =
+  ## Bounds check for integer-sized containers.
+  let ulen = cast[uint](len)
+  if hi-lo != -1 and (cast[uint](lo) >= ulen or cast[uint](hi) >= ulen):
+    raiseIndexError()