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

Programs that cast function pointers have escalating memory use #3737

Closed
saethlin opened this issue Jul 7, 2024 · 1 comment · Fixed by rust-lang/rust#128742
Closed

Programs that cast function pointers have escalating memory use #3737

saethlin opened this issue Jul 7, 2024 · 1 comment · Fixed by rust-lang/rust#128742
Labels
A-interpreter Area: affects the core interpreter C-bug Category: This is a bug.

Comments

@saethlin
Copy link
Member

saethlin commented Jul 7, 2024

Since rust-lang/rust#123781, this program has unbounded memory use:

fn main() {
    loop {
        oof as fn(); // oof as usize also works
    }
}

fn oof() {}

Currently it looks like the memory use from this is about 1 GB/min, which means that in practice it's likely that this technically-unbounded memory use will go unnoticed.

But in general, I've been trying to make sure that Miri's memory usage is bounded on as many workloads as reasonable so that if we're too slow, users can fall back to just letting Miri run for a long time. If the interpreter's memory grows over that time without bound, that's not an option. So it would be nice if we didn't have this memory growth.

This reminds me a lot of the situation that prompted us to cache our evaluations of consts: rust-lang/rust#118336. Does a similar approach work here?

@saethlin saethlin changed the title Programs that cast function pointers to integers have escalating memory use Programs that cast function pointers escalating memory use Jul 7, 2024
@saethlin saethlin changed the title Programs that cast function pointers escalating memory use Programs that cast function pointers have escalating memory use Jul 7, 2024
@RalfJung
Copy link
Member

RalfJung commented Jul 7, 2024

Since rust-lang/rust#123781,

A similar program where oof is generic would have had the same kind of escalating memory use already before that PR.

This reminds me a lot of the situation that prompted us to cache our evaluations of consts: rust-lang/rust#118336. Does a similar approach work here?

Yeah, I think so. We could make oof as fn() return one of a finite set of possible addresses for that function.

@RalfJung RalfJung added C-bug Category: This is a bug. A-interpreter Area: affects the core interpreter labels Aug 7, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 9, 2024
…=saethlin

miri: make vtable addresses not globally unique

Miri currently gives vtables a unique global address. That's not actually matching reality though. So this PR enables Miri to generate different addresses for the same type-trait pair.

To avoid generating an unbounded number of `AllocId` (and consuming unbounded amounts of memory), we use the "salt" technique that we also already use for giving constants non-unique addresses: the cache is keyed on a "salt" value n top of the actually relevant key, and Miri picks a random salt (currently in the range `0..16`) each time it needs to choose an `AllocId` for one of these globals -- that means we'll get up to 16 different addresses for each vtable. The salt scheme is integrated into the global allocation deduplication logic in `tcx`, and also used for functions and string literals. (So this also fixes the problem that casting the same function to a fn ptr over and over will consume unbounded memory.)

r? `@saethlin`
Fixes rust-lang/miri#3737
bors added a commit to rust-lang-ci/rust that referenced this issue Aug 11, 2024
…aethlin

miri: make vtable addresses not globally unique

Miri currently gives vtables a unique global address. That's not actually matching reality though. So this PR enables Miri to generate different addresses for the same type-trait pair.

To avoid generating an unbounded number of `AllocId` (and consuming unbounded amounts of memory), we use the "salt" technique that we also already use for giving constants non-unique addresses: the cache is keyed on a "salt" value n top of the actually relevant key, and Miri picks a random salt (currently in the range `0..16`) each time it needs to choose an `AllocId` for one of these globals -- that means we'll get up to 16 different addresses for each vtable. The salt scheme is integrated into the global allocation deduplication logic in `tcx`, and also used for functions and string literals. (So this also fixes the problem that casting the same function to a fn ptr over and over will consume unbounded memory.)

r? `@saethlin`
Fixes rust-lang/miri#3737
bors added a commit to rust-lang-ci/rust that referenced this issue Aug 13, 2024
…aethlin

miri: make vtable addresses not globally unique

Miri currently gives vtables a unique global address. That's not actually matching reality though. So this PR enables Miri to generate different addresses for the same type-trait pair.

To avoid generating an unbounded number of `AllocId` (and consuming unbounded amounts of memory), we use the "salt" technique that we also already use for giving constants non-unique addresses: the cache is keyed on a "salt" value n top of the actually relevant key, and Miri picks a random salt (currently in the range `0..16`) each time it needs to choose an `AllocId` for one of these globals -- that means we'll get up to 16 different addresses for each vtable. The salt scheme is integrated into the global allocation deduplication logic in `tcx`, and also used for functions and string literals. (So this also fixes the problem that casting the same function to a fn ptr over and over will consume unbounded memory.)

r? `@saethlin`
Fixes rust-lang/miri#3737
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Aug 13, 2024
miri: make vtable addresses not globally unique

Miri currently gives vtables a unique global address. That's not actually matching reality though. So this PR enables Miri to generate different addresses for the same type-trait pair.

To avoid generating an unbounded number of `AllocId` (and consuming unbounded amounts of memory), we use the "salt" technique that we also already use for giving constants non-unique addresses: the cache is keyed on a "salt" value n top of the actually relevant key, and Miri picks a random salt (currently in the range `0..16`) each time it needs to choose an `AllocId` for one of these globals -- that means we'll get up to 16 different addresses for each vtable. The salt scheme is integrated into the global allocation deduplication logic in `tcx`, and also used for functions and string literals. (So this also fixes the problem that casting the same function to a fn ptr over and over will consume unbounded memory.)

r? `@saethlin`
Fixes rust-lang/miri#3737
RalfJung pushed a commit to RalfJung/miri that referenced this issue Aug 14, 2024
miri: make vtable addresses not globally unique

Miri currently gives vtables a unique global address. That's not actually matching reality though. So this PR enables Miri to generate different addresses for the same type-trait pair.

To avoid generating an unbounded number of `AllocId` (and consuming unbounded amounts of memory), we use the "salt" technique that we also already use for giving constants non-unique addresses: the cache is keyed on a "salt" value n top of the actually relevant key, and Miri picks a random salt (currently in the range `0..16`) each time it needs to choose an `AllocId` for one of these globals -- that means we'll get up to 16 different addresses for each vtable. The salt scheme is integrated into the global allocation deduplication logic in `tcx`, and also used for functions and string literals. (So this also fixes the problem that casting the same function to a fn ptr over and over will consume unbounded memory.)

r? `@saethlin`
Fixes rust-lang#3737
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-interpreter Area: affects the core interpreter C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants