compiler generates volatile temps to keep C compiler from optimizing away stack roots

compiler/ccgcalls.nim

@@ -252,6 +252,7 @@ proc genCall(p: BProc, e: PNode, d: var TLoc) =
     genNamedParamCall(p, e, d)
   else:
     genPrefixCall(p, nil, e, d)
+  if d.s == onStack and containsGarbageCollectedRef(d.t): keepAlive(p, d)
 
 proc genAsgnCall(p: BProc, le, ri: PNode, d: var TLoc) =
   if ri.sons[0].kind == nkSym and sfInfixCall in ri.sons[0].sym.flags and
@@ -261,4 +262,5 @@ proc genAsgnCall(p: BProc, le, ri: PNode, d: var TLoc) =
     genNamedParamCall(p, ri, d)
   else:
     genPrefixCall(p, le, ri, d)
+  if d.s == onStack and containsGarbageCollectedRef(d.t): keepAlive(p, d)
 

compiler/ccgexprs.nim

@@ -15,7 +15,7 @@ proc lenField: PRope {.inline.} =
 proc intLiteral(i: biggestInt): PRope =
   if (i > low(int32)) and (i <= high(int32)):
     result = toRope(i)
-  elif i == low(int32):       
+  elif i == low(int32):
     # Nimrod has the same bug for the same reasons :-)
     result = toRope("(-2147483647 -1)")
   elif i > low(int64):
@@ -159,6 +159,7 @@ proc getStorageLoc(n: PNode): TStorageLoc =
 proc genRefAssign(p: BProc, dest, src: TLoc, flags: TAssignmentFlags) =
   if dest.s == OnStack or optRefcGC notin gGlobalOptions:
     appf(p.s[cpsStmts], "$1 = $2;$n", [rdLoc(dest), rdLoc(src)])
+    if needToKeepAlive in flags: keepAlive(p, dest)
   elif dest.s == OnHeap:
     # location is on heap
     # now the writer barrier is inlined for performance:
@@ -185,6 +186,7 @@ proc genRefAssign(p: BProc, dest, src: TLoc, flags: TAssignmentFlags) =
   else:
     appcg(p.module, p.s[cpsStmts], "#unsureAsgnRef((void**) $1, $2);$n",
          [addrLoc(dest), rdLoc(src)])
+    if needToKeepAlive in flags: keepAlive(p, dest)
 
 proc genGenericAsgn(p: BProc, dest, src: TLoc, flags: TAssignmentFlags) =
   # Consider: 
@@ -199,6 +201,7 @@ proc genGenericAsgn(p: BProc, dest, src: TLoc, flags: TAssignmentFlags) =
       appcg(p, cpsStmts,
            "memcpy((void*)$1, (NIM_CONST void*)$2, sizeof($3));$n",
            [addrLoc(dest), addrLoc(src), rdLoc(dest)])
+      if needToKeepAlive in flags: keepAlive(p, dest)
     else:
       appcg(p, cpsStmts, "#genericShallowAssign((void*)$1, (void*)$2, $3);$n",
            [addrLoc(dest), addrLoc(src), genTypeInfo(p.module, dest.t)])
@@ -229,12 +232,14 @@ proc genAssignment(p: BProc, dest, src: TLoc, flags: TAssignmentFlags) =
     else:
       if dest.s == OnStack or optRefcGC notin gGlobalOptions:
         appcg(p, cpsStmts, "$1 = #copyString($2);$n", [rdLoc(dest), rdLoc(src)])
+        if needToKeepAlive in flags: keepAlive(p, dest)
       elif dest.s == OnHeap:
         appcg(p, cpsStmts, "#asgnRefNoCycle((void**) $1, #copyString($2));$n",
              [addrLoc(dest), rdLoc(src)])
       else:
         appcg(p, cpsStmts, "#unsureAsgnRef((void**) $1, #copyString($2));$n",
              [addrLoc(dest), rdLoc(src)])
+        if needToKeepAlive in flags: keepAlive(p, dest)
   of tyTuple, tyObject:
     # XXX: check for subtyping?
     if needsComplexAssignment(dest.t):
@@ -302,11 +307,11 @@ proc putIntoDest(p: BProc, d: var TLoc, t: PType, r: PRope) =
     d.a = -1
 
 proc binaryStmt(p: BProc, e: PNode, d: var TLoc, frmt: string) =
-  var a, b: TLoc
+  var b: TLoc
   if d.k != locNone: InternalError(e.info, "binaryStmt")
-  InitLocExpr(p, e.sons[1], a)
+  InitLocExpr(p, e.sons[1], d)
   InitLocExpr(p, e.sons[2], b)
-  appcg(p, cpsStmts, frmt, [rdLoc(a), rdLoc(b)])
+  appcg(p, cpsStmts, frmt, [rdLoc(d), rdLoc(b)])
 
 proc unaryStmt(p: BProc, e: PNode, d: var TLoc, frmt: string) =
   var a: TLoc
@@ -824,8 +829,9 @@ proc genStrConcat(p: BProc, e: PNode, d: var TLoc) =
   app(p.s[cpsStmts], appends)
   if d.k == locNone:
     d = tmp
+    keepAlive(p, tmp)
   else:
-    genAssignment(p, d, tmp, {}) # no need for deep copying
+    genAssignment(p, d, tmp, {needToKeepAlive}) # no need for deep copying
 
 proc genStrAppend(p: BProc, e: PNode, d: var TLoc) =
   #  <Nimrod code>
@@ -841,12 +847,9 @@ proc genStrAppend(p: BProc, e: PNode, d: var TLoc) =
   #  }
   var
     a, dest: TLoc
-    L: int
     appends, lens: PRope
   assert(d.k == locNone)
-  L = 0
-  appends = nil
-  lens = nil
+  var L = 0
   initLocExpr(p, e.sons[1], dest)
   for i in countup(0, sonsLen(e) - 3):
     # compute the length expression:
@@ -864,6 +867,7 @@ proc genStrAppend(p: BProc, e: PNode, d: var TLoc) =
             [rdLoc(dest), rdLoc(a)])
   appcg(p, cpsStmts, "$1 = #resizeString($1, $2$3);$n",
        [rdLoc(dest), lens, toRope(L)])
+  keepAlive(p, dest)
   app(p.s[cpsStmts], appends)
 
 proc genSeqElemAppend(p: BProc, e: PNode, d: var TLoc) =
@@ -877,6 +881,7 @@ proc genSeqElemAppend(p: BProc, e: PNode, d: var TLoc) =
       rdLoc(a),
       getTypeDesc(p.module, skipTypes(e.sons[1].typ, abstractVar)),
       getTypeDesc(p.module, skipTypes(e.sons[2].Typ, abstractVar))])
+  keepAlive(p, a)
   initLoc(dest, locExpr, b.t, OnHeap)
   dest.r = ropef("$1->data[$1->$2-1]", [rdLoc(a), lenField()])
   genAssignment(p, dest, b, {needToCopy, afDestIsNil})
@@ -898,7 +903,7 @@ proc genNew(p: BProc, e: PNode) =
       "($1) #newObj($2, sizeof($3))", [getTypeDesc(p.module, reftype),
       genTypeInfo(p.module, refType),
       getTypeDesc(p.module, skipTypes(reftype.sons[0], abstractRange))])
-  genAssignment(p, a, b, {})  # set the object type:
+  genAssignment(p, a, b, {needToKeepAlive})  # set the object type:
   bt = skipTypes(refType.sons[0], abstractRange)
   genObjectInit(p, cpsStmts, bt, a, false)
 
@@ -912,7 +917,7 @@ proc genNewSeq(p: BProc, e: PNode) =
   c.r = ropecg(p.module, "($1) #newSeq($2, $3)", [
                getTypeDesc(p.module, seqtype),
                genTypeInfo(p.module, seqType), rdLoc(b)])
-  genAssignment(p, a, c, {})
+  genAssignment(p, a, c, {needToKeepAlive})
 
 proc genOf(p: BProc, x: PNode, typ: PType, d: var TLoc) =
   var a: TLoc
@@ -960,11 +965,12 @@ proc genNewFinalize(p: BProc, e: PNode) =
   b.r = ropecg(p.module, "($1) #newObj($2, sizeof($3))", [
       getTypeDesc(p.module, refType),
       ti, getTypeDesc(p.module, skipTypes(reftype.sons[0], abstractRange))])
-  genAssignment(p, a, b, {})  # set the object type:
+  genAssignment(p, a, b, {needToKeepAlive})  # set the object type:
   bt = skipTypes(refType.sons[0], abstractRange)
   genObjectInit(p, cpsStmts, bt, a, false)
 
 proc genRepr(p: BProc, e: PNode, d: var TLoc) =
+  # XXX we don't generate keep alive info for now here
   var a: TLoc
   InitLocExpr(p, e.sons[1], a)
   var t = skipTypes(e.sons[1].typ, abstractVarRange)
@@ -1019,7 +1025,7 @@ proc genDollar(p: BProc, n: PNode, d: var TLoc, frmt: string) =
   InitLocExpr(p, n.sons[1], a)
   a.r = ropecg(p.module, frmt, [rdLoc(a)])
   if d.k == locNone: getTemp(p, n.typ, d)
-  genAssignment(p, d, a, {})
+  genAssignment(p, d, a, {needToKeepAlive})
 
 proc genArrayLen(p: BProc, e: PNode, d: var TLoc, op: TMagic) =
   var a = e.sons[1]
@@ -1055,9 +1061,11 @@ proc genSetLengthSeq(p: BProc, e: PNode, d: var TLoc) =
       "$1 = ($3) #setLengthSeq(&($1)->Sup, sizeof($4), $2);$n", [
       rdLoc(a), rdLoc(b), getTypeDesc(p.module, t),
       getTypeDesc(p.module, t.sons[0])])
+  keepAlive(p, a)
 
 proc genSetLengthStr(p: BProc, e: PNode, d: var TLoc) =
   binaryStmt(p, e, d, "$1 = #setLengthStr($1, $2);$n")
+  keepAlive(P, d)
 
 proc genSwap(p: BProc, e: PNode, d: var TLoc) =
   # swap(a, b) -->
@@ -1246,6 +1254,7 @@ proc convStrToCStr(p: BProc, n: PNode, d: var TLoc) =
       [rdLoc(a)]))
 
 proc convCStrToStr(p: BProc, n: PNode, d: var TLoc) =
+  # XXX we don't generate keep alive info here
   var a: TLoc
   initLocExpr(p, n.sons[0], a)
   putIntoDest(p, d, skipTypes(n.typ, abstractVar),
@@ -1277,7 +1286,7 @@ proc genSeqConstr(p: BProc, t: PNode, d: var TLoc) =
   newSeq.r = ropecg(p.module, "($1) #newSeq($2, $3)", 
       [getTypeDesc(p.module, t.typ),
       genTypeInfo(p.module, t.typ), intLiteral(sonsLen(t))])
-  genAssignment(p, d, newSeq, {afSrcIsNotNil})
+  genAssignment(p, d, newSeq, {afSrcIsNotNil, needToKeepAlive})
   for i in countup(0, sonsLen(t) - 1):
     initLoc(arr, locExpr, elemType(skipTypes(t.typ, abstractInst)), OnHeap)
     arr.r = ropef("$1->data[$2]", [rdLoc(d), intLiteral(i)])
@@ -1298,7 +1307,7 @@ proc genArrToSeq(p: BProc, t: PNode, d: var TLoc) =
   newSeq.r = ropecg(p.module, "($1) #newSeq($2, $3)", 
       [getTypeDesc(p.module, t.typ),
       genTypeInfo(p.module, t.typ), intLiteral(L)])
-  genAssignment(p, d, newSeq, {afSrcIsNotNil})
+  genAssignment(p, d, newSeq, {afSrcIsNotNil, needToKeepAlive})
   initLocExpr(p, t.sons[1], a)
   for i in countup(0, L - 1):
     initLoc(elem, locExpr, elemType(skipTypes(t.typ, abstractInst)), OnHeap)
@@ -1363,7 +1372,10 @@ proc genMagicExpr(p: BProc, e: PNode, d: var TLoc, op: TMagic) =
     else:
       binaryStmt(p, e, d, "$1 = #subInt($1, $2);$n")
   of mConStrStr: genStrConcat(p, e, d)
-  of mAppendStrCh: binaryStmt(p, e, d, "$1 = #addChar($1, $2);$n")
+  of mAppendStrCh:
+    binaryStmt(p, e, d, "$1 = #addChar($1, $2);$n")
+    # strictly speaking we need to generate "keepAlive" here too, but this
+    # very likely not needed and would slow down the code too much I fear
   of mAppendStrStr: genStrAppend(p, e, d)
   of mAppendSeqElem: genSeqElemAppend(p, e, d)
   of mEqStr: genStrEquals(p, e, d)

compiler/cgen.nim

@@ -218,14 +218,16 @@ proc genObjectInit(p: BProc, section: TCProcSection, t: PType, a: TLoc,
 type
   TAssignmentFlag = enum
     needToCopy, needForSubtypeCheck, afDestIsNil, afDestIsNotNil, afSrcIsNil,
-    afSrcIsNotNil
+    afSrcIsNotNil, needToKeepAlive
   TAssignmentFlags = set[TAssignmentFlag]
 
 proc genRefAssign(p: BProc, dest, src: TLoc, flags: TAssignmentFlags)
 
+const
+  complexValueType = {tyArray, tyArrayConstr, tySet, tyTuple, tyObject}
+
 proc zeroVar(p: BProc, loc: TLoc, containsGCref: bool) = 
-  if skipTypes(loc.t, abstractVarRange).Kind notin
-      {tyArray, tyArrayConstr, tySet, tyTuple, tyObject}: 
+  if skipTypes(loc.t, abstractVarRange).Kind notin ComplexValueType: 
     if containsGcref and p.WithInLoop > 0:
       appf(p.s[cpsInit], "$1 = 0;$n", [rdLoc(loc)])
       var nilLoc: TLoc
@@ -248,8 +250,7 @@ proc zeroVar(p: BProc, loc: TLoc, containsGCref: bool) =
       genObjectInit(p, cpsStmts, loc.t, loc, true)
 
 proc zeroTemp(p: BProc, loc: TLoc) = 
-  if skipTypes(loc.t, abstractVarRange).Kind notin
-      {tyArray, tyArrayConstr, tySet, tyTuple, tyObject}: 
+  if skipTypes(loc.t, abstractVarRange).Kind notin complexValueType: 
     appf(p.s[cpsStmts], "$1 = 0;$n", [rdLoc(loc)])
     when false:
       var nilLoc: TLoc
@@ -257,7 +258,7 @@ proc zeroTemp(p: BProc, loc: TLoc) =
       nilLoc.r = toRope("NIM_NIL")
       # puts ``unsureAsgnRef`` etc to ``p.s[cpsStmts]``:
       genRefAssign(p, loc, nilLoc, {afSrcIsNil})
-  else: 
+  else:
     appf(p.s[cpsStmts], "memset((void*)$1, 0, sizeof($2));$n", 
          [addrLoc(loc), rdLoc(loc)])
     # XXX no object init necessary for temporaries?
@@ -295,6 +296,25 @@ proc getTemp(p: BProc, t: PType, result: var TLoc) =
   result.flags = {}
   initTemp(p, result)
 
+proc keepAlive(p: BProc, toKeepAlive: TLoc) =
+  if optRefcGC notin gGlobalOptions: return
+  var result: TLoc
+  inc(p.labels)
+  result.r = con("LOC", toRope(p.labels))
+  appf(p.s[cpsLocals], "volatile $1 $2;$n",
+      [getTypeDesc(p.module, toKeepAlive.t), result.r])
+  result.k = locTemp
+  result.a = -1
+  result.t = toKeepAlive.t
+  result.s = OnStack
+  result.flags = {}
+  if skipTypes(toKeepAlive.t, abstractVarRange).Kind notin complexValueType:
+    appf(p.s[cpsStmts], "$1 = $2;$n", [rdLoc(result), rdLoc(toKeepAlive)])
+  else:
+    appcg(p, cpsStmts,
+         "memcpy((void*)$1, (NIM_CONST void*)$2, sizeof($3));$n",
+         [addrLoc(result), addrLoc(toKeepAlive), rdLoc(result)])
+
 proc cstringLit(p: BProc, r: var PRope, s: string): PRope = 
   if gCmd == cmdCompileToLLVM: 
     inc(p.module.labels)

lib/system/gc.nim

@@ -374,13 +374,12 @@ proc addNewObjToZCT(res: PCell, gch: var TGcHeap) {.inline.} =
         return
     add(gch.zct, res)
 
-proc newObj(typ: PNimType, size: int, gch: var TGcHeap): pointer =
+proc rawNewObj(typ: PNimType, size: int, gch: var TGcHeap): pointer =
   # generates a new object and sets its reference counter to 0
   acquire(gch)
   sysAssert(typ.kind in {tyRef, tyString, tySequence}, "newObj: 1")
   collectCT(gch)
   var res = cast[PCell](rawAlloc(gch.region, size + sizeof(TCell)))
-  zeroMem(res, size+sizeof(TCell))
   sysAssert((cast[TAddress](res) and (MemAlign-1)) == 0, "newObj: 2")
   # now it is buffered in the ZCT
   res.typ = typ
@@ -398,7 +397,8 @@ proc newObj(typ: PNimType, size: int, gch: var TGcHeap): pointer =
   result = cellToUsr(res)
 
 proc newObj(typ: PNimType, size: int): pointer {.compilerRtl.} =
-  result = newObj(typ, size, gch)
+  result = rawNewObj(typ, size, gch)
+  zeroMem(result, size)
 
 proc newSeq(typ: PNimType, len: int): pointer {.compilerRtl.} =
   # `newObj` already uses locks, so no need for them here.
@@ -406,6 +406,32 @@ proc newSeq(typ: PNimType, len: int): pointer {.compilerRtl.} =
   cast[PGenericSeq](result).len = len
   cast[PGenericSeq](result).space = len
 
+proc newObjRC1(typ: PNimType, size: int): pointer {.compilerRtl.} =
+  # generates a new object and sets its reference counter to 1
+  acquire(gch)
+  sysAssert(typ.kind in {tyRef, tyString, tySequence}, "newObj: 1")
+  collectCT(gch)
+  var res = cast[PCell](rawAlloc(gch.region, size + sizeof(TCell)))
+  sysAssert((cast[TAddress](res) and (MemAlign-1)) == 0, "newObj: 2")
+  # now it is buffered in the ZCT
+  res.typ = typ
+  when debugGC and not hasThreadSupport:
+    if framePtr != nil and framePtr.prev != nil:
+      res.filename = framePtr.prev.filename
+      res.line = framePtr.prev.line
+  res.refcount = rcIncrement # refcount is 1  
+  sysAssert(isAllocatedPtr(gch.region, res), "newObj: 3")
+  when logGC: writeCell("new cell", res)
+  gcTrace(res, csAllocated)
+  release(gch)
+  result = cellToUsr(res)
+  zeroMem(result, size)
+
+proc newSeqRC1(typ: PNimType, len: int): pointer {.compilerRtl.} =
+  result = newObjRC1(typ, addInt(mulInt(len, typ.base.size), GenericSeqSize))
+  cast[PGenericSeq](result).len = len
+  cast[PGenericSeq](result).space = len
+  
 proc growObj(old: pointer, newsize: int, gch: var TGcHeap): pointer =
   acquire(gch)
   collectCT(gch)

todo.txt

@@ -1,9 +1,6 @@
 version 0.8.14
 ==============
 
-- implement a proper bugfix for C compilers optimizing away stack roots
-
-- cleanup file path handling in the compiler
 - warning for implicit openArray -> varargs convention
 - implement explicit varargs; **but** ``len(varargs)`` problem remains! 
   --> solve by implicit conversion from varargs to openarray