Fix ref bug in vmgen (#8424)

3 files changed, 50 insertions, 54 deletions

diff --git a/compiler/vm.nim b/compiler/vm.nim
index 373a64e39..d7e1b5da3 100644
--- a/compiler/vm.nim
+++ b/compiler/vm.nim
@@ -616,19 +616,12 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg =
       let rc = instr.regC
       case regs[ra].kind
       of rkNodeAddr:
-        # XXX: Workaround for vmgen bug:
         let n = regs[rc].regToNode
-        if (nfIsRef in regs[ra].nodeAddr[].flags or
-            regs[ra].nodeAddr[].kind == nkNilLit) and nfIsRef notin n.flags:
-          if regs[ra].nodeAddr[].kind == nkNilLit:
-            stackTrace(c, tos, pc, errNilAccess)
-          regs[ra].nodeAddr[][] = n[]
-          regs[ra].nodeAddr[].flags.incl nfIsRef
         # `var object` parameters are sent as rkNodeAddr. When they are mutated
         # vmgen generates opcWrDeref, which means that we must dereference
         # twice.
         # TODO: This should likely be handled differently in vmgen.
-        elif (nfIsRef notin regs[ra].nodeAddr[].flags and
+        if (nfIsRef notin regs[ra].nodeAddr[].flags and
             nfIsRef notin n.flags):
           regs[ra].nodeAddr[][] = n[]
         else:
diff --git a/compiler/vmgen.nim b/compiler/vmgen.nim
index d2243376c..31e04a6c0 100644
--- a/compiler/vmgen.nim
+++ b/compiler/vmgen.nim
@@ -37,7 +37,9 @@ when hasFFI:
   import evalffi
 
 type
-  TGenFlag = enum gfAddrOf, gfFieldAccess
+  TGenFlag = enum
+    gfNode # Affects how variables are loaded - always loads as rkNode
+    gfNodeAddr # Affects how variables are loaded - always loads as rkNodeAddr
   TGenFlags = set[TGenFlag]
 
 proc debugInfo(c: PCtx; info: TLineInfo): string =
@@ -563,7 +565,7 @@ proc genIndex(c: PCtx; n: PNode; arr: PType): TRegister =
 proc genAsgnPatch(c: PCtx; le: PNode, value: TRegister) =
   case le.kind
   of nkBracketExpr:
-    let dest = c.genx(le.sons[0], {gfAddrOf, gfFieldAccess})
+    let dest = c.genx(le.sons[0], {gfNode})
     let idx = c.genIndex(le.sons[1], le.sons[0].typ)
     c.gABC(le, opcWrArr, dest, idx, value)
     c.freeTemp(dest)
@@ -571,17 +573,17 @@ proc genAsgnPatch(c: PCtx; le: PNode, value: TRegister) =
   of nkDotExpr, nkCheckedFieldExpr:
     # XXX field checks here
     let left = if le.kind == nkDotExpr: le else: le.sons[0]
-    let dest = c.genx(left.sons[0], {gfAddrOf, gfFieldAccess})
+    let dest = c.genx(left.sons[0], {gfNode})
     let idx = genField(c, left.sons[1])
     c.gABC(left, opcWrObj, dest, idx, value)
     c.freeTemp(dest)
   of nkDerefExpr, nkHiddenDeref:
-    let dest = c.genx(le.sons[0], {gfAddrOf})
+    let dest = c.genx(le.sons[0], {gfNode})
     c.gABC(le, opcWrDeref, dest, 0, value)
     c.freeTemp(dest)
   of nkSym:
     if le.sym.isGlobal:
-      let dest = c.genx(le, {gfAddrOf})
+      let dest = c.genx(le, {gfNodeAddr})
       c.gABC(le, opcWrDeref, dest, 0, value)
       c.freeTemp(dest)
   else:
@@ -1252,41 +1254,21 @@ proc canElimAddr(n: PNode): PNode =
       # addr ( deref ( x )) --> x
       result = n.sons[0].sons[0]
 
-proc genAddrDeref(c: PCtx; n: PNode; dest: var TDest; opc: TOpcode;
-                  flags: TGenFlags) =
-  # a nop for certain types
-  let isAddr = opc in {opcAddrNode, opcAddrReg}
-  if isAddr and (let m = canElimAddr(n); m != nil):
+proc genAddr(c: PCtx, n: PNode, dest: var TDest, flags: TGenFlags) =
+  if (let m = canElimAddr(n); m != nil):
     gen(c, m, dest, flags)
     return
 
-  let af = if n[0].kind in {nkBracketExpr, nkDotExpr, nkCheckedFieldExpr}: {gfAddrOf, gfFieldAccess}
-           else: {gfAddrOf}
-  let newflags = if isAddr: flags+af else: flags
-  # consider:
-  # proc foo(f: var ref int) =
-  #   f = new(int)
-  # proc blah() =
-  #   var x: ref int
-  #   foo x
-  #
-  # The type of 'f' is 'var ref int' and of 'x' is 'ref int'. Hence for
-  # nkAddr we must not use 'unneededIndirection', but for deref we use it.
-  if not isAddr and unneededIndirection(n.sons[0]):
-    gen(c, n.sons[0], dest, newflags)
-    if gfAddrOf notin flags and fitsRegister(n.typ):
-      c.gABC(n, opcNodeToReg, dest, dest)
-  elif isAddr and isGlobal(n.sons[0]):
+  let af = if n[0].kind in {nkBracketExpr, nkDotExpr, nkCheckedFieldExpr}: {gfNode}
+           else: {gfNodeAddr}
+  let newflags = flags-{gfNode, gfNodeAddr}+af
+
+  if isGlobal(n.sons[0]):
     gen(c, n.sons[0], dest, flags+af)
   else:
     let tmp = c.genx(n.sons[0], newflags)
     if dest < 0: dest = c.getTemp(n.typ)
-    if not isAddr:
-      gABC(c, n, opc, dest, tmp)
-      assert n.typ != nil
-      if gfAddrOf notin flags and fitsRegister(n.typ):
-        c.gABC(n, opcNodeToReg, dest, dest)
-    elif c.prc.slots[tmp].kind >= slotTempUnknown:
+    if c.prc.slots[tmp].kind >= slotTempUnknown:
       gABC(c, n, opcAddrNode, dest, tmp)
       # hack ahead; in order to fix bug #1781 we mark the temporary as
       # permanent, so that it's not used for anything else:
@@ -1297,6 +1279,19 @@ proc genAddrDeref(c: PCtx; n: PNode; dest: var TDest; opc: TOpcode;
       gABC(c, n, opcAddrReg, dest, tmp)
     c.freeTemp(tmp)
 
+proc genDeref(c: PCtx, n: PNode, dest: var TDest, flags: TGenFlags) =
+  if unneededIndirection(n.sons[0]):
+    gen(c, n.sons[0], dest, flags)
+    if {gfNodeAddr, gfNode} * flags == {} and fitsRegister(n.typ):
+      c.gABC(n, opcNodeToReg, dest, dest)
+  else:
+    let tmp = c.genx(n.sons[0], flags)
+    if dest < 0: dest = c.getTemp(n.typ)
+    gABC(c, n, opcLdDeref, dest, tmp)
+    assert n.typ != nil
+    if {gfNodeAddr, gfNode} * flags == {} and fitsRegister(n.typ):
+      c.gABC(n, opcNodeToReg, dest, dest)
+
 proc whichAsgnOpc(n: PNode): TOpcode =
   case n.typ.skipTypes(abstractRange-{tyTypeDesc}).kind
   of tyBool, tyChar, tyEnum, tyOrdinal, tyInt..tyInt64, tyUInt..tyUInt64:
@@ -1382,7 +1377,7 @@ proc preventFalseAlias(c: PCtx; n: PNode; opc: TOpcode;
 proc genAsgn(c: PCtx; le, ri: PNode; requiresCopy: bool) =
   case le.kind
   of nkBracketExpr:
-    let dest = c.genx(le.sons[0], {gfAddrOf, gfFieldAccess})
+    let dest = c.genx(le.sons[0], {gfNode})
     let idx = c.genIndex(le.sons[1], le.sons[0].typ)
     let tmp = c.genx(ri)
     if le.sons[0].typ.skipTypes(abstractVarRange-{tyTypeDesc}).kind in {
@@ -1394,13 +1389,13 @@ proc genAsgn(c: PCtx; le, ri: PNode; requiresCopy: bool) =
   of nkDotExpr, nkCheckedFieldExpr:
     # XXX field checks here
     let left = if le.kind == nkDotExpr: le else: le.sons[0]
-    let dest = c.genx(left.sons[0], {gfAddrOf, gfFieldAccess})
+    let dest = c.genx(left.sons[0], {gfNode})
     let idx = genField(c, left.sons[1])
     let tmp = c.genx(ri)
     c.preventFalseAlias(left, opcWrObj, dest, idx, tmp)
     c.freeTemp(tmp)
   of nkDerefExpr, nkHiddenDeref:
-    let dest = c.genx(le.sons[0], {gfAddrOf})
+    let dest = c.genx(le.sons[0], {gfNode})
     let tmp = c.genx(ri)
     c.preventFalseAlias(le, opcWrDeref, dest, 0, tmp)
     c.freeTemp(tmp)
@@ -1409,7 +1404,7 @@ proc genAsgn(c: PCtx; le, ri: PNode; requiresCopy: bool) =
     checkCanEval(c, le)
     if s.isGlobal:
       withTemp(tmp, le.typ):
-        c.gen(le, tmp, {gfAddrOf})
+        c.gen(le, tmp, {gfNodeAddr})
         let val = c.genx(ri)
         c.preventFalseAlias(le, opcWrDeref, tmp, 0, val)
         c.freeTemp(val)
@@ -1427,7 +1422,7 @@ proc genAsgn(c: PCtx; le, ri: PNode; requiresCopy: bool) =
       else:
         gen(c, ri, dest)
   else:
-    let dest = c.genx(le, {gfAddrOf})
+    let dest = c.genx(le, {gfNodeAddr})
     genAsgn(c, dest, ri, requiresCopy)
 
 proc genTypeLit(c: PCtx; t: PType; dest: var TDest) =
@@ -1463,6 +1458,8 @@ proc genGlobalInit(c: PCtx; n: PNode; s: PSym) =
     c.freeTemp(tmp)
 
 proc genRdVar(c: PCtx; n: PNode; dest: var TDest; flags: TGenFlags) =
+  # gfNodeAddr and gfNode are mutually exclusive
+  assert card(flags * {gfNodeAddr, gfNode}) < 2
   let s = n.sym
   if s.isGlobal:
     if sfCompileTime in s.flags or c.mode == emRepl:
@@ -1474,13 +1471,13 @@ proc genRdVar(c: PCtx; n: PNode; dest: var TDest; flags: TGenFlags) =
       else: genGlobalInit(c, n, s)
     if dest < 0: dest = c.getTemp(n.typ)
     assert s.typ != nil
-    if gfAddrOf notin flags and fitsRegister(s.typ):
+    if gfNodeAddr in flags:
+      c.gABx(n, opcLdGlobalAddr, dest, s.position)
+    elif fitsRegister(s.typ) and gfNode notin flags:
       var cc = c.getTemp(n.typ)
       c.gABx(n, opcLdGlobal, cc, s.position)
       c.gABC(n, opcNodeToReg, dest, cc)
       c.freeTemp(cc)
-    elif {gfAddrOf, gfFieldAccess} * flags == {gfAddrOf}:
-      c.gABx(n, opcLdGlobalAddr, dest, s.position)
     else:
       c.gABx(n, opcLdGlobal, dest, s.position)
   else:
@@ -1498,7 +1495,8 @@ proc genRdVar(c: PCtx; n: PNode; dest: var TDest; flags: TGenFlags) =
       cannotEval(c, n)
 
 template needsRegLoad(): untyped =
-  gfAddrOf notin flags and fitsRegister(n.typ.skipTypes({tyVar, tyLent}))
+  {gfNode, gfNodeAddr} * flags == {} and
+    fitsRegister(n.typ.skipTypes({tyVar, tyLent}))
 
 proc genArrAccess2(c: PCtx; n: PNode; dest: var TDest; opc: TOpcode;
                    flags: TGenFlags) =
@@ -1634,7 +1632,7 @@ proc genVarSection(c: PCtx; n: PNode) =
             c.globals.add(sa)
             s.position = c.globals.len
         if a.sons[2].kind != nkEmpty:
-          let tmp = c.genx(a.sons[0], {gfAddrOf})
+          let tmp = c.genx(a.sons[0], {gfNodeAddr})
           let val = c.genx(a.sons[2])
           c.genAdditionalCopy(a.sons[2], opcWrDeref, tmp, 0, val)
           c.freeTemp(val)
@@ -1839,8 +1837,8 @@ proc gen(c: PCtx; n: PNode; dest: var TDest; flags: TGenFlags = {}) =
   of nkDotExpr: genObjAccess(c, n, dest, flags)
   of nkCheckedFieldExpr: genCheckedObjAccess(c, n, dest, flags)
   of nkBracketExpr: genArrAccess(c, n, dest, flags)
-  of nkDerefExpr, nkHiddenDeref: genAddrDeref(c, n, dest, opcLdDeref, flags)
-  of nkAddr, nkHiddenAddr: genAddrDeref(c, n, dest, opcAddrNode, flags)
+  of nkDerefExpr, nkHiddenDeref: genDeref(c, n, dest, flags)
+  of nkAddr, nkHiddenAddr: genAddr(c, n, dest, flags)
   of nkIfStmt, nkIfExpr: genIf(c, n, dest)
   of nkWhenStmt:
     # This is "when nimvm" node. Chose the first branch.
diff --git a/tests/vm/tref.nim b/tests/vm/tref.nim
index 27b7bf313..f5cd23da5 100644
--- a/tests/vm/tref.nim
+++ b/tests/vm/tref.nim
@@ -54,4 +54,9 @@ static:
     new(s)
     var ss = s
     s[] = 1
-    doAssert ss[] == 1
\ No newline at end of file
+    doAssert ss[] == 1
+
+static: # bug #8402
+  type R = ref object
+  var empty: R
+  let otherEmpty = empty
\ No newline at end of file