From 88e7adfcb78cd547668bde5485707db460e6587c Mon Sep 17 00:00:00 2001 From: Tomohiro Date: Sun, 1 Feb 2026 15:01:55 +0900 Subject: [PATCH 1/2] fixes #25459; `hashType` returns different hash from instantiated generics with distinct types (#25471) `hashType` proc returned the same hash value from different instanced generics types like `D[int64]` and `D[F]`. That caused the struct type with wrong field types. object/tuple type size check code is generated when it is compiled with `-d:checkAbi` option. --- compiler/ccgtypes.nim | 8 +++++++- compiler/sighashes.nim | 2 +- compiler/types.nim | 2 +- tests/ccgbugs2/m25459/g.nim | 11 +++++++++++ tests/ccgbugs2/m25459/h.nim | 8 ++++++++ tests/ccgbugs2/t25459.nim | 10 ++++++++++ tests/ccgbugs2/t25459b.nim | 31 +++++++++++++++++++++++++++++++ 7 files changed, 69 insertions(+), 3 deletions(-) create mode 100644 tests/ccgbugs2/m25459/g.nim create mode 100644 tests/ccgbugs2/m25459/h.nim create mode 100644 tests/ccgbugs2/t25459.nim create mode 100644 tests/ccgbugs2/t25459b.nim diff --git a/compiler/ccgtypes.nim b/compiler/ccgtypes.nim index 2e619c8065..11fe701c18 100644 --- a/compiler/ccgtypes.nim +++ b/compiler/ccgtypes.nim @@ -294,7 +294,12 @@ proc cacheGetType(tab: TypeCache; sig: SigHash): Rope = result = tab.getOrDefault(sig) proc addAbiCheck(m: BModule; t: PType, name: Rope) = - if isDefined(m.config, "checkAbi") and (let size = getSize(m.config, t); size != szUnknownSize): + if isDefined(m.config, "checkAbi") and (let size = getSize(m.config, t); size != szUnknownSize) and + not (t.kind == tyObject and searchTypeFor(t, proc (t: PType): bool {.nimcall.} = t.kind == tyUncheckedArray)): + # `UncheckedArray`, not `ptr UncheckedArray` type field in object types is a flexible array. + # `sizeof` in C and Nim doesn't always return the same value for object types containing it. + # making `getSize` in Nim always returns the same value as `sizeof` in C from flexible arrays seems hard. + # See `SEQ_DECL_SIZE` in lib/nimbase.h var msg = "backend & Nim disagree on size for: " msg.addTypeHeader(m.config, t) var msg2 = "" @@ -1067,6 +1072,7 @@ proc getTypeDescAux(m: BModule; origTyp: PType, check: var IntSet; kind: TypeDes else: getTupleDesc(m, t, result, check) if not isImportedType(t): m.s[cfsTypes].add(recdesc) + addAbiCheck(m, t, result) elif tfIncompleteStruct notin t.flags: discard # addAbiCheck(m, t, result) # already handled elsewhere of tySet: diff --git a/compiler/sighashes.nim b/compiler/sighashes.nim index 5d6d0e9a5b..a4f1e00880 100644 --- a/compiler/sighashes.nim +++ b/compiler/sighashes.nim @@ -154,7 +154,7 @@ proc hashType(c: var MD5Context, t: PType; flags: set[ConsiderFlag]; conf: Confi assert inst.kind == tyGenericInst c.hashType inst.genericHead, flags, conf for _, a in inst.genericInstParams: - c.hashType a, flags, conf + c.hashType a, flags+{CoDistinct}, conf t.typeInstImpl = inst return c &= char(t.kind) diff --git a/compiler/types.nim b/compiler/types.nim index 08c0c92dab..e18f97ff36 100644 --- a/compiler/types.nim +++ b/compiler/types.nim @@ -143,7 +143,7 @@ proc getFloatValue*(n: PNode): BiggestFloat = proc addTypeHeader*(result: var string, conf: ConfigRef; typ: PType; prefer: TPreferedDesc = preferMixed; getDeclarationPath = true) = result.add typeToString(typ, prefer) - if getDeclarationPath: result.addDeclaredLoc(conf, typ.sym) + if getDeclarationPath and typ.sym != nil: result.addDeclaredLoc(conf, typ.sym) proc getProcHeader*(conf: ConfigRef; sym: PSym; prefer: TPreferedDesc = preferName; getDeclarationPath = true): string = assert sym != nil diff --git a/tests/ccgbugs2/m25459/g.nim b/tests/ccgbugs2/m25459/g.nim new file mode 100644 index 0000000000..b97d4c73a4 --- /dev/null +++ b/tests/ccgbugs2/m25459/g.nim @@ -0,0 +1,11 @@ +proc v[T](_: typedesc[T]): int = + if T is int64: 6 else: 4 + +type + D*[T] = object + c*: seq[T] + k*: array[v(T), int] + F = distinct int64 + W* = object + y: D[F] + j*: D[int64] diff --git a/tests/ccgbugs2/m25459/h.nim b/tests/ccgbugs2/m25459/h.nim new file mode 100644 index 0000000000..45cf527f00 --- /dev/null +++ b/tests/ccgbugs2/m25459/h.nim @@ -0,0 +1,8 @@ +import ./g +export g + +proc a*(): W = + var e = D[int64]() + e.c.setLen(8) + e.k[1] = 0 + result = W(j: e) diff --git a/tests/ccgbugs2/t25459.nim b/tests/ccgbugs2/t25459.nim new file mode 100644 index 0000000000..a31c3c836b --- /dev/null +++ b/tests/ccgbugs2/t25459.nim @@ -0,0 +1,10 @@ +discard """ + targets: "c cpp" + matrix: "-d:checkAbi" +""" + +import ./m25459/h + +for _ in 0 ..< 500: + let u = new W + u[] = a() diff --git a/tests/ccgbugs2/t25459b.nim b/tests/ccgbugs2/t25459b.nim new file mode 100644 index 0000000000..127b4b3cc3 --- /dev/null +++ b/tests/ccgbugs2/t25459b.nim @@ -0,0 +1,31 @@ +discard """ + targets: "c cpp" + matrix: "-d:checkAbi" +""" + +proc v[T](_: typedesc[T]): int = + if T is int64: 2 else: 1 + +type + D[T] = object + k: array[v(T), int] + E[T] = object + k: array[v(T), int] + F = distinct int64 + W = object + a: D[int64] + b: D[F] + +proc csizeof[T](x {.bycopy.} : T): cint {.importc: "sizeof", nodecl.} + +var w: W +assert sizeof(w) == csizeof(w) + +var + e0: E[F] + e1: E[int64] +assert sizeof(e0) == csizeof(e0) +assert sizeof(e1) == csizeof(e1) + +var tup: (E[F], E[int64]) +assert sizeof(tup) == csizeof(tup) From bfc27867187e28dd3b5f2a887450cfc2c465da98 Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Mon, 2 Feb 2026 00:06:33 +0800 Subject: [PATCH 2/2] fixes #24706; Warn on implicit range downsizing (#25451) fixes #24706 --- changelog.md | 2 + compiler/condsyms.nim | 2 + compiler/lineinfos.nim | 4 +- compiler/sempass2.nim | 44 +++++++++++++- compiler/sigmatch.nim | 2 + tests/range/timplicitrangedownsizing.nim | 73 ++++++++++++++++++++++++ 6 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 tests/range/timplicitrangedownsizing.nim diff --git a/changelog.md b/changelog.md index e8c6e77cc4..2a9c8aabf2 100644 --- a/changelog.md +++ b/changelog.md @@ -33,6 +33,8 @@ errors. - Bitshift operators (`shl`, `shr`, `ashr`) now apply bitmasking to the right operand in the C/C++/VM/JS backends. +- Adds a new warning enabled by `--warning:ImplicitRangeConversion` that detects downsizing implicit conversions to range types (e.g., `int -> range[0..255]` or `range[1..256] -> range[0..255]`) that could cause runtime panics. Safe conversions like `range[0..255] -> range[0..65535]` and explicit casts are not warned on. + ## Standard library additions and changes [//]: # "Additions:" diff --git a/compiler/condsyms.nim b/compiler/condsyms.nim index fcd4cf218e..28c3d2f309 100644 --- a/compiler/condsyms.nim +++ b/compiler/condsyms.nim @@ -175,3 +175,5 @@ proc initDefines*(symbols: StringTableRef) = defineSymbol("nimHasSetLengthSeqUninitMagic") defineSymbol("nimHasPreviewDuplicateModuleError") + defineSymbol("nimHasImplicitRangeConversion") + diff --git a/compiler/lineinfos.nim b/compiler/lineinfos.nim index 5bf43592a9..d9d44f277d 100644 --- a/compiler/lineinfos.nim +++ b/compiler/lineinfos.nim @@ -98,6 +98,7 @@ type warnLongLiterals = "LongLiterals", warnUser = "User", warnGlobalVarConstructorTemporary = "GlobalVarConstructorTemporary", + warnImplicitRangeConversion = "ImplicitRangeConversion", # hints hintSuccess = "Success", hintSuccessX = "SuccessX", hintCC = "CC", @@ -206,6 +207,7 @@ const warnLongLiterals: "$1", warnUser: "$1", warnGlobalVarConstructorTemporary: "global variable '$1' initialization requires a temporary variable", + warnImplicitRangeConversion: "implicit range conversion $1", hintSuccess: "operation successful: $#", # keep in sync with `testament.isSuccess` hintSuccessX: "$build\n$loc lines; ${sec}s; $mem; proj: $project; out: $output", @@ -260,7 +262,7 @@ type proc computeNotesVerbosity(): array[0..3, TNoteKinds] = result = default(array[0..3, TNoteKinds]) - result[3] = {low(TNoteKind)..high(TNoteKind)} - {warnObservableStores, warnResultUsed, warnAnyEnumConv, warnBareExcept, warnStdPrefix} + result[3] = {low(TNoteKind)..high(TNoteKind)} - {warnObservableStores, warnResultUsed, warnAnyEnumConv, warnBareExcept, warnStdPrefix, warnImplicitRangeConversion} result[2] = result[3] - {hintStackTrace, hintExtendedContext, hintDeclaredLoc, hintProcessingStmt} result[1] = result[2] - {warnProveField, warnProveIndex, warnGcUnsafe, hintPath, hintDependency, hintCodeBegin, hintCodeEnd, diff --git a/compiler/sempass2.nim b/compiler/sempass2.nim index 1aca972261..02ec23fc3e 100644 --- a/compiler/sempass2.nim +++ b/compiler/sempass2.nim @@ -84,6 +84,7 @@ type gcUnsafe, isRecursive, isTopLevel, hasSideEffect, inEnforcedGcSafe: bool isInnerProc: bool inEnforcedNoSideEffects: bool + isArrayIndexing: bool currentExceptType: PType unknownRaises: seq[(PSym, TLineInfo)] currOptions: TOptions @@ -148,6 +149,37 @@ proc isLocalSym(a: PEffects, s: PSym): bool = s.typ != nil and (s.kind in {skLet, skVar, skResult} or (s.kind == skParam and isOutParam(s.typ))) and sfGlobal notin s.flags and s.owner == a.owner +proc isRangeSupertype(conf: ConfigRef; wider, narrower: PType): bool = + ## Check if `wider` type fully contains `narrower` type + ## Returns true if narrower fits entirely within wider (safe conversion) + if wider.isOrdinalType: + let wideFirst = firstOrd(conf, wider) + let wideLast = lastOrd(conf, wider) + let narrowFirst = firstOrd(conf, narrower) + let narrowLast = lastOrd(conf, narrower) + result = narrowFirst >= wideFirst and narrowLast <= wideLast + elif not narrower.isOrdinalType: + let wideFirst = firstFloat(wider) + let wideLast = lastFloat(wider) + let narrowFirst = firstFloat(narrower) + let narrowLast = lastFloat(narrower) + result = narrowFirst >= wideFirst and narrowLast <= wideLast + else: + # int -> float ranges; warn + result = false + +proc shouldWarnRangeConversion(conf: ConfigRef; formalType, argType: PType): bool = + ## Determine if an implicit range conversion should warn + ## We warn on conversions that are likely to cause panics + let f = formalType.skipTypes({tyGenericInst, tyAlias, tySink, tyDistinct}) + let a = argType.skipTypes({tyGenericInst, tyAlias, tySink, tyDistinct}) + if f.kind == tyRange: + # Only warn if formal range doesn't fully contain argument range + # Check if the ranges don't perfectly overlap + result = not isRangeSupertype(conf, f, a) + else: + result = false + proc lockLocations(a: PEffects; pragma: PNode) = if pragma.kind != nkExprColonExpr: localError(a.config, pragma.info, "locks pragma without argument") @@ -1504,6 +1536,11 @@ proc track(tracked: PEffects, n: PNode) = message(tracked.config, n.info, warnPtrToCstringConv, $n[1].typ) + # Check for implicit range conversions + if n.kind == nkHiddenStdConv and (not tracked.isArrayIndexing) and + shouldWarnRangeConversion(tracked.config, n.typ, n[1].typ): + message(tracked.config, n.info, warnImplicitRangeConversion, + typeToString(n[1].typ) & " -> " & typeToString(n.typ)) let t = n.typ.skipTypes(abstractInst) if t.kind == tyEnum: @@ -1542,7 +1579,12 @@ proc track(tracked: PEffects, n: PNode) = checkBounds(tracked, n[0], n[1]) track(tracked, n[0]) dec tracked.leftPartOfAsgn - for i in 1 ..< n.len: track(tracked, n[i]) + for i in 1 ..< n.len: + if i == 1: + tracked.isArrayIndexing = true + track(tracked, n[i]) + if i == 1: + tracked.isArrayIndexing = false inc tracked.leftPartOfAsgn of nkError: localError(tracked.config, n.info, errorToString(tracked.config, n)) diff --git a/compiler/sigmatch.nim b/compiler/sigmatch.nim index a8cf05ee81..6c8a01bd52 100644 --- a/compiler/sigmatch.nim +++ b/compiler/sigmatch.nim @@ -615,6 +615,8 @@ proc isGenericObjectOf(f, a: PType): bool = # use sym equality to check if the `tyGenericBody` types are equal result = aRoot != nil and f.sym == aRoot.sym + + proc isObjectSubtype(c: var TCandidate; a, f, fGenericOrigin: PType): int = var t = a assert t.kind == tyObject diff --git a/tests/range/timplicitrangedownsizing.nim b/tests/range/timplicitrangedownsizing.nim new file mode 100644 index 0000000000..1b10f2f322 --- /dev/null +++ b/tests/range/timplicitrangedownsizing.nim @@ -0,0 +1,73 @@ +discard """ +cmd: "nim check $options --hints:off --warning:ImplicitRangeConversion --warningaserror:ImplicitRangeConversion $file" +action: "reject" +nimout: ''' +timplicitrangedownsizing.nim(22, 5) Error: implicit range conversion int -> FakeUint8 [ImplicitRangeConversion] +timplicitrangedownsizing.nim(24, 5) Error: implicit range conversion OffByOneRange -> FakeUint8 [ImplicitRangeConversion] +timplicitrangedownsizing.nim(28, 5) Error: implicit range conversion int -> FakeUint8 [ImplicitRangeConversion] +timplicitrangedownsizing.nim(55, 6) Error: implicit range conversion float64 -> SmallFloat [ImplicitRangeConversion] +timplicitrangedownsizing.nim(59, 6) Error: implicit range conversion FloatRange -> SmallFloat [ImplicitRangeConversion] +timplicitrangedownsizing.nim(63, 6) Error: implicit range conversion float64 -> SmallFloat [ImplicitRangeConversion] +''' +""" +# Integer range tests +type FakeUint8 = range[0..255] +type OffByOneRange = range[1..256] +type WideRange = range[0..65535] + +var v: FakeUint8 +var x = 256 +var y = OffByOneRange(256) + +v = x # panics, should trigger warning +v = FakeUint8(x) # panics, should not trigger warning +v = y # panics should trigger warning + +proc xxx(v: FakeUint8)= discard + +xxx(x) # panics, should trigger warning +xxx(FakeUint8(x)) # panics, should not trigger warning + +# Test narrower to wider range conversions (should NOT warn) +proc acceptWide(v: WideRange) = discard + +var smallRange: FakeUint8 = FakeUint8(100) +acceptWide(smallRange) # OK - FakeUint8 (0..255) fits in WideRange (0..65535) + +var medRange: OffByOneRange = OffByOneRange(150) +acceptWide(medRange) # OK - OffByOneRange (1..256) fits in WideRange (0..65535) + +var w: WideRange +w = smallRange # OK - FakeUint8 range fits in WideRange +w = medRange # OK - OffByOneRange range fits in WideRange + +# Test narrower range passed to function (should NOT warn) +xxx(smallRange) # OK - FakeUint8 value fits in range[0..255] + +# Float range tests +type SmallFloat = range[0.0..10.0] +type FloatRange = range[5.0..15.0] +type WideFloatRange = range[0.0..100.0] + +var fv: SmallFloat +var fx = 11.5 # Out of range + +fv = fx # panics, should trigger warning +fv = SmallFloat(fx) # panics, should not trigger warning + +var fy = FloatRange(7.5) +fv = fy # panics, should trigger warning (5.0..15.0 → 0.0..10.0) + +proc fffx(v: SmallFloat) = discard + +fffx(fx) # panics, should trigger warning +fffx(SmallFloat(fx)) # panics, should not trigger warning + +# Test narrower to wider float range conversions (should NOT warn) +proc acceptWideFloat(v: WideFloatRange) = discard + +var smallFloatRange: SmallFloat = SmallFloat(5.0) +acceptWideFloat(smallFloatRange) # OK - SmallFloat (0.0..10.0) fits in WideFloatRange (0.0..100.0) + +var wf: WideFloatRange +wf = smallFloatRange # OK - SmallFloat range fits in WideFloatRange \ No newline at end of file