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

Newruntime: Double destruction of seq/string #11833

Closed
jwollen opened this issue Jul 26, 2019 · 1 comment
Closed

Newruntime: Double destruction of seq/string #11833

jwollen opened this issue Jul 26, 2019 · 1 comment

Comments

@jwollen
Copy link
Contributor

jwollen commented Jul 26, 2019

The following destroys the inner seq twice with --newruntime, causing unexpected behavior. To have it immediately crash consistently, use -d:useMalloc.
(The Foo is not important. This happens with any non-constant element)

type Foo = object

proc test() =
  var x = @[@[Foo()]]

test()

From the generated code:

T2_.len = 1; T2_.p = (tySequence_YBwSFr9cHnwXey9c6gHCsmuA_Content*) newSeqPayload(1, sizeof(tySequence_9aBObKOIQyNbQpSLHF0GtHw));
T3_.len = 1; T3_.p = (tySequence_9aBObKOIQyNbQpSLHF0GtHw_Content*) newSeqPayload(1, sizeof(tyObject_Foo_pMBAR5nJd0PD9cv5J1Hnd9bA));
...
eqsink__vltVh8v0Jk9bJ9bP86AQ9bCzA(colontmpD_, T3_);
T2_.p->data[0] = colontmpD_;
eqsink__6II9babRqgYpp6uFdPL4n3w(x, T2_);
...
eqdestroy__qK268sotx3d7bmLMEDvQtQ(colontmpD_);
eqdestroy__EKI3QOkat5QRImLwai9aGKw(x);

The inner seq is first deallocated through the temporary, then again as part of the outer seq.

Related (but not fixing the issue): The generated destructors don't nil the payload/length of the sequences.

@sinkingsugar
Copy link
Contributor

I would actually suggest to run any --newruntime test with -d:useMalloc btw.
It's the only way to figure memory corruption reliably and sooner.

@jwollen jwollen changed the title Newuntime: Double destruction of seq/string Newruntime: Double destruction of seq/string Jul 26, 2019
Clyybber added a commit to Clyybber/Nim that referenced this issue Aug 11, 2019
Clyybber added a commit to Clyybber/Nim that referenced this issue Aug 12, 2019
Clyybber added a commit to Clyybber/Nim that referenced this issue Aug 15, 2019
Clyybber added a commit to Clyybber/Nim that referenced this issue Aug 16, 2019
Clyybber added a commit to Clyybber/Nim that referenced this issue Aug 20, 2019
Clyybber added a commit to Clyybber/Nim that referenced this issue Aug 21, 2019
Araq added a commit that referenced this issue Aug 23, 2019
@Araq Araq closed this as completed in ce7f29e Aug 24, 2019
Clyybber added a commit to Clyybber/Nim that referenced this issue Aug 25, 2019
Clyybber added a commit to Clyybber/Nim that referenced this issue Sep 28, 2019
Clyybber added a commit to Clyybber/Nim that referenced this issue Sep 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants