From 79b86f642f311f0f3d8e941ca321dc46f54dc564 Mon Sep 17 00:00:00 2001 From: Araq Date: Wed, 24 Jun 2026 14:17:10 +0200 Subject: [PATCH] IC: more bugfixes --- compiler/ccgtypes.nim | 28 +++++++++++++++++++++++++--- compiler/nifbackend.nim | 12 +++++++++++- compiler/semtypinst.nim | 20 +++++++++++++------- compiler/sighashes.nim | 15 ++++++++++++++- compiler/vm.nim | 37 +++++++++++++++++++++++++++++++++++-- 5 files changed, 98 insertions(+), 14 deletions(-) diff --git a/compiler/ccgtypes.nim b/compiler/ccgtypes.nim index eb82c854c7..3fae86f5cf 100644 --- a/compiler/ccgtypes.nim +++ b/compiler/ccgtypes.nim @@ -1408,10 +1408,24 @@ proc genTypeInfoAuxBase(m: BModule; typ, origType: PType; m.hcrCreateTypeInfosProc.addCast(typ = ptrType(CPointer)): m.hcrCreateTypeInfosProc.add(cAddr(name)) else: - m.s[cfsStrData].addDeclWithVisibility(Private): - m.s[cfsStrData].addVar(kind = Local, name = name, typ = "TNimType") if m.config.cmd == cmdNifC: + # Emit-everywhere (see genTypeInfoV1's perModuleCg gate): every demanding + # `cg` process emits this type info's tentative definition. Declare it + # `extern` first (the data analogue of a proc prototype) so a TU whose copy + # the merge stage drops still has a valid declaration; wrap the definition + # as a droppable `'d'` unit the merge stage assigns to a single owner so + # exactly one external-linkage tentative definition survives (preserving + # the RTTI pointer identity refc relies on). + m.s[cfsStrData].addDeclWithVisibility(Extern): + m.s[cfsStrData].addVar(kind = Local, name = name, typ = "TNimType") + m.s[cfsStrData].add(cnifDefDirective(name, "d", icNifName(m, origType))) + m.s[cfsStrData].addDeclWithVisibility(Private): + m.s[cfsStrData].addVar(kind = Local, name = name, typ = "TNimType") + m.s[cfsStrData].add(cnifEndDefs()) m.icDataDefs.add (name, icNifName(m, origType)) + else: + m.s[cfsStrData].addDeclWithVisibility(Private): + m.s[cfsStrData].addVar(kind = Local, name = name, typ = "TNimType") proc genTypeInfoAux(m: BModule; typ, origType: PType, name: Rope; info: TLineInfo) = @@ -2124,7 +2138,15 @@ proc genTypeInfoV1(m: BModule; t: PType; info: TLineInfo): Rope = return prefixTI(result) var owner = t.skipTypes(typedescPtrs).itemId.module - if owner != m.module.position and myModuleOpenForCodegen(m, FileIndex owner): + # In the per-module backend (`cg`) V1 RTTI is emit-everywhere like procs, + # consts and V2 type info: every demanding module emits the `'d'` definition + # (deduped to one owner by the merge stage). The owner-routing below would + # instead push the definition into the owner module's *unwritten* backend + # module (discarded in this process) and emit only an extern here, leaving the + # symbol undefined at link — the refc `NTI*` undefined-reference bug. (V2 got + # this gate in 8e0dd4bfb; V1, only reached under `--mm:refc`, was missed.) + let perModuleCg = m.config.cmd == cmdNifC and m.config.icBackendStage == "cg" + if not perModuleCg and owner != m.module.position and myModuleOpenForCodegen(m, FileIndex owner): dbgNti "extern:ownerRouted" # make sure the type info is created in the owner module discard genTypeInfoV1(m.g.mods[owner], origType, info) diff --git a/compiler/nifbackend.nim b/compiler/nifbackend.nim index f3bcec2114..04a8bf2f65 100644 --- a/compiler/nifbackend.nim +++ b/compiler/nifbackend.nim @@ -140,7 +140,17 @@ proc signatureHasMetaType(t: PType; depth: int = 0): bool = # as meta and drop it from the owned-routine seeding -> undefined symbols # at link (its only definer never emits it). return false - if t.kind in {tyTyped, tyUntyped, tyTypeDesc, tyStatic, tyGenericParam, + if t.kind == tyStatic: + # A RESOLVED static value (the `256` in `MDigest[256]`, the `N` in + # `HashList[T, N]`, …) is carried as a `tyStatic` node inside the otherwise + # fully-concrete `tyGenericInst`, but it is NOT meta: the routine is a normal + # runtime routine the owner must emit. Only an UNRESOLVED `static T` parameter + # (no bound value, `t.n == nil`) is meta. Without this, every routine whose + # signature touches a `static`-parameterized generic instance (the bulk of + # the SSZ/`MDigest` API) is dropped from the owned-routine seeding and ends up + # an undefined reference at link (mirrors the tyGenericBody case above). + return t.n == nil + if t.kind in {tyTyped, tyUntyped, tyTypeDesc, tyGenericParam, tyAnything, tyFromExpr, tyError}: return true for k in t.kids: diff --git a/compiler/semtypinst.nim b/compiler/semtypinst.nim index 1823db0e2d..9a230faf83 100644 --- a/compiler/semtypinst.nim +++ b/compiler/semtypinst.nim @@ -838,15 +838,21 @@ proc replaceTypeVarsTAux(cl: var TReplTypeVars, t: PType, isInstValue = false): # trough replaceObjBranches in order to resolve any pending nkRecWhen nodes result = t - # Slow path, we have some work to do - if t.kind == tyRef and t.hasElementType and t.elementType.kind == tyObject and t.elementType.n != nil: + # Slow path, we have some work to do. CRUCIAL: only ever mutate a type that + # is LOCAL to the module we are instantiating in (`uniqueId.module == + # idgen.module`). A type loaded from another module's NIF (foreign) already + # had its object branches resolved when it was originally compiled; mutating + # it in place here is an old→new heap write that re-homes the loaded type to + # the instantiation site (its sym then looks owned by the consumer module and + # loses its `info`, colliding C type names — the libp2p `Message` bug). The + # prior `state != Sealed` guard was insufficient: a freshly-LOADED type is + # `Complete`, not `Sealed` (`Sealed` only means "already re-written to a NIF"). + if t.kind == tyRef and t.hasElementType and t.elementType.kind == tyObject and + t.elementType.n != nil and t.elementType.uniqueId.module == cl.c.idgen.module.int: discard replaceObjBranches(cl, t.elementType.n) - elif result.n != nil and t.kind == tyObject and result.state != Sealed: - # A type loaded from the IC cache already had its object branches - # resolved when it was originally compiled, and must not be mutated in - # place (nor copied, which would break object-inheritance identity), so - # only non-Sealed types are processed here. + elif result.n != nil and t.kind == tyObject and result.state != Sealed and + result.uniqueId.module == cl.c.idgen.module.int: # Invalidate the type size as we may alter its structure result.size = -1 result.n = replaceObjBranches(cl, result.n) diff --git a/compiler/sighashes.nim b/compiler/sighashes.nim index d6db0b0fa0..963bb404a4 100644 --- a/compiler/sighashes.nim +++ b/compiler/sighashes.nim @@ -10,6 +10,7 @@ ## Computes hash values for routine (proc, method etc) signatures. import ast, ropes, modulegraphs, options, msgs, pathutils +from lineinfos import FileIndex from std/hashes import Hash import std/tables import types @@ -74,7 +75,19 @@ proc hashTypeSym(c: var MD5Context, s: PSym; conf: ConfigRef) = c &= ":anon" else: var it = s - c &= customPath(conf.toFullPath(s.info)) + # The source file path disambiguates same-named object types from different + # modules whose owner-chain names also coincide (e.g. libp2p kademlia/protobuf + # `Message` vs rendezvous/protobuf `Message`, both modules named `protobuf`). + # A type sym that reaches the backend as a `Complete` stub never individually + # loaded carries `unknownLineInfo` (fileIndex -1), which `toFullPath` collapses + # to the `???` placeholder — so the two would hash to ONE mangled C name and the + # wrong struct gets emitted. Fall back to the sym's HOME module file (its + # per-module NIF-suffix path, stable+unique) for the path. Only fires on a -1 + # fileIndex; non-IC type syms always have a real `info`, so the fast path is + # taken and the hash is unchanged (koch boot byte-equal). + let infoFi = s.info.fileIndex + let pathFi = if infoFi.int32 >= 0'i32: infoFi else: s.itemId.module.int32.FileIndex + c &= customPath(conf.toFullPath(pathFi)) when defined(icDbgHash): var ownerSteps = 0 while it != nil: diff --git a/compiler/vm.nim b/compiler/vm.nim index 851ba1d9db..23c579dd17 100644 --- a/compiler/vm.nim +++ b/compiler/vm.nim @@ -1313,8 +1313,41 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg = # a macro observed this symbol's implementation: NeedsImpl edge to # its home module under IC. recordIcImplDep(c.graph, a.sym) - regs[ra].node = if a.sym.ast.isNil: newNode(nkNilLit) - else: copyTree(a.sym.ast) + if a.sym.ast.isNil: + regs[ra].node = newNode(nkNilLit) + else: + let tree = copyTree(a.sym.ast) + # A NIF-loaded routine's `ast[paramsPos]` is an `nkEmpty` placeholder: + # ast2nif strips the formal params (recoverable from `typ.n`, see + # writeNode's `skipParams`). A macro that reads `fn.getImpl[paramsPos]` + # — e.g. taskpools `spawn` reads the return type via `getImpl[3][0]` — + # needs them, so reconstruct a read-only formalParams from the proc + # type. The synthesized type-expression nodes carry the resolved + # `PType`, which is all a macro can query for a loaded routine. + if tree.kind in {nkProcDef, nkFuncDef, nkMethodDef, nkIteratorDef, + nkConverterDef, nkMacroDef, nkTemplateDef, nkLambda, nkDo} and + tree.safeLen > paramsPos and tree[paramsPos].kind == nkEmpty and + a.sym.typ != nil and a.sym.typ.n != nil and + a.sym.typ.n.kind == nkFormalParams: + let t = a.sym.typ + let fp = newNodeI(nkFormalParams, a.sym.info) + let rt = t.returnType + # `opMapTypeInstToAst` (inst=true) reproduces a source-like type + # declaration — crucially it renders an array's range bound as + # `range 0..N` (the `inst=false` form emits `range[0, N]`, which + # re-sems to "'range' expects one type parameter"). + fp.add(if rt != nil: opMapTypeInstToAst(c.cache, rt, a.sym.info, c.idgen) + else: newNodeI(nkEmpty, a.sym.info)) + for i in 1 ..< t.n.len: + if t.n[i].kind == nkSym: + let p = t.n[i].sym + let def = newNodeI(nkIdentDefs, p.info) + def.add newIdentNode(p.name, p.info) + def.add opMapTypeInstToAst(c.cache, p.typ, p.info, c.idgen) + def.add newNodeI(nkEmpty, p.info) + fp.add def + tree[paramsPos] = fp + regs[ra].node = tree regs[ra].node.flags.incl nfIsRef else: stackTrace(c, tos, pc, "node is not a symbol")