diff --git a/compiler/ast2nif.nim b/compiler/ast2nif.nim index 0504d18aca..b1032e66d6 100644 --- a/compiler/ast2nif.nim +++ b/compiler/ast2nif.nim @@ -496,8 +496,19 @@ proc writeTypeDef(w: var Writer; dest: var IcBuilder; typ: PType) = # name in isolation (cg seeks the `.t.nif`/`.s.nif` index entry), so its # fields must be DEFS here, not entry-deduped SymUses whose def lives # elsewhere in the `(lowered)` entry and is never read by the seek. + # + # `emittedFieldSyms` only guards against a field being def'd twice WITHIN one + # reclist, so scope it per-reclist: a generic object and its instances SHARE one + # field PSym (same itemId) yet each instance carries a DISTINCT field type (e.g. + # `MDigest[256].data: array[32,byte]` vs `MDigest[384].data: array[48,byte]`), so + # each reclist needs its OWN typed def. A Writer-global set deduped every instance + # after the first to a typeless `SymUse` stub (nil typ/owner on load → crash in + # destructor lifting). Field NIF names are local (no module suffix, not in the + # global `c.syms`), so def'ing the same field in two reclists never collides. inc w.inTypeReclist + let savedFieldSyms = move w.emittedFieldSyms writeNode(w, dest, typ.nImpl) + w.emittedFieldSyms = savedFieldSyms dec w.inTypeReclist writeSym(w, dest, typ.ownerFieldImpl) writeSym(w, dest, typ.symImpl) diff --git a/compiler/options.nim b/compiler/options.nim index 14521d8a59..fc239beb63 100644 --- a/compiler/options.nim +++ b/compiler/options.nim @@ -29,7 +29,7 @@ const nimEnableCovariance* = defined(nimEnableCovariance) - icFormatVersion* = "25" + icFormatVersion* = "26" ## Version of the IC cache format (the sem-NIF module layout written by ## ast2nif.nim plus the iface/impl/edges side files). Bump it whenever ## that layout changes: `commandIc` wipes a nimcache whose `ic.version` diff --git a/compiler/vmgen.nim b/compiler/vmgen.nim index 999b0756de..79bbba7d44 100644 --- a/compiler/vmgen.nim +++ b/compiler/vmgen.nim @@ -1921,7 +1921,11 @@ proc genCheckedObjAccessAux(c: PCtx; n: PNode; dest: var TDest; flags: TGenFlags let strType = getSysType(c.graph, n.info, tyString) var msgReg: TDest = c.getTemp(strType) let fieldName = $accessExpr[1] - let msg = genFieldDefect(c.config, fieldName, disc.sym) + # Re-navigate the discriminant in the object type: under `nim ic` `disc.sym` is a + # field-use stub with a nil `owner`, which `genFieldDefect` dereferences. Look up the + # canonical discriminant field by name. Byte-neutral for non-IC (returns the same sym). + let dfield = lookupFieldAgain(accessExpr[0].typ, disc.sym) + let msg = genFieldDefect(c.config, fieldName, dfield) let strLit = newStrNode(msg, accessExpr[1].info) strLit.typ = strType c.genLit(strLit, msgReg) diff --git a/tests/ic/minitordera.nim b/tests/ic/minitordera.nim new file mode 100644 index 0000000000..a35ac0eed7 --- /dev/null +++ b/tests/ic/minitordera.nim @@ -0,0 +1,29 @@ +# Helper for tinitorder (not a test itself; no `discard`). +# +# Imports minitorderb and, in its OWN init, reads the state minitorderb set up. +# With the wrong (importer-first) init order `gBState` is still 0 here. It also +# allocates a seq in its init so that under `--mm:refc` the GC (set up by the +# system module's init) must already be live — i.e. the system module's init has +# to be ordered first. + +import minitorderb + +var + gASawB = -1 + gAItems: seq[int] + +proc getASawB*(): int = gASawB +proc getACount*(): int = gAItems.len + +proc recordA() = + gASawB = getBState() + gAItems = @[1, 2, 3] + # Allocate (and drop) enough garbage to force a GC cycle DURING module init. + # Under refc that runs a conservative stack scan, which needs the main + # thread's stack bottom already set — i.e. `initStackBottomWith` must run + # before the module inits, not after them. + for i in 0 ..< 100_000: + let s = @[i, i + 1, i + 2] + doAssert s.len == 3 + +recordA() diff --git a/tests/ic/minitorderb.nim b/tests/ic/minitorderb.nim new file mode 100644 index 0000000000..8c406e8e94 --- /dev/null +++ b/tests/ic/minitorderb.nim @@ -0,0 +1,15 @@ +# Helper for tinitorder (not a test itself; no `discard`). +# +# An imported module whose INIT sets module-level state at runtime. Under the +# per-module backend its init must run BEFORE any importer's init (the imported +# module is a dependency → post-order). With the buggy importer-first order this +# module's `setupB` runs too late and importers observe `gBState == 0`. + +var gBState: int + +proc getBState*(): int = gBState + +proc setupB() = + gBState = 42 + +setupB() diff --git a/tests/ic/mmethanimal.nim b/tests/ic/mmethanimal.nim new file mode 100644 index 0000000000..76e0bcaf68 --- /dev/null +++ b/tests/ic/mmethanimal.nim @@ -0,0 +1,15 @@ +# Helper for tmethitanium: the OWNER module of a `{.base.}` method. Its concrete +# base body is emitted here; the whole-program dispatcher is synthesized into the +# main module. Under `--debugger:native` the backend uses the Itanium mangling +# scheme, which encodes the signature instead of the `disamb`, so the base method +# and its same-signature dispatcher want the identical clean C name. The +# clean-vs-unique tie-break used to depend on a per-MODULE set (`mangledPrcs`), +# which the per-module IC backend cannot share — the base mangled clean at this +# owner but `speak_u` (an unstable `itemId.item`) at every demander, so it was +# defined once and referenced under names nobody defined. See +# ccgutils.makeUnique (disamb, not itemId) + ccgtypes.fillBackendName. + +type Animal* = ref object of RootObj + +method speak*(a: Animal): string {.base.} = + "generic-animal-sound" diff --git a/tests/ic/mmethdog.nim b/tests/ic/mmethdog.nim new file mode 100644 index 0000000000..22d0de7130 --- /dev/null +++ b/tests/ic/mmethdog.nim @@ -0,0 +1,14 @@ +# Helper for tmethitanium: an override in a DIFFERENT module than the base, plus +# a `procCall` super-reference to the base from this non-owner module. That +# cross-module reference to the base impl is what diverged from the base's +# definition name under the per-module Itanium mangling. + +import mmethanimal + +type Dog* = ref object of Animal + +method speak*(a: Dog): string = + "woof" + +proc speakBoth*(a: Dog): string = + procCall(speak(Animal(a))) & "/" & speak(a) diff --git a/tests/ic/tinitorder.nim b/tests/ic/tinitorder.nim new file mode 100644 index 0000000000..93d0d94f4c --- /dev/null +++ b/tests/ic/tinitorder.nim @@ -0,0 +1,23 @@ +discard """ +output: '''42 3''' +""" + +# Regression test for per-module-backend module-init ORDERING. +# +# NimMain must call each module's init in DEPENDENCY (post-order) order: an +# imported module's init has to run before its importer's. The whole-program +# backend gets this for free (it iterates `modulesClosed`, built in module-finish +# order); the per-module backend reconstructs it in `nifbackend`. The earlier +# code iterated `bl.mods` by POSITION, which runs importers before their +# dependencies (an importer gets a lower file position than the modules it +# imports) — and the system module (which runs `initGC` in its init) was not +# ordered first at all. +# +# Module chain: tinitorder -> minitordera -> minitorderb. `minitorderb`'s init +# sets a global to 42; `minitordera`'s init reads it (and allocates a seq). With +# the buggy order `minitordera` runs first and reads 0 (or, under refc, crashes +# allocating before the GC is up). Correct order prints `42 3`. + +import minitordera + +echo getASawB(), " ", getACount() diff --git a/tests/ic/tmethitanium.nim b/tests/ic/tmethitanium.nim new file mode 100644 index 0000000000..f226af37c1 --- /dev/null +++ b/tests/ic/tmethitanium.nim @@ -0,0 +1,33 @@ +discard """ + output: '''woof +generic-animal-sound +generic-animal-sound/woof''' +""" + +# NOTE: the `--debugger:native` that triggers the Itanium mangling lives in the +# sibling `tmethitanium_temp.nim.cfg` (the IC test harness compiles the generated +# `_temp.nim` and does not thread a `matrix`/`$options` switch into the cg +# children; a project cfg is read by the driver, which forwards it). + +# Regression test: under `nim ic --debugger:native` the per-module backend uses +# the Itanium C name mangling, which encodes the signature and drops the +# `disamb`. A `{.base.}` method (owner module mmethanimal) and its whole-program +# dispatcher (synthesized into this main module) then share a signature, so the +# clean-name uniqueness probe (`m.g.mangledPrcs`) — which only sees the current +# module — gave the base a clean name at its owner but an unstable +# `itemId.item`-based `speak_u` at each demander. Result: the base was defined +# once (clean) but referenced under names defined nowhere ("undefined reference +# to speak_u1") while the dispatcher collided with the clean base ("multiple +# definition of speak"). This was the bulk of nimbus-eth2's libp2p method link +# failures under `nim ic`. Fixed by making the Itanium scheme use the stable +# `disamb` (ccgutils.makeUnique) and always uniquify routine names under the +# per-module backend (ccgtypes.fillBackendName), plus forwarding +# `--debugger:native` to the cg children (deps.computeForwardedArgs). + +import mmethanimal, mmethdog + +let a: Animal = Dog() +echo speak(a) # dispatches -> override +let b: Animal = Animal() +echo speak(b) # dispatches -> base body +echo speakBoth(Dog()) # procCall to base from non-owner module diff --git a/tests/ic/tstaticgenfield.nim b/tests/ic/tstaticgenfield.nim new file mode 100644 index 0000000000..690245529e --- /dev/null +++ b/tests/ic/tstaticgenfield.nim @@ -0,0 +1,38 @@ +discard """ + output: '''9''' +""" + +# Regression test for object-field serialization of static-generic instances +# under `nim ic`. +# +# A generic object's instances SHARE one field PSym (same itemId) while each +# instance carries a DISTINCT field type, e.g. `Digest[32].data: array[32,byte]` +# vs `Digest[48].data: array[48,byte]` (this is exactly nimcrypto's `MDigest`, +# which crashed compiling nimbus's `altair.nim`). The `.s.bif` writer DEFs each +# field once inside its owning type's reclist and references it as a bare SymUse +# elsewhere, deduping by a per-Writer `emittedFieldSyms` set. That set wrongly +# spanned DIFFERENT type reclists: after the first instance's `data` def, every +# other instance's reclist got a typeless SymUse stub instead of its own typed +# def. On load that field had a nil `typ`/`owner`, and `=destroy` lifting +# (`liftdestructors.fillBodyObj`) dereferenced it -> SIGSEGV. The fix scopes the +# dedup per-reclist so each instance reclist is a self-contained typed def. +# +# The `seq` field forces `=destroy` to be lifted for `Outer`, which walks the +# reclists of both `Digest` instances (the crash path). + +type + Digest[n: static int] = object + data: array[n, byte] + Outer = object + a: Digest[32] + b: Digest[48] + s: seq[int] + +proc use(o: Outer): int = + result = o.a.data[0].int + o.b.data[0].int + o.s.len + +var o: Outer +o.a.data[0] = 4 +o.b.data[0] = 2 +o.s = @[1, 2, 3] +echo use(o)