mirrors/Nim
1
0
Fork 0
mirror of https://github.com/nim-lang/Nim.git synced 2026-08-04 14:38:38 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix #25993; In-place object construction zeroes destination before evaluating self-referencing field values (#25994)

Browse Source
This commit is contained in:
Ryan McConnell
2026-08-03 05:51:16 -04:00
committed by GitHub
parent f6651e6c70
commit 5137d273e5
4 changed files with 177 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

109
compiler/aliases.nim
View File

@@ -21,6 +21,44 @@ type
TAnalysisResult* = enum
arNo, arMaybe, arYes
PartFlag* = enum
pfStructural ## use structural prefix-chain detection and tree-walk
pfBidirectional ## also check reverse direction per field in nkObjConstr
func sameLocation(a, b: PNode): bool =
template sameConstIndex(a, b: PNode): bool =
a.kind in nkLiterals and b.kind in nkLiterals and a.intVal == b.intVal
var a = a
var b = b
while a.kind in {nkHiddenStdConv, nkHiddenSubConv, nkConv}: a = a[1]
while b.kind in {nkHiddenStdConv, nkHiddenSubConv, nkConv}: b = b[1]
if a.kind != b.kind: return false
case a.kind
of nkSym: result = a.sym.id == b.sym.id
of nkDotExpr, nkCheckedFieldExpr:
result = a[1].kind == nkSym and b[1].kind == nkSym and
sameLocation(a[0], b[0]) and a[1].sym.id == b[1].sym.id
of nkBracketExpr:
result = sameLocation(a[0], b[0]) and sameConstIndex(a[1], b[1])
of nkObjUpConv, nkObjDownConv, nkDerefExpr, nkHiddenDeref:
result = sameLocation(a[0], b[0])
else: result = false
proc isAccessorPrefixOf(a, b: PNode): bool =
var cur = b
while cur.kind in {nkDotExpr, nkBracketExpr, nkCheckedFieldExpr, nkObjUpConv,
nkObjDownConv, nkHiddenDeref, nkDerefExpr,
nkHiddenStdConv, nkHiddenSubConv, nkConv}:
if sameLocation(cur, a): return true
case cur.kind
of nkDotExpr, nkBracketExpr, nkCheckedFieldExpr, nkObjUpConv, nkObjDownConv,
nkHiddenDeref, nkDerefExpr:
cur = cur[0]
of nkHiddenStdConv, nkHiddenSubConv, nkConv:
cur = cur[1]
else: discard
result = sameLocation(cur, a)
proc isPartOfAux(a, b: PType, marker: var IntSet): TAnalysisResult
proc isPartOfAux(n: PNode, b: PType, marker: var IntSet): TAnalysisResult =
@@ -70,14 +108,28 @@ proc isPartOf(a, b: PType): TAnalysisResult =
# watch out: parameters reversed because I'm too lazy to change the code...
result = isPartOfAux(b, a, marker)
proc isPartOf*(a, b: PNode): TAnalysisResult =
## checks if location `a` can be part of location `b`. We treat seqs and
## strings as pointers because the code gen often just passes them as such.
proc isPartOf*(a, b: PNode; flags: set[PartFlag] = {}): TAnalysisResult =
## Checks if location `a` can be part of location `b`: i.e. whether writing to
## `b` could affect what `a` reads. We treat seqs and strings as pointers
## because the code gen often just passes them as such.
##
## Note: `a` can only be part of `b`, if `a`'s type can be part of `b`'s
## type. Since however type analysis is more expensive, we perform it only
## if necessary.
##
## When `pfStructural` is set additional aliasing is detected:
## * a structural prefix of an accessor chain is considered part of it
## (e.g. `x.f <| x.f.g`). Normally `x.f !<| x.f.g` because the
## same-kind `nkDotExpr` comparison treats the differing field names as
## siblings, but `pfStructural` walks the chain to recognise the
## relationship.
## * Unrecognised node kinds are traversed recursively.
##
## When `pfBidirectional` is set:
## * In `nkObjConstr` the reverse direction `isPartOf(value, a)` is also
## checked per field value so that reads hidden behind calls/closures
## are detected.
##
## cases:
##
## YES-cases:
@@ -86,13 +138,14 @@ proc isPartOf*(a, b: PNode): TAnalysisResult =
## x[] <| x
## x[i] <| x
## x.f <| x
## x.f <| x.f.g # when pfStructural (prefix chain)
## ```
##
## NO-cases:
## ```
## x !<| y # depending on type and symbol kind
## x[constA] !<| x[constB]
## x.f !<| x.g
## x.f !<| x.g # sibling fields at same level
## x.f !<| y.f iff x !<= y
## ```
##
@@ -121,7 +174,7 @@ proc isPartOf*(a, b: PNode): TAnalysisResult =
else:
result = arNo
of nkBracketExpr:
result = isPartOf(a[0], b[0])
result = isPartOf(a[0], b[0], flags)
if a.len >= 2 and b.len >= 2:
# array accesses:
if result == arYes and isDeepConstExpr(a[1]) and isDeepConstExpr(b[1]):
@@ -131,7 +184,11 @@ proc isPartOf*(a, b: PNode): TAnalysisResult =
var y = if b[1].kind == nkHiddenStdConv: b[1][1] else: b[1]
if sameValue(x, y): result = arYes
elif pfStructural in flags and isAccessorPrefixOf(a, b):
result = arYes
else: result = arNo
elif pfStructural in flags and isAccessorPrefixOf(a, b):
result = arYes
# else: maybe and no are accurate
else:
# pointer derefs:
@@ -139,22 +196,25 @@ proc isPartOf*(a, b: PNode): TAnalysisResult =
if isPartOf(a.typ, b.typ) != arNo: result = arMaybe
of nkDotExpr:
result = isPartOf(a[0], b[0])
result = isPartOf(a[0], b[0], flags)
if result != arNo:
# if the fields are different, it's not the same location
if a[1].sym.id != b[1].sym.id:
result = arNo
if pfStructural in flags and isAccessorPrefixOf(a, b):
result = arYes
else:
result = arNo
of nkHiddenDeref, nkDerefExpr:
result = isPartOf(a[0], b[0])
result = isPartOf(a[0], b[0], flags)
# weaken because of indirection:
if result != arYes:
if isPartOf(a.typ, b.typ) != arNo: result = arMaybe
of nkHiddenStdConv, nkHiddenSubConv, nkConv:
result = isPartOf(a[1], b[1])
result = isPartOf(a[1], b[1], flags)
of nkObjUpConv, nkObjDownConv, nkCheckedFieldExpr:
result = isPartOf(a[0], b[0])
result = isPartOf(a[0], b[0], flags)
else: result = arNo
# Calls return a new location, so a default of ``arNo`` is fine.
else:
@@ -167,31 +227,31 @@ proc isPartOf*(a, b: PNode): TAnalysisResult =
case b.kind
of Ix0Kinds:
# a* !<| b.f iff a* !<| b
result = isPartOf(a, b[0])
result = isPartOf(a, b[0], flags)
of DerefKinds:
# a* !<| b[] iff
result = arNo
if isPartOf(a.typ, b.typ) != arNo:
result = isPartOf(a, b[0])
result = isPartOf(a, b[0], flags)
if result == arNo: result = arMaybe
of Ix1Kinds:
# a* !<| T(b) iff a* !<| b
result = isPartOf(a, b[1])
result = isPartOf(a, b[1], flags)
of nkSym:
# b is an atom, so we have to check a:
case a.kind
of Ix0Kinds:
# a.f !<| b* iff a.f !<| b*
result = isPartOf(a[0], b)
result = isPartOf(a[0], b, flags)
of Ix1Kinds:
result = isPartOf(a[1], b)
result = isPartOf(a[1], b, flags)
of DerefKinds:
if isPartOf(a.typ, b.typ) != arNo:
result = isPartOf(a[0], b)
result = isPartOf(a[0], b, flags)
if result == arNo: result = arMaybe
else:
result = arNo
@@ -199,20 +259,29 @@ proc isPartOf*(a, b: PNode): TAnalysisResult =
of nkObjConstr:
result = arNo
for i in 1..<b.len:
let res = isPartOf(a, b[i][1])
let res = isPartOf(a, b[i][1], flags)
if res != arNo:
result = res
if res == arYes: break
if pfBidirectional in flags:
let res2 = isPartOf(b[i][1], a, {pfStructural})
if res2 != arNo:
result = res2
if res2 == arYes: break
of nkCallKinds:
result = arNo
for i in 1..<b.len:
let res = isPartOf(a, b[i])
let res = isPartOf(a, b[i], flags)
if res != arNo:
result = res
if res == arYes: break
of nkBracket:
if b.len > 0:
result = isPartOf(a, b[0])
result = isPartOf(a, b[0], flags)
else:
result = arNo
else: result = arNo
else:
if pfStructural in flags:
for i in 0..<b.safeLen:
if isPartOf(a, b[i], flags) != arNo: return arMaybe
result = arNo

4
compiler/ccgcalls.nim
View File

@@ -51,7 +51,7 @@ proc preventNrvo(p: BProc; dest, le, ri: PNode): bool =
if le != nil:
for i in 1..<ri.len:
let r = ri[i]
if isPartOf(le, r) != arNo: return true
if isPartOf(le, r, {pfStructural}) != arNo: return true
# we use the weaker 'canRaise' here in order to prevent too many
# annoying warnings, see #14514
if canRaise(ri[0]) and
@@ -61,7 +61,7 @@ proc preventNrvo(p: BProc; dest, le, ri: PNode): bool =
if dest != nil and dest != le:
for i in 1..<ri.len:
let r = ri[i]
if isPartOf(dest, r) != arNo: return true
if isPartOf(dest, r, {pfStructural}) != arNo: return true
proc hasNoInit(call: PNode): bool {.inline.} =
result = call[0].kind == nkSym and sfNoInit in call[0].sym.flags

2
compiler/ccgexprs.nim
View File

@@ -1907,7 +1907,7 @@ proc genObjConstr(p: BProc, e: PNode, d: var TLoc) =
isRef or
d.k == locNone or
(d.t != nil and not sameBackendType(t, d.t.skipTypes(abstractInstOwned))) or
(isPartOf(d.lode, e) != arNo)
(isPartOf(d.lode, e, {pfStructural, pfBidirectional}) != arNo)
var tmp: TLoc = default(TLoc)
var r: Rope

85
tests/ccgbugs/tobjconstr_self_alias.nim Normal file
View File

@@ -0,0 +1,85 @@
discard """
matrix: "--mm:refc; --mm:arc; --mm:orc"
output: '''42
55
42
42
42
42'''
"""
# bug #25993 : an object constructor assigned to a location zeroed the
# destination before evaluating a field value that reads from inside that same
# destination, so `tp.h = H(a: tp.h.a)` produced `a == 0`.
type
Inner = object
a: int
b: int
Mid = object
inner: Inner
x: int
RefT = ref object
h: Inner
other: Inner
m: Mid
# --------------------------------------------------------------------------
# bug demonstrations: each printed 0 before the fix
# --------------------------------------------------------------------------
proc refDotField(v: int) =
# dest `t.h` is a field of a ref; value reads `t.h.a` (nested in dest)
let t = RefT()
t.h.a = v
t.h = Inner(a: t.h.a)
echo t.h.a
proc nestedConstr(v: int) =
# dest `t.m`; nested constructor value reads `t.m.inner.a` (nested in dest)
let t = RefT()
t.m.inner.a = v
t.m = Mid(inner: Inner(a: t.m.inner.a), x: 0)
echo t.m.inner.a
proc refDeepField(v: int) =
# dest `t.m.inner`; value reads `t.m.inner.a` (nested in dest)
let t = RefT()
t.m.inner.a = v
t.m.inner = Inner(a: t.m.inner.a)
echo t.m.inner.a
var gT: RefT
proc readsField(t: RefT): int = t.h.a
proc viaCall(v: int) =
# read of dest hidden behind a call whose argument is the root ref
let t = RefT()
t.h.a = v
t.h = Inner(a: readsField(t))
echo t.h.a
proc viaClosureGlobal(v: int) =
# read of dest hidden behind a closure reaching it through a global
let t = RefT()
t.h.a = v
gT = t
let cl = proc(): int = gT.h.a
t.h = Inner(a: cl())
echo t.h.a
proc viaClosureCapture(v: int) =
# read of dest hidden behind a closure that captures the root ref
let t = RefT()
t.h.a = v
let cl = proc(): int = t.h.a
t.h = Inner(a: cl())
echo t.h.a
refDotField(42)
nestedConstr(55)
refDeepField(42)
viaCall(42)
viaClosureGlobal(42)
viaClosureCapture(42)