fixes #19724; don't be aggressive when you infer sink parameters (#20314)

* fixes #19724; don't be aggressive when you infer sink parameters

* better logic and updated tests

* wip

* fixes tests (#20330)

* restore tests

* try splitPath

Co-authored-by: xflywind <43030857+xflywind@users.noreply.github.com>

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: ringabout <43030857+ringabout@users.noreply.github.com>
Co-authored-by: xflywind <43030857+xflywind@users.noreply.github.com>
This commit is contained in:
Andreas Rumpf 2022-09-12 00:58:29 +02:00 • committed by GitHub
commit 6bf21e7b3d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 18 additions and 10 deletions

View file

@ -67,7 +67,7 @@ type
exc: PNode # stack of exceptions exc: PNode # stack of exceptions
tags: PNode # list of tags tags: PNode # list of tags
forbids: PNode # list of tags forbids: PNode # list of tags
bottom, inTryStmt, inExceptOrFinallyStmt, leftPartOfAsgn: int bottom, inTryStmt, inExceptOrFinallyStmt, leftPartOfAsgn, inIfStmt: int
owner: PSym owner: PSym
ownerModule: PSym ownerModule: PSym
init: seq[int] # list of initialized variables init: seq[int] # list of initialized variables
@ -681,6 +681,7 @@ proc breaksBlock(n: PNode): bool =
proc trackCase(tracked: PEffects, n: PNode) = proc trackCase(tracked: PEffects, n: PNode) =
track(tracked, n[0]) track(tracked, n[0])
inc tracked.inIfStmt
let oldState = tracked.init.len let oldState = tracked.init.len
let oldFacts = tracked.guards.s.len let oldFacts = tracked.guards.s.len
let stringCase = n[0].typ != nil and skipTypes(n[0].typ, let stringCase = n[0].typ != nil and skipTypes(n[0].typ,
@ -707,9 +708,11 @@ proc trackCase(tracked: PEffects, n: PNode) =
if count >= toCover: tracked.init.add id if count >= toCover: tracked.init.add id
# else we can't merge # else we can't merge
setLen(tracked.guards.s, oldFacts) setLen(tracked.guards.s, oldFacts)
dec tracked.inIfStmt
proc trackIf(tracked: PEffects, n: PNode) = proc trackIf(tracked: PEffects, n: PNode) =
track(tracked, n[0][0]) track(tracked, n[0][0])
inc tracked.inIfStmt
let oldFacts = tracked.guards.s.len let oldFacts = tracked.guards.s.len
addFact(tracked.guards, n[0][0]) addFact(tracked.guards, n[0][0])
let oldState = tracked.init.len let oldState = tracked.init.len
@ -740,6 +743,7 @@ proc trackIf(tracked: PEffects, n: PNode) =
if count >= toCover: tracked.init.add id if count >= toCover: tracked.init.add id
# else we can't merge as it is not exhaustive # else we can't merge as it is not exhaustive
setLen(tracked.guards.s, oldFacts) setLen(tracked.guards.s, oldFacts)
dec tracked.inIfStmt
proc trackBlock(tracked: PEffects, n: PNode) = proc trackBlock(tracked: PEffects, n: PNode) =
if n.kind in {nkStmtList, nkStmtListExpr}: if n.kind in {nkStmtList, nkStmtListExpr}:
@ -819,6 +823,10 @@ proc passedToEffectsDelayedParam(tracked: PEffects; n: PNode) =
markSideEffect(tracked, n, n.info) markSideEffect(tracked, n, n.info)
]# ]#
proc checkForSink(tracked: PEffects; n: PNode) =
if tracked.inIfStmt == 0:
checkForSink(tracked.config, tracked.c.idgen, tracked.owner, n)
proc trackCall(tracked: PEffects; n: PNode) = proc trackCall(tracked: PEffects; n: PNode) =
template gcsafeAndSideeffectCheck() = template gcsafeAndSideeffectCheck() =
if notGcSafe(op) and not importedFromC(a): if notGcSafe(op) and not importedFromC(a):
@ -918,7 +926,7 @@ proc trackCall(tracked: PEffects; n: PNode) =
case op[i].kind case op[i].kind
of tySink: of tySink:
createTypeBoundOps(tracked, op[i][0], n.info) createTypeBoundOps(tracked, op[i][0], n.info)
checkForSink(tracked.config, tracked.c.idgen, tracked.owner, n[i]) checkForSink(tracked, n[i])
of tyVar: of tyVar:
tracked.hasDangerousAssign = true tracked.hasDangerousAssign = true
#of tyOut: #of tyOut:
@ -1080,7 +1088,7 @@ proc track(tracked: PEffects, n: PNode) =
if tracked.owner.kind != skMacro and n[0].typ.kind notin {tyOpenArray, tyVarargs}: if tracked.owner.kind != skMacro and n[0].typ.kind notin {tyOpenArray, tyVarargs}:
createTypeBoundOps(tracked, n[0].typ, n.info) createTypeBoundOps(tracked, n[0].typ, n.info)
if n[0].kind != nkSym or not isLocalVar(tracked, n[0].sym): if n[0].kind != nkSym or not isLocalVar(tracked, n[0].sym):
checkForSink(tracked.config, tracked.c.idgen, tracked.owner, n[1]) checkForSink(tracked, n[1])
if not tracked.hasDangerousAssign and n[0].kind != nkSym: if not tracked.hasDangerousAssign and n[0].kind != nkSym:
tracked.hasDangerousAssign = true tracked.hasDangerousAssign = true
of nkVarSection, nkLetSection: of nkVarSection, nkLetSection:
@ -1187,9 +1195,9 @@ proc track(tracked: PEffects, n: PNode) =
if x.kind == nkExprColonExpr: if x.kind == nkExprColonExpr:
if x[0].kind == nkSym: if x[0].kind == nkSym:
notNilCheck(tracked, x[1], x[0].sym.typ) notNilCheck(tracked, x[1], x[0].sym.typ)
checkForSink(tracked.config, tracked.c.idgen, tracked.owner, x[1]) checkForSink(tracked, x[1])
else: else:
checkForSink(tracked.config, tracked.c.idgen, tracked.owner, x) checkForSink(tracked, x)
setLen(tracked.guards.s, oldFacts) setLen(tracked.guards.s, oldFacts)
if tracked.owner.kind != skMacro: if tracked.owner.kind != skMacro:
# XXX n.typ can be nil in runnableExamples, we need to do something about it. # XXX n.typ can be nil in runnableExamples, we need to do something about it.
@ -1204,7 +1212,7 @@ proc track(tracked: PEffects, n: PNode) =
createTypeBoundOps(tracked, n[i][0].typ, n.info) createTypeBoundOps(tracked, n[i][0].typ, n.info)
else: else:
createTypeBoundOps(tracked, n[i].typ, n.info) createTypeBoundOps(tracked, n[i].typ, n.info)
checkForSink(tracked.config, tracked.c.idgen, tracked.owner, n[i]) checkForSink(tracked, n[i])
of nkPragmaBlock: of nkPragmaBlock:
let pragmaList = n[0] let pragmaList = n[0]
var bc = createBlockContext(tracked) var bc = createBlockContext(tracked)
@ -1271,7 +1279,7 @@ proc track(tracked: PEffects, n: PNode) =
of nkBracket: of nkBracket:
for i in 0..<n.safeLen: for i in 0..<n.safeLen:
track(tracked, n[i]) track(tracked, n[i])
checkForSink(tracked.config, tracked.c.idgen, tracked.owner, n[i]) checkForSink(tracked, n[i])
if tracked.owner.kind != skMacro: if tracked.owner.kind != skMacro:
createTypeBoundOps(tracked, n.typ, n.info) createTypeBoundOps(tracked, n.typ, n.info)
of nkBracketExpr: of nkBracketExpr:

View file

@ -138,11 +138,11 @@ if dirExists(this.value):
par = (dir_1: parentDir(this.value), front_1: par = (dir_1: parentDir(this.value), front_1:
wasMoved(:tmpD_1) wasMoved(:tmpD_1)
`=copy`(:tmpD_1, `=copy`(:tmpD_1,
:tmpD_3 = splitPath do: :tmpD_3 = splitDrive do:
wasMoved(:tmpD_2) wasMoved(:tmpD_2)
`=copy`(:tmpD_2, this.value) `=copy`(:tmpD_2, this.value)
:tmpD_2 :tmpD_2
:tmpD_3.tail) :tmpD_3.path)
:tmpD_1) :tmpD_1)
`=destroy`(:tmpD_3) `=destroy`(:tmpD_3)
if dirExists(par.dir): if dirExists(par.dir):
@ -357,7 +357,7 @@ proc getSubDirs(parent, front: string): seq[string] = @[]
method check(this: Foo) {.base.} = method check(this: Foo) {.base.} =
this.isValid = fileExists(this.value) this.isValid = fileExists(this.value)
let par = if dirExists(this.value): (dir: this.value, front: "") let par = if dirExists(this.value): (dir: this.value, front: "")
else: (dir: parentDir(this.value), front: splitPath(this.value).tail) else: (dir: parentDir(this.value), front: splitDrive(this.value).path)
if dirExists(par.dir): if dirExists(par.dir):
this.matchDirs = getSubDirs(par.dir, par.front) this.matchDirs = getSubDirs(par.dir, par.front)
else: else: