fixes #25693; continues the bugfix story (#25876)

(cherry picked from commit 7a5e35c83e)
This commit is contained in:
Andreas Rumpf
2026-06-08 22:54:03 +02:00
committed by narimiran
parent 090cfee525
commit 8c02426855
6 changed files with 124 additions and 22 deletions

View File

@@ -463,6 +463,15 @@ proc openShadowScope*(c: PContext) =
symbols: initStrTable(),
depthLevel: c.scopeDepth)
proc rememberShadowDefs*(c: PContext) =
## bug #25693: a template/macro operand's local definitions are sem-checked in
## a shadow scope that is then discarded. Record those definitions so that a
## later re-emission (e.g. a captured `typed` fragment expanded more than once)
## can be detected as a redefinition rather than silently miscompiled.
for s in c.currentScope.symbols:
if s.kind in {skVar, skLet, skForVar} and {sfGenSym, sfWasGenSym} * s.flags == {}:
c.shadowDiscardedDefs.incl s.id
proc closeShadowScope*(c: PContext) =
## closes the shadow scope, but doesn't merge any of the symbols
## Does not check for unused symbols or missing forward decls since a macro

View File

@@ -256,6 +256,11 @@ type
procParamTypeBackendAliases
## Keep the old proc type compatibility rules that ignore backend
## c type aliases.
injectedSymbolRedefinition
## Allow a template to inject a symbol *definition* that is then emitted
## more than once (e.g. a `typed` argument captured by a `{.dirty.}`
## template and re-emitted). This is a redefinition and rejected by
## default; enabling this restores the old, unsound behavior. See #25693.
SymbolFilesOption* = enum
disabledSf, writeOnlySf, readOnlySf, v2Sf, stressTest

View File

@@ -247,6 +247,26 @@ proc newSymG*(kind: TSymKind, n: PNode, c: PContext): PSym =
if result.kind notin {kind, skTemp}:
localError(c.config, n.info, "cannot use symbol of kind '$1' as a '$2'" %
[result.kind.toHumanStr, kind.toHumanStr])
# bug #25693: a local declared inside a template/macro operand (recorded in
# `shadowDiscardedDefs`) can be captured by a `{.dirty.}` template and
# re-emitted as a definition more than once. The first emission keeps the
# original symbol (so a leaked dirty-template name still resolves); every
# later emission gets a fresh copy, so distinct emissions don't share one
# symbol - which the destructor/liveness analysis would otherwise miscompile.
# Unlike a plain redefinition check this is control-flow agnostic, so the
# common "emit a `typed` body in several mutually-exclusive branches" pattern
# keeps working. gensym'ed locals (and ones derived from a gensym name) are
# excluded: the gensym machinery already keeps their names unique, and a
# fresh copy would reuse the unique name and clash in the same scope.
if kind in {skVar, skLet, skForVar} and
{sfGenSym, sfWasGenSym} * result.flags == {} and
result.id in c.shadowDiscardedDefs:
if containsOrIncl(c.realizedDefs, result.id):
let fresh = copySym(result, c.idgen)
fresh.ast = result.ast
put(c.p, result, fresh)
c.hasSymRedefs = true
result = fresh
when false:
if sfGenSym in result.flags and result.kind notin {skTemplate, skMacro, skParam}:
# declarative context, so produce a fresh gensym:

View File

@@ -186,6 +186,18 @@ type
inTypeofContext*: int
semAsgnOpr*: proc (c: PContext; n: PNode; k: TNodeKind): PNode {.nimcall.}
shadowDiscardedDefs*: IntSet
# ids of local symbols that were declared inside a template/macro operand's
# shadow scope and then discarded; re-emitting such a symbol as a
# definition gives a fresh copy so distinct emissions don't share a symbol.
# See bug #25693 and `rememberShadowDefs`.
realizedDefs*: IntSet
# ids from `shadowDiscardedDefs` already realized once; the first emission
# keeps the original symbol (so leaked dirty-template names still resolve),
# later emissions get a fresh copy.
hasSymRedefs*: bool
# set once a redefinition mapping has been installed; makes `getGenSym`
# consult the proc-con mapping for non-gensym symbols too.
TBorrowState* = enum
bsNone, bsReturnNotMatch, bsNoDistinct, bsGeneric, bsNotSupported, bsMatch
@@ -278,7 +290,10 @@ proc get*(p: PProcCon; key: PSym): PSym =
result = p.mapping.getOrDefault(key.itemId)
proc getGenSym*(c: PContext; s: PSym): PSym =
if sfGenSym notin s.flags: return s
# `c.hasSymRedefs` additionally routes ordinary (non-gensym) symbols through
# the mapping so a re-emitted definition can redirect them to its fresh copy,
# see bug #25693 and `newSymG`.
if sfGenSym notin s.flags and not c.hasSymRedefs: return s
var it = c.p
while it != nil:
result = get(it, s)
@@ -340,6 +355,8 @@ proc newContext*(graph: ModuleGraph; module: PSym): PContext =
userPragmas: initStrTable(),
generics: @[],
unknownIdents: initIntSet(),
shadowDiscardedDefs: initIntSet(),
realizedDefs: initIntSet(),
cache: graph.cache,
graph: graph,
signatures: initStrTable(),

