fixes #20865; fixes #20987; Missing bounds check in array slicing (#23814)

fixes #20865
fixes #20987

compiler/ccgcalls.nim

@@ -170,7 +170,7 @@ proc fixupCall(p: BProc, le, ri: PNode, d: var TLoc,
     line(p, cpsStmts, pl)
     if canRaise: raiseExit(p)
 
-proc genBoundsCheck(p: BProc; arr, a, b: TLoc)
+proc genBoundsCheck(p: BProc; arr, a, b: TLoc; arrTyp: PType)
 
 proc reifiedOpenArray(n: PNode): bool {.inline.} =
   var x = n
@@ -191,15 +191,15 @@ proc genOpenArraySlice(p: BProc; q: PNode; formalType, destType: PType; prepareF
   var a = initLocExpr(p, q[1])
   var b = initLocExpr(p, q[2])
   var c = initLocExpr(p, q[3])
-  # but first produce the required index checks:
-  if optBoundsCheck in p.options:
-    genBoundsCheck(p, a, b, c)
-  if prepareForMutation:
-    linefmt(p, cpsStmts, "#nimPrepareStrMutationV2($1);$n", [byRefLoc(p, a)])
   # bug #23321: In the function mapType, ptrs (tyPtr, tyVar, tyLent, tyRef)
   # are mapped into ctPtrToArray, the dereference of which is skipped
-  # in the `genref`. We need to skip these ptrs here
+  # in the `genDeref`. We need to skip these ptrs here
   let ty = skipTypes(a.t, abstractVar+{tyPtr, tyRef})
+  # but first produce the required index checks:
+  if optBoundsCheck in p.options:
+    genBoundsCheck(p, a, b, c, ty)
+  if prepareForMutation:
+    linefmt(p, cpsStmts, "#nimPrepareStrMutationV2($1);$n", [byRefLoc(p, a)])
   let dest = getTypeDesc(p.module, destType)
   let lengthExpr = "($1)-($2)+1" % [rdLoc(c), rdLoc(b)]
   case ty.kind

compiler/ccgexprs.nim

@@ -1021,8 +1021,8 @@ proc genCStringElem(p: BProc, n, x, y: PNode, 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) =
-  let ty = skipTypes(arr.t, abstractVarRange)
+proc genBoundsCheck(p: BProc; arr, a, b: TLoc; arrTyp: PType) =
+  let ty = arrTyp
   case ty.kind
   of tyOpenArray, tyVarargs:
     if reifiedOpenArray(arr.lode):
@@ -1857,7 +1857,7 @@ proc genArrayLen(p: BProc, e: PNode, d: var TLoc, op: TMagic) =
       var b = initLocExpr(p, a[2])
       var c = initLocExpr(p, a[3])
       if optBoundsCheck in p.options:
-        genBoundsCheck(p, m, b, c)
+        genBoundsCheck(p, m, b, c, skipTypes(m.t, abstractVarRange))
       if op == mHigh:
         putIntoDest(p, d, e, ropecg(p.module, "(($2)-($1))", [rdLoc(b), rdLoc(c)]))
       else:

tests/arc/topenarray.nim

@@ -68,3 +68,19 @@ block:
   doAssert foo(noBugConst) == expected
   let noBugSeq = @["0", "c", "a"]
   doAssert foo(noBugSeq) == expected
+
+block: # bug #20865
+  var p: pointer
+  var x: array[0, int]
+  # echo toOpenArray(x, 0, 1)[0] # Raises IndexDefect
+  doAssertRaises(IndexDefect):
+    echo toOpenArray(cast[ptr array[0, int]](p)[], 0, 1)[0] # Does not raise IndexDefect
+
+block: # bug #20987
+  var v: array[1, byte]
+
+  var p = cast[ptr array[0, byte]](addr v)
+
+  doAssertRaises(IndexDefect):
+    echo toOpenArray(p[], 1, 2)
+

tests/overload/t8829.nim

@@ -2,7 +2,7 @@ block:
   let txt = "Hello World"
 
   template `[]`[T](p: ptr T, span: Slice[int]): untyped =
-    toOpenArray(cast[ptr array[0, T]](p)[], span.a, span.b)
+    toOpenArray(cast[ptr UncheckedArray[T]](p), span.a, span.b)
 
   doAssert $cast[ptr uint8](txt[0].addr)[0 ..< txt.len] == 
                 "[72, 101, 108, 108, 111, 32, 87, 111, 114, 108, 100]"
@@ -12,7 +12,7 @@ block:
   let txt = "Hello World"
 
   template `[]`[T](p: ptr T, span: Slice[int]): untyped =
-    toOpenArray(cast[ptr array[0, T]](p)[], span.a, span.b)
+    toOpenArray(cast[ptr UncheckedArray[T]](p), span.a, span.b)
 
   doAssert $cast[ptr uint8](txt[0].addr)[0 ..< txt.len] == 
                 "[72, 101, 108, 108, 111, 32, 87, 111, 114, 108, 100]"