From 94358e03e914e171182a6773e51d9ec33a593aa5 Mon Sep 17 00:00:00 2001 From: Araq Date: Mon, 12 Dec 2011 01:40:23 +0100 Subject: [PATCH] compiler generates volatile temps to keep C compiler from optimizing away stack roots --- compiler/ccgcalls.nim | 2 ++ compiler/ccgexprs.nim | 44 +++++++++++++++++++++++++++---------------- compiler/cgen.nim | 32 +++++++++++++++++++++++++------ lib/system/gc.nim | 32 ++++++++++++++++++++++++++++--- todo.txt | 3 --- 5 files changed, 85 insertions(+), 28 deletions(-) diff --git a/compiler/ccgcalls.nim b/compiler/ccgcalls.nim index 570c931fbb..b28765f8f2 100644 --- a/compiler/ccgcalls.nim +++ b/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) diff --git a/compiler/ccgexprs.nim b/compiler/ccgexprs.nim index ddd272fba4..dc2b5857ad 100755 --- a/compiler/ccgexprs.nim +++ b/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) = # @@ -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) diff --git a/compiler/cgen.nim b/compiler/cgen.nim index aa07f40aa3..16210c026b 100755 --- a/compiler/cgen.nim +++ b/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) diff --git a/lib/system/gc.nim b/lib/system/gc.nim index 3edde8a865..66b84a8c5a 100755 --- a/lib/system/gc.nim +++ b/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) diff --git a/todo.txt b/todo.txt index 33dc52bf94..9f77fcf2ef 100755 --- a/todo.txt +++ b/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