Skip to content

Commit

Permalink
fix double-free and compiler crash (#1414)
Browse files Browse the repository at this point in the history
## Summary

* fix a double-free issue when assigning to `sink` parameters of
  instantiated generic tuple type
* fix a compiler crash when passing an empty `seq`, `array`, or `set`
  construction to a generic instantiation receiver type

Fixes #1415.

## Details

Post-match fitting didn't consider generic tuple instantiations when
applying implicit conversions, meaning that the `nkHiddenSubConv` or
`nkHiddenStdConv` inserted earlier by `sigmatch` stayed.

This led to two issues:
* `nkStmtListExpr`s of empty container type didn't have their types
  fixed properly, causing crashes when the empty type reached into the
  MIR phase
* a conversion between `sink T` and `T` is considered an rvalue
  conversion by `proto_mir`, which resulted in assignments involving
  these implicit conversions effectively being *shallow* assignments
  (and thus double frees)

Skipping `tyGenericInst` and `tyAlias` (for good measure) makes sure
the implicit conversion is collapsed when the destination type is a
generic instantiation, fixing both issues. A test is added for both.
  • Loading branch information
zerbina authored Aug 14, 2024
1 parent ee6d6aa commit a1decd8
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 4 deletions.
7 changes: 4 additions & 3 deletions compiler/sem/sem.nim
Original file line number Diff line number Diff line change
Expand Up @@ -202,10 +202,11 @@ proc applyConversion(c: PContext, conv, n: PNode): tuple[n: PNode, keep: bool] =
else:
(n, tmp.keep)
else:
if conv.typ.kind in {tySet, tyTuple}:
let styp = conv.typ.skipTypes({tyAlias, tyGenericInst})
if styp.kind in {tySet, tyTuple}:
# apply the type directly and drop the conversion
n.typ = conv.typ
elif conv.typ.kind in {tyOpenArray, tyVarargs, tySequence, tyArray} and
elif styp.kind in {tyOpenArray, tyVarargs, tySequence, tyArray} and
n.typ.isEmptyContainer:
# fixup empty container types
let
Expand All @@ -221,7 +222,7 @@ proc applyConversion(c: PContext, conv, n: PNode): tuple[n: PNode, keep: bool] =
n.typ = typ

# keep to-openArray conversions, later processing still needs them
(n, conv.typ.kind notin {tySequence, tyArray, tyTuple, tySet})
(n, styp.kind notin {tySequence, tyArray, tyTuple, tySet})

proc fitNodePostMatch(c: PContext, n: PNode): PNode =
## Performs post-processing on the result of a ``paramTypesMatch``
Expand Down
12 changes: 11 additions & 1 deletion tests/lang_exprs/tempty_typed_expressions.nim
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ discard """
'''
"""

type Generic[T] = seq[T]

proc get(x: pointer): pointer = x
proc get(x: array[0, int]): array[0, int] = x
proc get(x: seq[int]): seq[int] = x
Expand All @@ -17,6 +19,10 @@ proc get2(x: openArray[int]): int =
# parameter was passed correctly
x.len

proc get3(x: Generic[int]): Generic[int] =
# test with a generic receiver type
x

# simple case: empty-container typed expression is passed directly
discard get(nil)
discard get([])
Expand All @@ -26,6 +32,8 @@ discard get({})
doAssert get2([]) == 0
doAssert get2(@[]) == 0

discard get3(@[])

# more complex case: statement-list expressions
discard get((discard; nil))
# XXX: not working yet
Expand All @@ -34,4 +42,6 @@ discard get((discard; @[]))
discard get((discard; {}))

doAssert get2((discard; [])) == 0
doAssert get2((discard; @[])) == 0
doAssert get2((discard; @[])) == 0

discard get3((discard; @[]))
24 changes: 24 additions & 0 deletions tests/lang_types/tuples/tsink_generic_tuple_assignment.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
discard """
description: '''
Regression test for a bug where assigning a ``sink`` generic tuple type
to a variable of non-``sink`` type led to the value being shallow copied.
'''
matrix: "--showir:mir_in:test --hints:off"
nimout: '''
-- MIR: test
scope:
def x: sink Tuple[system.int]
def v: Tuple[system.int]
v = sink x
-- end
'''
"""

type Tuple[T] = (T, T)

proc test(x: sink Tuple[int]) =
var v: Tuple[int]
v = x # this assignment was considered to involve a conversion

test default(Tuple[int])

0 comments on commit a1decd8

Please sign in to comment.