orc: fix overflow checking regression (#25089)

Raising exceptions halfway through a memory allocation is undefined
behavior since exceptions themselves require multiple allocations and
the allocator functions are not reentrant.

It is of course also expensive performance-wise to introduce lots of
exception-raising code everywhere since it breaks many optimisations and
bloats the code.

Finally, performing pointer arithmetic with signed integers is incorrect
for example on on a 32-bit systems that allows up to 3gb of address
space for applications (large address extensions) and unnecessary
elsewhere - broadly, stuff inside the memory allocator is generated by
the compiler or controlled by the standard library meaning that
applications should not be forced to pay this price.

If we wanted to check for overflow, the right way would be in the
initial allocation location where both the size and count of objects is
known.

The code is updated to use the same arithmetic operator style as for
refc with unchecked operations rather than disabling overflow checking
wholesale in the allocator module - there are reasons for both, but
going with the existing flow seems like an easier place to start.

Author: arnetheduck

lib/core/typeinfo.nim

@@ -134,19 +134,17 @@ else:
   proc zeroNewElements(len: int; p: pointer; addlen, elemSize, elemAlign: int) {.
     importCompilerProc.}
 
-template `+!!`(a, b): untyped = cast[pointer](cast[int](a) + b)
+include system/ptrarith
 
 proc getDiscriminant(aa: pointer, n: ptr TNimNode): int =
   assert(n.kind == nkCase)
-  var d: int
-  let a = cast[int](aa)
+  let a = aa +! n.offset
   case n.typ.size
-  of 1: d = int(cast[ptr uint8](a +% n.offset)[])
-  of 2: d = int(cast[ptr uint16](a +% n.offset)[])
-  of 4: d = int(cast[ptr uint32](a +% n.offset)[])
-  of 8: d = int(cast[ptr uint64](a +% n.offset)[])
+  of 1: int(cast[ptr uint8](a)[])
+  of 2: int(cast[ptr uint16](a)[])
+  of 4: int(cast[ptr uint32](a)[])
+  of 8: cast[int](cast[ptr uint64](a)[])
   else: raiseAssert "unreachable"
-  return d
 
 proc selectBranch(aa: pointer, n: ptr TNimNode): ptr TNimNode =
   let discr = getDiscriminant(aa, n)
@@ -240,17 +238,14 @@ proc skipRange(x: PNimType): PNimType {.inline.} =
   result = x
   if result.kind == tyRange: result = result.base
 
-proc align(address, alignment: int): int =
-  result = (address + (alignment - 1)) and not (alignment - 1)
-
 proc `[]`*(x: Any, i: int): Any =
   ## Accessor for an any `x` that represents an array or a sequence.
   case x.rawType.kind
   of tyArray:
     let bs = x.rawType.base.size
     if i >=% x.rawType.size div bs:
       raise newException(IndexDefect, formatErrorIndexBound(i, x.rawType.size div bs))
-    return newAny(x.value +!! i*bs, x.rawType.base)
+    return newAny(x.value +! i*bs, x.rawType.base)
   of tySequence:
     when defined(gcDestructors):
       var s = cast[ptr NimSeqV2Reimpl](x.value)
@@ -259,14 +254,14 @@ proc `[]`*(x: Any, i: int): Any =
       let bs = x.rawType.base.size
       let ba = x.rawType.base.align
       let headerSize = align(sizeof(int), ba)
-      return newAny(s.p +!! (headerSize+i*bs), x.rawType.base)
+      return newAny(s.p +! (headerSize+i*bs), x.rawType.base)
     else:
       var s = cast[ppointer](x.value)[]
       if s == nil: raise newException(ValueError, "sequence is nil")
       let bs = x.rawType.base.size
       if i >=% cast[PGenSeq](s).len:
         raise newException(IndexDefect, formatErrorIndexBound(i, cast[PGenSeq](s).len-1))
-      return newAny(s +!! (align(GenericSeqSize, x.rawType.base.align)+i*bs), x.rawType.base)
+      return newAny(s +! (align(GenericSeqSize, x.rawType.base.align)+i*bs), x.rawType.base)
   else: raiseAssert "unreachable"
 
 proc `[]=`*(x: Any, i: int, y: Any) =
@@ -277,7 +272,7 @@ proc `[]=`*(x: Any, i: int, y: Any) =
     if i >=% x.rawType.size div bs:
       raise newException(IndexDefect, formatErrorIndexBound(i, x.rawType.size div bs))
     assert y.rawType == x.rawType.base
-    genericAssign(x.value +!! i*bs, y.value, y.rawType)
+    genericAssign(x.value +! i*bs, y.value, y.rawType)
   of tySequence:
     when defined(gcDestructors):
       var s = cast[ptr NimSeqV2Reimpl](x.value)
@@ -287,15 +282,15 @@ proc `[]=`*(x: Any, i: int, y: Any) =
       let ba = x.rawType.base.align
       let headerSize = align(sizeof(int), ba)
       assert y.rawType == x.rawType.base
-      genericAssign(s.p +!! (headerSize+i*bs), y.value, y.rawType)
+      genericAssign(s.p +! (headerSize+i*bs), y.value, y.rawType)
     else:
       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(IndexDefect, formatErrorIndexBound(i, cast[PGenSeq](s).len-1))
       assert y.rawType == x.rawType.base
-      genericAssign(s +!! (align(GenericSeqSize, x.rawType.base.align)+i*bs), y.value, y.rawType)
+      genericAssign(s +! (align(GenericSeqSize, x.rawType.base.align)+i*bs), y.value, y.rawType)
   else: raiseAssert "unreachable"
 
 proc len*(x: Any): int =
@@ -352,13 +347,13 @@ proc fieldsAux(p: pointer, n: ptr TNimNode,
   case n.kind
   of nkNone: assert(false)
   of nkSlot:
-    ret.add((n.name, newAny(p +!! n.offset, n.typ)))
+    ret.add((n.name, newAny(p +! n.offset, n.typ)))
     assert ret[ret.len()-1][0] != nil
   of nkList:
     for i in 0..n.len-1: fieldsAux(p, n.sons[i], ret)
   of nkCase:
     var m = selectBranch(p, n)
-    ret.add((n.name, newAny(p +!! n.offset, n.typ)))
+    ret.add((n.name, newAny(p +! n.offset, n.typ)))
     if m != nil: fieldsAux(p, m, ret)
 
 iterator fields*(x: Any): tuple[name: string, any: Any] =
@@ -409,7 +404,7 @@ proc `[]=`*(x: Any, fieldName: string, value: Any) =
   let n = getFieldNode(x.value, t.node, fieldName)
   if n != nil:
     assert n.typ == value.rawType
-    genericAssign(x.value +!! n.offset, value.value, value.rawType)
+    genericAssign(x.value +! n.offset, value.value, value.rawType)
   else:
     raise newException(ValueError, "invalid field name: " & fieldName)
 
@@ -422,7 +417,7 @@ proc `[]`*(x: Any, fieldName: string): Any =
   assert x.rawType.kind in {tyTuple, tyObject}
   let n = getFieldNode(x.value, t.node, fieldName)
   if n != nil:
-    result = Any(value: x.value +!! n.offset)
+    result = Any(value: x.value +! n.offset)
     result.rawType = n.typ
   elif x.rawType.kind == tyObject and x.rawType.base != nil:
     return `[]`(newAny(x.value, x.rawType.base), fieldName)

lib/std/private/dragonbox.nim

@@ -1043,15 +1043,6 @@ proc toDecimal64*(ieeeSignificand: uint64; ieeeExponent: uint64): FloatingDecima
 #  ToChars
 # ==================================================================================================
 
-when false:
-  template `+!`(x: cstring; offset: int): cstring = cast[cstring](cast[uint](x) + uint(offset))
-
-  template dec(x: cstring; offset=1) = x = cast[cstring](cast[uint](x) - uint(offset))
-  template inc(x: cstring; offset=1) = x = cast[cstring](cast[uint](x) + uint(offset))
-
-  proc memset(x: cstring; ch: char; L: int) {.importc, nodecl.}
-  proc memmove(a, b: cstring; L: int) {.importc, nodecl.}
-
 proc utoa8DigitsSkipTrailingZeros*(buf: var openArray[char]; pos: int; digits: uint32): int {.inline.} =
   dragonbox_Assert(digits >= 1)
   dragonbox_Assert(digits <= 99999999'u32)

lib/system.nim

@@ -1132,12 +1132,7 @@ import std/private/since
 import system/ctypes
 export ctypes
 
-proc align(address, alignment: int): int =
-  if alignment == 0: # Actually, this is illegal. This branch exists to actively
-                     # hide problems.
-    result = address
-  else:
-    result = (address + (alignment - 1)) and not (alignment - 1)
+include system/ptrarith
 
 include system/rawquits
 when defined(genode):

lib/system/arc.nim

@@ -83,9 +83,9 @@ when defined(gcAtomicArc) and hasThreadSupport:
     atomicLoadN(x.rc.addr, ATOMIC_ACQUIRE) shr rcShift
 else:
   template decrement(cell: Cell): untyped =
-    dec(cell.rc, rcIncrement)
+    cell.rc = cell.rc -% rcIncrement
   template increment(cell: Cell): untyped =
-    inc(cell.rc, rcIncrement)
+    cell.rc = cell.rc +% rcIncrement
   template count(x: Cell): untyped =
     x.rc shr rcShift
 
@@ -94,7 +94,7 @@ when not defined(nimHasQuirky):
 
 proc nimNewObj(size, alignment: int): pointer {.compilerRtl.} =
   let hdrSize = align(sizeof(RefHeader), alignment)
-  let s = size + hdrSize
+  let s = size +% hdrSize
   when defined(nimscript):
     discard
   else:

lib/system/cellseqs_v1.nim

@@ -22,9 +22,9 @@ proc contains(s: CellSeq, c: PCell): bool {.inline.} =
   return false
 
 proc resize(s: var CellSeq) =
-  s.cap = s.cap div 2 + s.cap
-  let d = cast[PCellArray](alloc(s.cap * sizeof(PCell)))
-  copyMem(d, s.d, s.len * sizeof(PCell))
+  s.cap = s.cap div 2 +% s.cap
+  let d = cast[PCellArray](alloc(cast[Natural](s.cap *% sizeof(PCell))))
+  copyMem(d, s.d, s.len *% sizeof(PCell))
   dealloc(s.d)
   s.d = d
 
@@ -37,7 +37,7 @@ proc add(s: var CellSeq, c: PCell) {.inline.} =
 proc init(s: var CellSeq, cap: int = 1024) =
   s.len = 0
   s.cap = cap
-  s.d = cast[PCellArray](alloc0(cap * sizeof(PCell)))
+  s.d = cast[PCellArray](alloc0(cast[Natural](cap *% sizeof(PCell))))
 
 proc deinit(s: var CellSeq) =
   dealloc(s.d)

lib/system/cellseqs_v2.nim

@@ -17,26 +17,26 @@ type
     d: CellArray[T]
 
 proc resize[T](s: var CellSeq[T]) =
-  s.cap = s.cap div 2 + s.cap
-  var newSize = s.cap * sizeof(CellTuple[T])
+  s.cap = s.cap div 2 +% s.cap
+  let newSize = s.cap *% sizeof(CellTuple[T])
   when compileOption("threads"):
-    s.d = cast[CellArray[T]](reallocShared(s.d, newSize))
+    s.d = cast[CellArray[T]](reallocShared(s.d, cast[Natural](newSize)))
   else:
-    s.d = cast[CellArray[T]](realloc(s.d, newSize))
+    s.d = cast[CellArray[T]](realloc(s.d, cast[Natural](newSize)))
 
 proc add[T](s: var CellSeq[T], c: T, t: PNimTypeV2) {.inline.} =
   if s.len >= s.cap:
     s.resize()
   s.d[s.len] = (c, t)
-  inc(s.len)
+  s.len = s.len +% 1
 
 proc init[T](s: var CellSeq[T], cap: int = 1024) =
   s.len = 0
   s.cap = cap
   when compileOption("threads"):
-    s.d = cast[CellArray[T]](allocShared(uint(s.cap * sizeof(CellTuple[T]))))
+    s.d = cast[CellArray[T]](allocShared(cast[Natural](s.cap *% sizeof(CellTuple[T]))))
   else:
-    s.d = cast[CellArray[T]](alloc(s.cap * sizeof(CellTuple[T])))
+    s.d = cast[CellArray[T]](alloc(cast[Natural](s.cap *% sizeof(CellTuple[T]))))
 
 proc deinit[T](s: var CellSeq[T]) =
   if s.d != nil:
@@ -49,5 +49,6 @@ proc deinit[T](s: var CellSeq[T]) =
   s.cap = 0
 
 proc pop[T](s: var CellSeq[T]): (T, PNimTypeV2) =
-  result = s.d[s.len-1]
-  dec s.len
+  let last = s.len -% 1
+  s.len = last
+  s.d[last]

lib/system/channels_builtin.nim

@@ -180,7 +180,6 @@ proc deinitRawChannel(p: pointer) =
   deinitSysCond(c.cond)
 
 when not usesDestructors:
-
   proc storeAux(dest, src: pointer, mt: PNimType, t: PRawChannel,
                 mode: LoadStoreMode) {.benign.}
 
@@ -203,9 +202,6 @@ when not usesDestructors:
 
   proc storeAux(dest, src: pointer, mt: PNimType, t: PRawChannel,
                 mode: LoadStoreMode) =
-    template `+!`(p: pointer; x: int): pointer =
-      cast[pointer](cast[int](p) +% x)
-
     var
       d = cast[int](dest)
       s = cast[int](src)

lib/system/gc.nim

@@ -173,11 +173,11 @@ proc addZCT(s: var CellSeq, c: PCell) {.noinline.} =
 
 proc cellToUsr(cell: PCell): pointer {.inline.} =
   # convert object (=pointer to refcount) to pointer to userdata
-  result = cast[pointer](cast[int](cell)+%ByteAddress(sizeof(Cell)))
+  cell +! sizeof(Cell)
 
 proc usrToCell(usr: pointer): PCell {.inline.} =
   # convert pointer to userdata to object (=pointer to refcount)
-  result = cast[PCell](cast[int](usr)-%ByteAddress(sizeof(Cell)))
+  cast[PCell](usr -! sizeof(Cell))
 
 proc extGetCellType(c: pointer): PNimType {.compilerproc.} =
   # used for code generation concerning debugging

lib/system/gc_ms.nim

@@ -94,11 +94,11 @@ template gcAssert(cond: bool, msg: string) =
 
 proc cellToUsr(cell: PCell): pointer {.inline.} =
   # convert object (=pointer to refcount) to pointer to userdata
-  result = cast[pointer](cast[int](cell)+%ByteAddress(sizeof(Cell)))
+  cell +! sizeof(Cell)
 
 proc usrToCell(usr: pointer): PCell {.inline.} =
   # convert pointer to userdata to object (=pointer to refcount)
-  result = cast[PCell](cast[int](usr)-%ByteAddress(sizeof(Cell)))
+  cast[PCell](usr -! sizeof(Cell))
 
 proc extGetCellType(c: pointer): PNimType {.compilerproc.} =
   # used for code generation concerning debugging

lib/system/gc_regions.nim

@@ -101,16 +101,10 @@ template withRegion*(r: var MemRegion; body: untyped) =
     tlRegion = oldRegion
 
 template inc(p: pointer, s: int) =
-  p = cast[pointer](cast[int](p) +% s)
+  p = p +! s
 
 template dec(p: pointer, s: int) =
-  p = cast[pointer](cast[int](p) -% s)
-
-template `+!`(p: pointer, s: int): pointer =
-  cast[pointer](cast[int](p) +% s)
-
-template `-!`(p: pointer, s: int): pointer =
-  cast[pointer](cast[int](p) -% s)
+  p = p -! s
 
 const nimMinHeapPages {.intdefine.} = 4