From e0963b87615498c33c0e36b24c61810aa9d38877 Mon Sep 17 00:00:00 2001
From: zerbina <100542850+zerbina@users.noreply.github.com>
Date: Thu, 8 Aug 2024 20:56:02 +0200
Subject: [PATCH] vmgen: respect overflow checks for 64-bit negation (#1408)

## Summary

Negation of 64-bit integers didn't respect the overflow check setting,
resulting in overflow defects even when the checks are disabled. This
is now fixed.

Fixes https://github.com/nim-works/nimskull/issues/1406.

## Details

The VM presently doesn't support unchecked negation. For ease of
implementation, unchecked negation is emulated in bytecode as
`not(x) + 1`.
---
 compiler/vm/vmgen.nim                    | 17 +++++++++++++++--
 tests/overflw/tunchecked_negation_32.nim | 10 ++++++++++
 tests/overflw/tunchecked_negation_64.nim | 10 ++++++++++
 3 files changed, 35 insertions(+), 2 deletions(-)
 create mode 100644 tests/overflw/tunchecked_negation_32.nim
 create mode 100644 tests/overflw/tunchecked_negation_64.nim

diff --git a/compiler/vm/vmgen.nim b/compiler/vm/vmgen.nim
index 01fbe39faef..c530a4a7488 100644
--- a/compiler/vm/vmgen.nim
+++ b/compiler/vm/vmgen.nim
@@ -3018,8 +3018,21 @@ proc gen(c: var TCtx; n: CgNode; dest: var TDest) =
       clearDest(c, n, dest)
   of cnkNeg:
     prepare(c, dest, n.typ)
-    let a = c.genx(n[0])
-    c.gABC(n, pick(n, opcUnaryMinusInt, opcUnaryMinusFloat), dest, a)
+    let
+      a = c.genx(n[0])
+      op = pick(n, opcUnaryMinusInt, opcUnaryMinusFloat)
+    if op == opcUnaryMinusInt:
+      # the VM has no built-in unchecked integer negation, so it's emulated in
+      # an overflow-safe manner
+      # XXX: this is sub-optimal. In the future, all VM integer operations
+      #      should be unchecked
+      c.gABC(n, opcBitnotInt, dest, a)    # invert
+      let tmp = c.getTemp(slotTempInt)
+      c.gABx(n, opcLdImmInt, tmp, 1)
+      c.gABC(n, opcAddu, dest, dest, tmp) # + 1 (two's complement)
+      c.freeTemp(tmp)
+    else:
+      c.gABC(n, op, dest, a)
     c.freeTemp(a)
   of cnkAdd: binaryArith(c, n, n[0], n[1], dest, opcAddu, opcAddFloat)
   of cnkSub: binaryArith(c, n, n[0], n[1], dest, opcSubu, opcSubFloat)
diff --git a/tests/overflw/tunchecked_negation_32.nim b/tests/overflw/tunchecked_negation_32.nim
new file mode 100644
index 00000000000..e03c0260300
--- /dev/null
+++ b/tests/overflw/tunchecked_negation_32.nim
@@ -0,0 +1,10 @@
+discard """
+  description: '''
+    Ensure that disabling overflow checks works for 32-bit integer negations
+  '''
+  targets: "c js vm"
+  matrix: "--overflowChecks:off"
+"""
+
+var x = low(int32)
+discard -x
\ No newline at end of file
diff --git a/tests/overflw/tunchecked_negation_64.nim b/tests/overflw/tunchecked_negation_64.nim
new file mode 100644
index 00000000000..23ed5382a6a
--- /dev/null
+++ b/tests/overflw/tunchecked_negation_64.nim
@@ -0,0 +1,10 @@
+discard """
+  description: '''
+    Ensure that disabling overflow checks works for 64-bit integer negations
+  '''
+  targets: "c js vm"
+  matrix: "--overflowChecks:off"
+"""
+
+var x = low(int64)
+discard -x