Skip to content

Commit

Permalink
fix #18977; disallow change branch of an object variant in ORC (#21526)
Browse files Browse the repository at this point in the history
* fix #18977 disallow change branch of an object variant in ORC

* check errors for goto exception

* fixes conditions

* fixes tests

* add a test case for #18977
  • Loading branch information
ringabout authored Mar 16, 2023
1 parent 6552a27 commit b5ee81f
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 32 deletions.
4 changes: 3 additions & 1 deletion compiler/ccgstmts.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1579,6 +1579,8 @@ proc genDiscriminantCheck(p: BProc, a, tmp: TLoc, objtype: PType,
"#FieldDiscriminantCheck((NI)(NU)($1), (NI)(NU)($2), $3, $4);$n",
[rdLoc(a), rdLoc(tmp), discriminatorTableName(p.module, t, field),
lit])
if p.config.exc == excGoto:
raiseExit(p)

when false:
proc genCaseObjDiscMapping(p: BProc, e: PNode, t: PType, field: PSym; d: var TLoc) =
Expand All @@ -1603,7 +1605,7 @@ proc asgnFieldDiscriminant(p: BProc, e: PNode) =
initLocExpr(p, e[0], a)
getTemp(p, a.t, tmp)
expr(p, e[1], tmp)
if optTinyRtti notin p.config.globalOptions and p.inUncheckedAssignSection == 0:
if p.inUncheckedAssignSection == 0:
let field = dotExpr[1].sym
genDiscriminantCheck(p, a, tmp, dotExpr[0].typ, field)
message(p.config, e.info, warnCaseTransition)
Expand Down
45 changes: 33 additions & 12 deletions compiler/injectdestructors.nim
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ import
intsets, strtabs, ast, astalgo, msgs, renderer, magicsys, types, idents,
strutils, options, lowerings, tables, modulegraphs,
lineinfos, parampatterns, sighashes, liftdestructors, optimizer,
varpartitions, aliasanalysis, dfa
varpartitions, aliasanalysis, dfa, wordrecg

when defined(nimPreviewSlimSystem):
import std/assertions

from trees import exprStructuralEquivalent, getRoot
from trees import exprStructuralEquivalent, getRoot, whichPragma

type
Con = object
Expand All @@ -35,6 +35,7 @@ type
idgen: IdGenerator
body: PNode
otherUsage: TLineInfo
inUncheckedAssignSection: int

Scope = object # we do scope-based memory management.
# a scope is comparable to an nkStmtListExpr like
Expand Down Expand Up @@ -342,15 +343,16 @@ It is best to factor out piece of object that needs custom destructor into separ
return

# generate: if le != tmp: `=destroy`(le)
let branchDestructor = produceDestructorForDiscriminator(c.graph, objType, leDotExpr[1].sym, n.info, c.idgen)
let cond = newNodeIT(nkInfix, n.info, getSysType(c.graph, unknownLineInfo, tyBool))
cond.add newSymNode(getMagicEqSymForType(c.graph, le.typ, n.info))
cond.add le
cond.add tmp
let notExpr = newNodeIT(nkPrefix, n.info, getSysType(c.graph, unknownLineInfo, tyBool))
notExpr.add newSymNode(createMagic(c.graph, c.idgen, "not", mNot))
notExpr.add cond
result.add newTree(nkIfStmt, newTree(nkElifBranch, notExpr, c.genOp(branchDestructor, le)))
if c.inUncheckedAssignSection != 0:
let branchDestructor = produceDestructorForDiscriminator(c.graph, objType, leDotExpr[1].sym, n.info, c.idgen)
let cond = newNodeIT(nkInfix, n.info, getSysType(c.graph, unknownLineInfo, tyBool))
cond.add newSymNode(getMagicEqSymForType(c.graph, le.typ, n.info))
cond.add le
cond.add tmp
let notExpr = newNodeIT(nkPrefix, n.info, getSysType(c.graph, unknownLineInfo, tyBool))
notExpr.add newSymNode(createMagic(c.graph, c.idgen, "not", mNot))
notExpr.add cond
result.add newTree(nkIfStmt, newTree(nkElifBranch, notExpr, c.genOp(branchDestructor, le)))
result.add newTree(nkFastAsgn, le, tmp)

proc genWasMoved(c: var Con, n: PNode): PNode =
Expand Down Expand Up @@ -877,14 +879,33 @@ proc p(n: PNode; c: var Con; s: var Scope; mode: ProcessMode; tmpFlags = {sfSing
nkTypeOfExpr, nkMixinStmt, nkBindStmt:
result = n

of nkStringToCString, nkCStringToString, nkChckRangeF, nkChckRange64, nkChckRange, nkPragmaBlock:
of nkStringToCString, nkCStringToString, nkChckRangeF, nkChckRange64, nkChckRange:
result = shallowCopy(n)
for i in 0 ..< n.len:
result[i] = p(n[i], c, s, normal)
if n.typ != nil and hasDestructor(c, n.typ):
if mode == normal:
result = ensureDestruction(result, n, c, s)

of nkPragmaBlock:
var inUncheckedAssignSection = 0
let pragmaList = n[0]
for pi in pragmaList:
if whichPragma(pi) == wCast:
case whichPragma(pi[1])
of wUncheckedAssign:
inUncheckedAssignSection = 1
else:
discard
result = shallowCopy(n)
inc c.inUncheckedAssignSection, inUncheckedAssignSection
for i in 0 ..< n.len:
result[i] = p(n[i], c, s, normal)
dec c.inUncheckedAssignSection, inUncheckedAssignSection
if n.typ != nil and hasDestructor(c, n.typ):
if mode == normal:
result = ensureDestruction(result, n, c, s)

of nkHiddenSubConv, nkHiddenStdConv, nkConv:
# we have an "ownership invariance" for all constructors C(x).
# See the comment for nkBracket construction. If the caller wants
Expand Down
26 changes: 26 additions & 0 deletions tests/arc/t18977.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
discard """
matrix: "--mm:arc"
"""

type
E = enum
a, b, c, d
X = object
v: int
O = object
case kind: E
of a:
a: int
of {b, c}:
b: float
else:
d: X

proc `=destroy`(x: var X) =
echo "x destroyed"

var o = O(kind: d, d: X(v: 12345))
doAssert o.d.v == 12345

doAssertRaises(FieldDefect):
o.kind = a
28 changes: 18 additions & 10 deletions tests/arc/tcaseobj.nim
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ proc `=destroy`(o: var TMyObj) =
o.p = nil
echo "myobj destroyed"

proc `=`(dst: var TMyObj, src: TMyObj) =
proc `=copy`(dst: var TMyObj, src: TMyObj) =
`=destroy`(dst)
dst.p = alloc(src.len)
dst.len = src.len
Expand Down Expand Up @@ -170,18 +170,24 @@ proc test_myobject =
x.x1 = "x1"
x.x2 = "x2"
x.y1 = "ljhkjhkjh"
x.kind1 = true
{.cast(uncheckedAssign).}:
x.kind1 = true
x.y2 = @["1", "2"]
x.kind2 = true
{.cast(uncheckedAssign).}:
x.kind2 = true
x.z1 = "yes"
x.kind2 = false
{.cast(uncheckedAssign).}:
x.kind2 = false
x.z2 = @["1", "2"]
x.kind2 = true
{.cast(uncheckedAssign).}:
x.kind2 = true
x.z1 = "yes"
x.kind2 = true # should be no effect
doAssert(x.z1 == "yes")
x.kind2 = false
x.kind1 = x.kind2 # support self assignment with effect
{.cast(uncheckedAssign).}:
x.kind2 = false
{.cast(uncheckedAssign).}:
x.kind1 = x.kind2 # support self assignment with effect

try:
x.kind1 = x.flag # flag is not accesible
Expand All @@ -207,8 +213,9 @@ type
error*: string

proc init(): RocksDBResult[string] =
result.ok = true
result.value = "ok"
{.cast(uncheckedAssign).}:
result.ok = true
result.value = "ok"

echo init()

Expand All @@ -222,7 +229,8 @@ type MyObj = object
of true: x1: string

var a = MyObj(kind: false, x0: 1234)
a.kind = true
{.cast(uncheckedAssign).}:
a.kind = true
doAssert(a.x1 == "")

block:
Expand Down
23 changes: 15 additions & 8 deletions tests/arc/tcaseobjcopy.nim
Original file line number Diff line number Diff line change
Expand Up @@ -169,18 +169,23 @@ proc test_myobject =
x.x1 = "x1"
x.x2 = "x2"
x.y1 = "ljhkjhkjh"
x.kind1 = true
{.cast(uncheckedAssign).}:
x.kind1 = true
x.y2 = @["1", "2"]
x.kind2 = true
{.cast(uncheckedAssign).}:
x.kind2 = true
x.z1 = "yes"
x.kind2 = false
{.cast(uncheckedAssign).}:
x.kind2 = false
x.z2 = @["1", "2"]
x.kind2 = true
{.cast(uncheckedAssign).}:
x.kind2 = true
x.z1 = "yes"
x.kind2 = true # should be no effect
doAssert(x.z1 == "yes")
x.kind2 = false
x.kind1 = x.kind2 # support self assignment with effect
{.cast(uncheckedAssign).}:
x.kind2 = false
x.kind1 = x.kind2 # support self assignment with effect

try:
x.kind1 = x.flag # flag is not accesible
Expand All @@ -206,7 +211,8 @@ type
error*: string

proc init(): RocksDBResult[string] =
result.ok = true
{.cast(uncheckedAssign).}:
result.ok = true
result.value = "ok"

echo init()
Expand All @@ -221,7 +227,8 @@ type MyObj = object
of true: x1: string

var a = MyObj(kind: false, x0: 1234)
a.kind = true
{.cast(uncheckedAssign).}:
a.kind = true
doAssert(a.x1 == "")

block:
Expand Down
3 changes: 2 additions & 1 deletion tests/destructor/tgotoexceptions7.nim
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ proc helper = doAssert(false)

proc main(i: int) =
var obj = Obj(kind: kindA, s: "abc")
obj.kind = kindB
{.cast(uncheckedAssign).}:
obj.kind = kindB
obj.i = 2
try:
var objA = ObjA()
Expand Down

0 comments on commit b5ee81f

Please sign in to comment.