View File

@@ -2871,6 +2871,7 @@ proc matchesAux(c: PContext, n, nOrig: PNode, m: var TCandidate, marker: var Int
if m.calleeSym != nil and m.calleeSym.kind notin {skTemplate, skMacro}:
c.mergeShadowScope
else:
c.rememberShadowDefs
c.closeShadowScope
m.state = csNoMatch
m.firstMismatch.arg = a
@@ -2927,7 +2928,10 @@ proc matchesAux(c: PContext, n, nOrig: PNode, m: var TCandidate, marker: var Int
setSon(m.call, formal.position + 1, container)
else:
incrIndexType(container.typ)
container.add n[a]
# bug #25693: like the scalar `tyUntyped` case in `paramTypesMatchAux`,
# a previous overload candidate may have sem-checked the operand in
# place; templates/macros expect the pristine AST, so use `nOrig`.
container.add nOrig[a]
elif n[a].kind == nkExprEqExpr:
# named param
m.firstMismatch.kind = kUnknownNamedParam
@@ -3026,7 +3030,8 @@ proc matchesAux(c: PContext, n, nOrig: PNode, m: var TCandidate, marker: var Int
setSon(m.call, formal.position + 1, container)
else:
incrIndexType(container.typ)
container.add n[a]
# bug #25693: see the leading isVarargsUntyped branch above.
container.add nOrig[a]
else:
m.baseTypeMatch = false
m.typedescMatched = false
@@ -3078,6 +3083,7 @@ proc matchesAux(c: PContext, n, nOrig: PNode, m: var TCandidate, marker: var Int
if m.state == csMatch and not (m.calleeSym != nil and m.calleeSym.kind in {skTemplate, skMacro}):
c.mergeShadowScope
else:
c.rememberShadowDefs
c.closeShadowScope
inc a

View File

@@ -1,32 +1,77 @@
discard """
output: "ok"
output: '''ok
ok
ok'''
"""
# bug #25693
template g(b: untyped) {.dirty.} =
template t: untyped = b
proc d() = discard @[0]
proc g(_: int) = discard
proc f(a: var seq[int], _: string) =
let p = @[0]
d()
a = p
let q = "a"
g:
var a: seq[int]
try:
f(a, q & "1")
except CatchableError:
discard
try:
f(a, q & "1")
except CatchableError:
discard
block: t()
block: t()
echo "ok"
block: # scalar `untyped` parameter
template g(b: untyped) {.dirty.} =
template t: untyped = b
proc g(_: int) = discard
let q = "a"
g:
var a: seq[int]
try:
f(a, q & "1")
except CatchableError:
discard
try:
f(a, q & "1")
except CatchableError:
discard
block: t()
block: t()
echo "ok"
block: # `typed` parameter captured and re-emitted: each emission gets its own
# symbols, otherwise the destructor/liveness pass miscompiles the shared
# local `a` and the program crashes at runtime
template g(b: typed) {.dirty.} =
template t: untyped = b
let q = "a"
g:
var a: seq[int]
try:
f(a, q & "1")
except CatchableError:
discard
try:
f(a, q & "1")
except CatchableError:
discard
block: t()
block: t()
echo "ok"
block: # `varargs[untyped]` parameter takes the same pristine-AST path
template g(b: varargs[untyped]) {.dirty.} =
template t: untyped = b
proc g(_: int) = discard
let q = "a"
g:
var a: seq[int]
try:
f(a, q & "1")
except CatchableError:
discard
try:
f(a, q & "1")
except CatchableError:
discard
block: t()
block: t()
echo "ok"