diff --git a/compiler/lookups.nim b/compiler/lookups.nim index acaad9d9b4..5bdc560cc3 100644 --- a/compiler/lookups.nim +++ b/compiler/lookups.nim @@ -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 diff --git a/compiler/options.nim b/compiler/options.nim index 95d10717ae..2e7de72656 100644 --- a/compiler/options.nim +++ b/compiler/options.nim @@ -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 diff --git a/compiler/sem.nim b/compiler/sem.nim index 38da68a0b0..972a84027f 100644 --- a/compiler/sem.nim +++ b/compiler/sem.nim @@ -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: diff --git a/compiler/semdata.nim b/compiler/semdata.nim index d25c1a46b3..376821e7e9 100644 --- a/compiler/semdata.nim +++ b/compiler/semdata.nim @@ -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(), diff --git a/compiler/sigmatch.nim b/compiler/sigmatch.nim index 65326b7e4e..a94ad12722 100644 --- a/compiler/sigmatch.nim +++ b/compiler/sigmatch.nim @@ -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 diff --git a/tests/template/toverload_over_untyped.nim b/tests/template/toverload_over_untyped.nim index 0f734480d6..ff76663915 100644 --- a/tests/template/toverload_over_untyped.nim +++ b/tests/template/toverload_over_untyped.nim @@ -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"