-
Notifications
You must be signed in to change notification settings - Fork 13.3k
miri: make vtable addresses not globally unique #128742
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
Conversation
Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri The Miri subtree was changed cc @rust-lang/miri |
6144d02
to
5cab8ae
Compare
Comment typo, then r=me :) |
…ld=always (even if that breaks backtraces)
8f58975
to
dec5b46
Compare
@bors r=saethlin |
…=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
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#128742 (miri: make vtable addresses not globally unique) - rust-lang#128815 (Add `Steal::is_stolen()`) - rust-lang#128859 (Fix the name of signal 19 in library/std/src/sys/pal/unix/process/process_unix/tests.rs for mips/sparc linux) - rust-lang#128864 (Use `SourceMap::end_point` instead of `- BytePos(1)` in arg removal suggestion) - rust-lang#128865 (Ensure let stmt compound assignment removal suggestion respect codepoint boundaries) - rust-lang#128874 (Disable verbose bootstrap command failure logging by default) r? `@ghost` `@rustbot` modify labels: rollup
might be causing this? #128877 (comment) |
Hm, yes it might. This is odd though... I think for now the easiest option is to tweak the test. |
All right this should do it. |
This comment has been minimized.
This comment has been minimized.
@bors r- |
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
… fully predictable
32d3104
to
4763d12
Compare
@bors r=saethlin |
…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
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
@bors retry (msvc ci elevated filesystem errors rates) |
☀️ Test successful - checks-actions |
Finished benchmarking commit (591ecb8): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)Results (primary 4.3%, secondary 1.5%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 753.953s -> 751.332s (-0.35%) |
Pinging back #127883. |
Perf looks to be just recovery from bimodality, not a genuine improvement, which seems plausible for this change. |
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 range0..16
) each time it needs to choose anAllocId
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 intcx
, 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