Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

double destroy with sink and generic tuple type #1415

Closed
zerbina opened this issue Aug 14, 2024 · 0 comments · Fixed by #1414
Closed

double destroy with sink and generic tuple type #1415

zerbina opened this issue Aug 14, 2024 · 0 comments · Fixed by #1414
Labels
bug Something isn't working compiler/sem Related to semantic-analysis system of the compiler

Comments

@zerbina
Copy link
Collaborator

zerbina commented Aug 14, 2024

Assigning to a sink parameter that uses an instantiated generic tuple type creates an unsafe
shallow copy, which can lead to double frees, depending on the type.

Example

type
  Object = object
    val: int
  Tuple[T] = (T,)

proc `=copy`(x: var Object, y: Object) =
  echo "copied"

proc `=destroy`(x: var Object) =
  if x.val != 0:
    echo "destroyed: ", x.val

proc test(x: sink Tuple[Object]) =
  var y: Tuple[Object]
  y = x # last use of `x`, so this should move, or at least create a proper copy

test((Object(val: 1),))

Actual Output

destroyed: 1
destroyed: 1

Expected Output

destroyed: 1

Additional Information

  • removing the sink makes the issue go away (no destructor is called in that case)
  • Tuple not being generic also makes the issue go away
@zerbina zerbina added bug Something isn't working compiler/sem Related to semantic-analysis system of the compiler labels Aug 14, 2024
github-merge-queue bot pushed a commit that referenced this issue Aug 14, 2024
## 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler/sem Related to semantic-analysis system of the compiler
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant