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

Miri diagnostics: Show location of creation/invalidation in own code #2185

Open
Noratrieb opened this issue Jun 3, 2022 · 9 comments
Open
Labels
A-aliasing Area: This affects the aliasing model (Stacked/Tree Borrows) A-diagnostics errors and warnings emitted by miri C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement

Comments

@Noratrieb
Copy link
Member

Noratrieb commented Jun 3, 2022

I'm in a crate (rustc_arena) where some UB was found. The created/invalidated spans point at core, since it's in the same workspace.

Backtrace

test tests::bench_copy ... error: Undefined Behavior: attempting a write access using <210850> at alloc86975[0x0], but that tag does not exist in the borrow stack for this location
    --> /home/nilsh/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:1342:9
     |
1342 |         copy_nonoverlapping(&src as *const T, dst, 1);
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |         |
     |         attempting a write access using <210850> at alloc86975[0x0], but that tag does not exist in the borrow stack for this location
     |         this error occurs as part of an access at alloc86975[0x0..0xc]
     |
     = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
     = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <210850> was created by a retag at offsets [0x0..0xffc]
    --> /home/nilsh/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/slice/mod.rs:507:9
     |
507  |         self as *mut [T] as *mut T
     |         ^^^^
help: <210850> was later invalidated at offsets [0x0..0xffc]
    --> compiler/rustc_arena/src/lib.rs:90:48
     |
90   |         MaybeUninit::slice_as_mut_ptr(unsafe { &mut *self.storage.as_mut() })
     |                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^
```

It would be useful to be able to make it point to the actual offending calls in rustc_arena, since the actual bug was there.

See here: rust-lang/rust#97711

@RalfJung
Copy link
Member

RalfJung commented Jun 3, 2022

Do you have a concrete example / testcase for this? That would help make the discussion less abstract.

Cc @saethlin

@Noratrieb
Copy link
Member Author

I have updated the issue with better info

@saethlin
Copy link
Member

saethlin commented Jun 3, 2022

I think the problem here is that core counts as a local crate. Really, hacking on rust-lang/rust is going to be a bear because of this. It currently has 121 crates, so a huge fraction of all code in the build counts as a local crate which is approximately the opposite of what the local crate filtering is supposed to do.

At the moment, the only thing I can think of to do for this is to give the end user more control over the local crate selection. But they might as well just use -Zmiri-track-pointer-tag if they're going to have to run the tool twice anyway. I'm not really sure what a better default would be. For sure using the local crate is the wrong decision because that goes wrong for tests.

@RalfJung
Copy link
Member

RalfJung commented Jun 3, 2022

Oh, this is rustc_arena in the rustc workspace? Yeah that would explain this...

@RalfJung
Copy link
Member

RalfJung commented Jun 3, 2022

We could hard-code core, alloc, and std as never being local? 🤷

@RalfJung RalfJung added C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement A-aliasing Area: This affects the aliasing model (Stacked/Tree Borrows) A-diagnostics errors and warnings emitted by miri labels Jun 5, 2022
@Noratrieb
Copy link
Member Author

Maybe it would make sense to have a flag like -Zmiri-diagnostic-exclude-crates (or a better name) to mark these crates as non-local even if they are. But I'm not sure whether adding more and more flags actually solves issues or only creates new ones (people not knowing about the flags).

@Noratrieb
Copy link
Member Author

I think there is a need for a kind of miri-walkthrough that explains the most important flags and ways to use miri in a nice tutorial. I might take a look at that.

@RalfJung
Copy link
Member

RalfJung commented Jun 8, 2022

Yes, a "Miri book" would probably make a lot of sense. :-)

@RalfJung
Copy link
Member

RalfJung commented Aug 7, 2022

At least the particular span slice/mod.rs:507:9 could possibly be fixed by marking slice.as_mut_ptr as track_caller and having the Stacked Borrows diagnostic code respect that attribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-aliasing Area: This affects the aliasing model (Stacked/Tree Borrows) A-diagnostics errors and warnings emitted by miri C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement
Projects
None yet
Development

No branches or pull requests

3 participants