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

Leak when returning out of box syntax #62289

Closed
agashlin opened this issue Jul 2, 2019 · 5 comments · Fixed by #62331
Closed

Leak when returning out of box syntax #62289

agashlin opened this issue Jul 2, 2019 · 5 comments · Fixed by #62331
Labels
A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@agashlin
Copy link

agashlin commented Jul 2, 2019

@nils-ohlmeier mentioned a leak coming from vec![f()?] but not vec![f()?; 1] here, I think I tracked this down to the use of box in vec! when given a comma-separated list.

I've made a smallish test that demonstrates it with stats_alloc, if it gets much simpler than this then the leak doesn't occur when built with --release.

I'm testing on rustc 1.37.0-nightly (0af8e87 2019-06-30) nightly-x86_64-pc-windows-msvc, targeting x86_64-pc-windows-msvc.

@Centril Centril added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 2, 2019
@agashlin
Copy link
Author

agashlin commented Jul 2, 2019

Valgrind Memcheck also reports heap in use at exit on a Linux VM with the test linked above.

@nils-ohlmeier
Copy link

Thanks a lot @agashlin for creating a reduced test case and creating this issue here.

I stumbled over the original issue when carg-fuzz reported a memory leak in this code https://github.com/mozilla/webrtc-sdp/blob/3a46c37c6d11e2e856b946f4a2e11d90adde3af6/src/attribute_type.rs#L2060-L2062

This PR appears to fix the issue https://github.com/mozilla/webrtc-sdp/pull/151/files

This was also with rustc 1.37.0-nightly on Linux. I can check if I can repro with the fuzz test case on other platforms.

@Manishearth
Copy link
Member

Looks like the early return doesn't interact well with box syntax's preallocation.

MIR generation issue? cc @oli-obk

@jonas-schievink jonas-schievink added A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html C-bug Category: This is a bug. labels Jul 2, 2019
@matthewjasper
Copy link
Contributor

value.ty should be expr.ty here:

this.schedule_drop_storage_and_value(
expr_span,
scope,
result,
value.ty,
);

Now that schedule_drop can only schedule drops for locals, we should get the type from self.local_decls to avoid this sort of problem in the future.

wesleywiser added a commit to wesleywiser/rust that referenced this issue Jul 4, 2019
Centril added a commit to Centril/rust that referenced this issue Jul 5, 2019
…r=matthewjasper

Fix leak when early returning out of `box` syntax

Fixes rust-lang#62289

r? @matthewjasper
wesleywiser added a commit to wesleywiser/rust that referenced this issue Jul 12, 2019
bors added a commit that referenced this issue Jul 14, 2019
…sper

Fix leak when early returning out of `box` syntax

Fixes #62289

r? @matthewjasper
@nils-ohlmeier
Copy link

Confirmed with my fuzzing test case that the problem is fixed in the latest rust nightly.
Thanks for fixing it so quickly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants