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

Leaking a heap allocation of pointer results in confusing output #2812

Closed
nonl4331 opened this issue Mar 12, 2023 · 6 comments · Fixed by rust-lang/rust#109061
Closed

Leaking a heap allocation of pointer results in confusing output #2812

nonl4331 opened this issue Mar 12, 2023 · 6 comments · Fixed by rust-lang/rust#109061
Assignees

Comments

@nonl4331
Copy link

Take the code:

let local = 0usize;
let leak_this = Box::new(&local as *const usize);
Box::into_raw(leak_this);

Running cargo miri run produces the following output:

The following memory was leaked: alloc1673 (Rust heap, size: 8, align: 8) {
    ╾0x28970[a1657]<3157>─╼                         │ ╾──────╼
}
alloc1657 (deallocated)

It is not immediately clear why alloc1657 is shown or the relation between alloc1673 and alloc1657. If alloc1657 were not deallocated it would be possible to interpret alloc1657 as leaked memory.
One suggestion would be to explicitly mention that the leaked memory contains alloc1657 and expand a1657 to alloc1657 for clarity.

The following memory was leaked: alloc1673 (Rust heap, size: 8, align: 8) {
    ╾0x28970[alloc1657]<3157>─╼                         │ ╾──────╼
}
alloc1673 contains a pointer: alloc1657 (deallocated)
@saethlin
Copy link
Member

(my comment is directed at everyone else, not OP)

The fact that the leak report also says a1657 instead of alloc1657 also doesn't help with the confusing newcomer experience.

But more importantly: Why are we even mentioning that alloc1657 is deallocated? The program has exited, so allocations can only be deallocated or leaked. If an allocation is leaked, it should be part of the leak report.

@oli-obk
Copy link
Contributor

oli-obk commented Mar 12, 2023

Hmm... I thought it to be strictly better to tell the user about all the things their leaked alloc references, but it was definitely not based on any data

@saethlin
Copy link
Member

saethlin commented Mar 12, 2023

This is not the first time I've seen a newcomer be bewildered by the amount of text that gets written to the screen and just ignore most of it. It is of course possible to puzzle through this report, but the fact that it has so much information and dense notation and a lot of these IDs I think causes people to skim over it way too often.

It wouldn't hurt to link people to something like a glossary or the rustc error index.

But in this case specifically, I think unless we can explain how a user would use the

alloc1657 (deallocated)

to debug their leak, not outputting that sounds like a better option.

@oli-obk
Copy link
Contributor

oli-obk commented Mar 12, 2023

In that case, maybe we should change the Allocation printing logic (in rustc_interpret), too?

The following memory was leaked: alloc1673 (Rust heap, size: 8, align: 8) {
    ╾0x28970[dangling]─╼                         │ ╾──────╼
}

@saethlin
Copy link
Member

saethlin commented Mar 12, 2023

I think Miri prints 0xaddr[noalloc] in that case (for use of dangling pointers). We should try to be consistent, so whatever the terminology we should try to reuse the code.

@RalfJung
Copy link
Member

One suggestion would be to explicitly mention that the leaked memory contains alloc1657 and expand a1657 to alloc1657 for clarity.

FWIW, what happens here is that 0x28970[alloc1657]<3157> is too wide to fit into the space that 8 bytes take in the memory dump, so alloc gets shortened to a such that the alignment of bytes is preserved in the output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants