revive #10228 (fix #9880) (#10610)

* Make index out of bounds more useful by including the 'bounds'.
* fixes #9880 index out of bounds (remaining cases); revives #10228
* change err msg to: `index 3 not in 0 .. 1`

11 files changed, 88 insertions, 36 deletions

diff --git a/compiler/semfold.nim b/compiler/semfold.nim
index 237a5127a..f0b03018f 100644
--- a/compiler/semfold.nim
+++ b/compiler/semfold.nim
@@ -493,11 +493,11 @@ proc foldArrayAccess(m: PSym, n: PNode; g: ModuleGraph): PNode =
       result = x.sons[int(idx)]
       if result.kind == nkExprColonExpr: result = result.sons[1]
     else:
-      localError(g.config, n.info, formatErrorIndexBound(idx, sonsLen(x)+1) & $n)
+      localError(g.config, n.info, formatErrorIndexBound(idx, sonsLen(x)-1) & $n)
   of nkBracket:
     idx = idx - firstOrd(g.config, x.typ)
     if idx >= 0 and idx < x.len: result = x.sons[int(idx)]
-    else: localError(g.config, n.info, formatErrorIndexBound(idx, x.len+1) & $n)
+    else: localError(g.config, n.info, formatErrorIndexBound(idx, x.len-1) & $n)
   of nkStrLit..nkTripleStrLit:
     result = newNodeIT(nkCharLit, x.info, n.typ)
     if idx >= 0 and idx < len(x.strVal):
diff --git a/compiler/vm.nim b/compiler/vm.nim
index fd1df3ce9..74f2a367d 100644
--- a/compiler/vm.nim
+++ b/compiler/vm.nim
@@ -11,6 +11,7 @@
 ## An instruction is 1-3 int32s in memory, it is a register based VM.
 
 import ast except getstr
+import system/indexerrors
 
 import
   strutils, astalgo, msgs, vmdef, vmgen, nimsets, types, passes,
@@ -473,7 +474,6 @@ proc setLenSeq(c: PCtx; node: PNode; newLen: int; info: TLineInfo) =
       node.sons[i] = getNullValue(typ.sons[0], info, c.config)
 
 const
-  errIndexOutOfBounds = "index out of bounds"
   errNilAccess = "attempt to access a nil address"
   errOverOrUnderflow = "over- or underflow"
   errConstantDivisionByZero = "division by zero"
@@ -577,7 +577,7 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg =
       # a = b[c]
       decodeBC(rkNode)
       if regs[rc].intVal > high(int):
-        stackTrace(c, tos, pc, errIndexOutOfBounds)
+        stackTrace(c, tos, pc, formatErrorIndexBound(regs[rc].intVal, high(int)))
       let idx = regs[rc].intVal.int
       let src = regs[rb].node
       if src.kind in {nkStrLit..nkTripleStrLit}:
@@ -585,11 +585,11 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg =
           regs[ra].node = newNodeI(nkCharLit, c.debug[pc])
           regs[ra].node.intVal = src.strVal[idx].ord
         else:
-          stackTrace(c, tos, pc, errIndexOutOfBounds)
+          stackTrace(c, tos, pc, formatErrorIndexBound(idx, src.strVal.len-1))
       elif src.kind notin {nkEmpty..nkFloat128Lit} and idx <% src.len:
         regs[ra].node = src.sons[idx]
       else:
-        stackTrace(c, tos, pc, errIndexOutOfBounds)
+        stackTrace(c, tos, pc, formatErrorIndexBound(idx, src.len-1))
     of opcLdStrIdx:
       decodeBC(rkInt)
       let idx = regs[rc].intVal.int
@@ -599,7 +599,7 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg =
       elif idx == s.len and optLaxStrings in c.config.options:
         regs[ra].intVal = 0
       else:
-        stackTrace(c, tos, pc, errIndexOutOfBounds)
+        stackTrace(c, tos, pc, formatErrorIndexBound(idx, s.len-1))
     of opcWrArr:
       # a[b] = c
       decodeBC(rkNode)
@@ -609,11 +609,11 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg =
         if idx <% arr.strVal.len:
           arr.strVal[idx] = chr(regs[rc].intVal)
         else:
-          stackTrace(c, tos, pc, errIndexOutOfBounds)
+          stackTrace(c, tos, pc, formatErrorIndexBound(idx, arr.strVal.len-1))
       elif idx <% arr.len:
         writeField(arr.sons[idx], regs[rc])
       else:
-        stackTrace(c, tos, pc, errIndexOutOfBounds)
+        stackTrace(c, tos, pc, formatErrorIndexBound(idx, arr.len-1))
     of opcLdObj:
       # a = b.c
       decodeBC(rkNode)
@@ -644,7 +644,7 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg =
       if idx <% regs[ra].node.strVal.len:
         regs[ra].node.strVal[idx] = chr(regs[rc].intVal)
       else:
-        stackTrace(c, tos, pc, errIndexOutOfBounds)
+        stackTrace(c, tos, pc, formatErrorIndexBound(idx, regs[ra].node.strVal.len-1))
     of opcAddrReg:
       decodeB(rkRegisterAddr)
       regs[ra].regAddr = addr(regs[rb])
@@ -1361,7 +1361,7 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg =
       if src.kind notin {nkEmpty..nkNilLit} and idx <% src.len:
         regs[ra].node = src.sons[idx]
       else:
-        stackTrace(c, tos, pc, errIndexOutOfBounds)
+        stackTrace(c, tos, pc, formatErrorIndexBound(idx, src.len-1))
     of opcNSetChild:
       decodeBC(rkNode)
       let idx = regs[rb].intVal.int
@@ -1369,7 +1369,7 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg =
       if dest.kind notin {nkEmpty..nkNilLit} and idx <% dest.len:
         dest.sons[idx] = regs[rc].node
       else:
-        stackTrace(c, tos, pc, errIndexOutOfBounds)
+        stackTrace(c, tos, pc, formatErrorIndexBound(idx, dest.len-1))
     of opcNAdd:
       decodeBC(rkNode)
       var u = regs[rb].node
@@ -1774,7 +1774,7 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg =
       if contains(g.cacheSeqs, destKey) and idx <% g.cacheSeqs[destKey].len:
         regs[ra].node = g.cacheSeqs[destKey][idx.int]
       else:
-        stackTrace(c, tos, pc, errIndexOutOfBounds)
+        stackTrace(c, tos, pc, formatErrorIndexBound(idx, g.cacheSeqs[destKey].len-1))
     of opcNctPut:
       let g = c.graph
       let destKey = regs[ra].node.strVal
diff --git a/lib/core/typeinfo.nim b/lib/core/typeinfo.nim
index 32fedd0c1..d6dd16b54 100644
--- a/lib/core/typeinfo.nim
+++ b/lib/core/typeinfo.nim
@@ -27,6 +27,8 @@
 include "system/inclrtl.nim"
 include "system/hti.nim"
 
+import system/indexerrors
+
 {.pop.}
 
 type
@@ -201,14 +203,14 @@ proc `[]`*(x: Any, i: int): Any =
   of tyArray:
     var bs = x.rawType.base.size
     if i >=% x.rawType.size div bs:
-      raise newException(IndexError, "index out of bounds")
+      raise newException(IndexError, formatErrorIndexBound(i, x.rawType.size div bs))
     return newAny(x.value +!! i*bs, x.rawType.base)
   of tySequence:
     var s = cast[ppointer](x.value)[]
     if s == nil: raise newException(ValueError, "sequence is nil")
     var bs = x.rawType.base.size
     if i >=% cast[PGenSeq](s).len:
-      raise newException(IndexError, "index out of bounds")
+      raise newException(IndexError, formatErrorIndexBound(i, cast[PGenSeq](s).len-1))
     return newAny(s +!! (GenericSeqSize+i*bs), x.rawType.base)
   else: assert false
 
@@ -218,7 +220,7 @@ proc `[]=`*(x: Any, i: int, y: Any) =
   of tyArray:
     var bs = x.rawType.base.size
     if i >=% x.rawType.size div bs:
-      raise newException(IndexError, "index out of bounds")
+      raise newException(IndexError, formatErrorIndexBound(i, x.rawType.size div bs))
     assert y.rawType == x.rawType.base
     genericAssign(x.value +!! i*bs, y.value, y.rawType)
   of tySequence:
@@ -226,7 +228,7 @@ proc `[]=`*(x: Any, i: int, y: Any) =
     if s == nil: raise newException(ValueError, "sequence is nil")
     var bs = x.rawType.base.size
     if i >=% cast[PGenSeq](s).len:
-      raise newException(IndexError, "index out of bounds")
+      raise newException(IndexError, formatErrorIndexBound(i, cast[PGenSeq](s).len-1))
     assert y.rawType == x.rawType.base
     genericAssign(s +!! (GenericSeqSize+i*bs), y.value, y.rawType)
   else: assert false
diff --git a/lib/pure/collections/sharedstrings.nim b/lib/pure/collections/sharedstrings.nim
index b283cd4b1..ca52ec63c 100644
--- a/lib/pure/collections/sharedstrings.nim
+++ b/lib/pure/collections/sharedstrings.nim
@@ -12,7 +12,7 @@
 type
   UncheckedCharArray = UncheckedArray[char]
 
-import system/helpers2
+import system/indexerrors
 
 type
   Buffer = ptr object
diff --git a/lib/pure/os.nim b/lib/pure/os.nim
index e88c3c6e8..0b9c8babc 100644
--- a/lib/pure/os.nim
+++ b/lib/pure/os.nim
@@ -47,7 +47,7 @@
 include "system/inclrtl"
 
 import
-  strutils, pathnorm
+  strutils, pathnorm, system/indexerrors
 
 const weirdTarget = defined(nimscript) or defined(js)
 
@@ -2551,7 +2551,7 @@ elif defined(windows):
       ownArgv = parseCmdLine($getCommandLine())
       ownParsedArgv = true
     if i < ownArgv.len and i >= 0: return TaintedString(ownArgv[i])
-    raise newException(IndexError, "invalid index")
+    raise newException(IndexError, formatErrorIndexBound(i, ownArgv.len-1))
 
 elif defined(genode):
   proc paramStr*(i: int): TaintedString =
@@ -2570,7 +2570,7 @@ elif not defined(createNimRtl) and
   proc paramStr*(i: int): TaintedString {.tags: [ReadIOEffect].} =
     # Docstring in nimdoc block.
     if i < cmdCount and i >= 0: return TaintedString($cmdLine[i])
-    raise newException(IndexError, "invalid index")
+    raise newException(IndexError, formatErrorIndexBound(i, cmdCount-1))
 
   proc paramCount*(): int {.tags: [ReadIOEffect].} =
     # Docstring in nimdoc block.
diff --git a/lib/system/indexerrors.nim b/lib/system/indexerrors.nim
index 8bd69ad71..cb1d522c9 100644
--- a/lib/system/indexerrors.nim
+++ b/lib/system/indexerrors.nim
@@ -1,7 +1,7 @@
 # imported by other modules, unlike helpers.nim which is included
 
 template formatErrorIndexBound*[T](i, a, b: T): string =
-  "index out of bounds: (a: " & $a & ") <= (i: " & $i & ") <= (b: " & $b & ") "
+  "index " & $i & " not in " & $a & " .. " & $b
 
 template formatErrorIndexBound*[T](i, n: T): string =
-  "index out of bounds: (i: " & $i & ") <= (n: " & $n & ") "
+  formatErrorIndexBound(i, 0, n)
diff --git a/lib/system/jssys.nim b/lib/system/jssys.nim
index d7718e4f4..27dd9b020 100644
--- a/lib/system/jssys.nim
+++ b/lib/system/jssys.nim
@@ -7,6 +7,8 @@
 #    distribution, for details about the copyright.
 #
 
+import system/indexerrors
+
 proc log*(s: cstring) {.importc: "console.log", varargs, nodecl.}
 
 type
@@ -157,8 +159,8 @@ proc raiseDivByZero {.exportc: "raiseDivByZero", noreturn, compilerProc.} =
 proc raiseRangeError() {.compilerproc, noreturn.} =
   raise newException(RangeError, "value out of range")
 
-proc raiseIndexError() {.compilerproc, noreturn.} =
-  raise newException(IndexError, "index out of bounds")
+proc raiseIndexError(i, a, b: int) {.compilerproc, noreturn.} =
+  raise newException(IndexError, formatErrorIndexBound(int(i), int(a), int(b)))
 
 proc raiseFieldError(f: string) {.compilerproc, noreturn.} =
   raise newException(FieldError, f & " is not accessible")
@@ -626,7 +628,7 @@ proc arrayConstr(len: int, value: JSRef, typ: PNimType): JSRef {.
 
 proc chckIndx(i, a, b: int): int {.compilerproc.} =
   if i >= a and i <= b: return i
-  else: raiseIndexError()
+  else: raiseIndexError(i, a, b)
 
 proc chckRange(i, a, b: int): int {.compilerproc.} =
   if i >= a and i <= b: return i
diff --git a/tests/exception/testindexerroroutput.nims b/tests/exception/testindexerroroutput.nims
new file mode 100644
index 000000000..e282f14b4
--- /dev/null
+++ b/tests/exception/testindexerroroutput.nims
@@ -0,0 +1,23 @@
+mode = ScriptMode.Verbose
+
+case paramStr(3):
+  of "test1":
+    #543
+    block:
+      let s = "abc"
+      discard s[len(s)]
+  of "test2":
+    #537
+    block:
+      var s = "abc"
+      s[len(s)] = 'd'
+  of "test3":
+    #588
+    block:
+      let arr = ['a', 'b', 'c']
+      discard arr[len(arr)]
+  of "test4":
+    #588
+    block:
+      var arr = ['a', 'b', 'c']
+      arr[len(arr)] = 'd'
diff --git a/tests/exception/tindexerrorformatbounds.nim b/tests/exception/tindexerrorformatbounds.nim
new file mode 100644
index 000000000..7563c5ffa
--- /dev/null
+++ b/tests/exception/tindexerrorformatbounds.nim
@@ -0,0 +1,31 @@
+import os, osproc, strutils
+
+const characters = "abcdefghijklmnopqrstuvwxyz"
+var s: string
+
+# # chcks.nim:23
+# # test formatErrorIndexBound returns correct bounds
+block:
+  s = characters
+  try:
+    discard s[0..999]
+  except IndexError:
+    let msg = getCurrentExceptionMsg()
+    let expected = "index $# not in 0 .. $#" % [$len(s), $(len(s)-1)]
+    doAssert msg.contains expected, $(msg, expected)
+
+block:
+  try:
+    discard paramStr(999)
+  except IndexError:
+    let msg = getCurrentExceptionMsg()
+    let expected = "index 999 not in 0 .. 0"
+    doAssert msg.contains expected, $(msg, expected)
+
+block:
+  const nim = getCurrentCompilerExe()
+  for i in 1..4:
+    let (outp, errC) = execCmdEx("$# e tests/exception/testindexerroroutput.nims test$#" % [nim, $i])
+    let expected = "index 3 not in 0 .. 2"
+    doAssert errC != 0
+    doAssert outp.contains expected, $(outp, errC, expected, i)
diff --git a/tests/misc/tinvalidarrayaccess.nim b/tests/misc/tinvalidarrayaccess.nim
index ab44d98e8..f8bce45ef 100644
--- a/tests/misc/tinvalidarrayaccess.nim
+++ b/tests/misc/tinvalidarrayaccess.nim
@@ -1,14 +1,14 @@
 discard """
-  errormsg: "index out of bounds: (a: 0) <= (i: 2) <= (b: 1) "
+  errormsg: "index 2 not in 0 .. 1"
   line: 18
 """
-
 block:
   try:
     let a = @[1,2]
     echo a[3]
   except Exception as e:
-    doAssert e.msg == "index out of bounds: (i:3) <= (n:1) "
+    doAssert e.msg == "index 3 not in 0 .. 1"
+      # note: this is not being tested, because the CT error happens before
 
 block:
   type TTestArr = array[0..1, int16]
diff --git a/tests/misc/tinvalidarrayaccess2.nim b/tests/misc/tinvalidarrayaccess2.nim
index a791dc4e7..0a0703834 100644
--- a/tests/misc/tinvalidarrayaccess2.nim
+++ b/tests/misc/tinvalidarrayaccess2.nim
@@ -1,5 +1,5 @@
 discard """
-  errormsg: "index out of bounds: (a: 0) <= (i: 3) <= (b: 1) "
+  errormsg: "index 3 not in 0 .. 1"
   line: 9
 """
 
@@ -8,9 +8,3 @@ discard """
 let a = [1,2]
 echo a[3]
 
-when false:
-  # TOOD: this case is not yet handled, giving: "index out of bounds"
-  proc fun()=
-    let a = @[1,2]
-    echo a[3]
-  static: fun()