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

Support setting Miri flags in the rustc file via attributes #3670

Open
RalfJung opened this issue Jun 13, 2024 · 2 comments
Open

Support setting Miri flags in the rustc file via attributes #3670

RalfJung opened this issue Jun 13, 2024 · 2 comments
Labels
A-interpreter Area: affects the core interpreter C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement C-proposal Category: a proposal for something we might want to do, or maybe not; details still being worked out

Comments

@RalfJung
Copy link
Member

It could be quite useful to support a crate-level attribute on binary crates like #[miri::flag("-Zmiri-disable-leak-check")]. That would help both on the playground and for doctests, to control the flags the file is run with.

Not sure how to best implement that though, and it may be considered an abomination by the compiler team. ;)

@RalfJung RalfJung added C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement C-proposal Category: a proposal for something we might want to do, or maybe not; details still being worked out A-interpreter Area: affects the core interpreter labels Jun 13, 2024
@RalfJung
Copy link
Member Author

RalfJung commented Jun 15, 2024

@oli-obk any idea what the best way might be to implement such an attribute? I am imagining something like

#![miri::flag("-Zmiri-tree-borrows")]

Usually this would be gated by cfg_attr(miri, ...).

In a lib crate this has no effect (we should likely lint against that), but in a binary crate this makes Miri act as-if those flags were passes on the command line.

Or should this maybe even be a more general rustc feature?

@oli-obk
Copy link
Contributor

oli-obk commented Jun 19, 2024

yea I've wanted this as a general rustc feature before, but it can't really work because some flags are processed before the source file is even opened.

I think I would prefer just allowing miri flags to also be used via native attributes like #[miri::tree_borrows]. We can set up the infrastructure to not duplicate the logic between attributes and command line flags

workingjubilee added a commit to workingjubilee/rustc that referenced this issue Jul 14, 2024
…oc, r=Mark-Simulacrum

Remove memory leaks in doctests in `core`, `alloc`, and `std`

cc `@RalfJung`  rust-lang#126067 rust-lang/miri#3670

Should be no actual *documentation* changes[^1], all added/modified lines in the doctests are hidden with `#`,

This PR splits the existing memory leaks in doctests in `core`, `alloc`, and `std` into two general categories:

1. "Non-focused" memory leaks that are incidental to the thing being documented, and/or are easy to remove, i.e. they are only there because preventing the leak would make the doctest less clear and/or concise.
    - These doctests simply have a comment like `# // Prevent leaks for Miri.` above the added line that removes the memory leak.
    - [^2]Some of these would perhaps be better as part of the public documentation part of the doctest, to clarify that a memory leak can happen if it is not otherwise mentioned explicitly in the documentation  (specifically the ones in `(A)Rc::increment_strong_count(_in)`).
2. "Focused" memory leaks that are intentional and documented, and/or are possibly fragile to remove.
    - These doctests have a `# // FIXME` comment above the line that removes the memory leak, with a note that once `-Zmiri-disable-leak-check` can be applied at test granularity, these tests should be "un-unleakified" and have `-Zmiri-disable-leak-check` enabled.
    - Some of these are possibly fragile (e.g. unleaking the result of `Vec::leak`) and thus should definitely not be made part of the documentation.

This should be all of the leaks currently in `core` and `alloc`. I only found one leak in `std`, and it was in the first category (excluding the modules `@RalfJung` mentioned in rust-lang#126067 , and reducing the number of iterations of [one test](https://github.com/rust-lang/rust/blob/master/library/std/src/sync/once_lock.rs#L49-L94) from 1000 to 10)

[^1]: assuming [^2] is not added
[^2]: backlink
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jul 14, 2024
Rollup merge of rust-lang#127446 - zachs18:miri-stdlib-leaks-core-alloc, r=Mark-Simulacrum

Remove memory leaks in doctests in `core`, `alloc`, and `std`

cc `@RalfJung`  rust-lang#126067 rust-lang/miri#3670

Should be no actual *documentation* changes[^1], all added/modified lines in the doctests are hidden with `#`,

This PR splits the existing memory leaks in doctests in `core`, `alloc`, and `std` into two general categories:

1. "Non-focused" memory leaks that are incidental to the thing being documented, and/or are easy to remove, i.e. they are only there because preventing the leak would make the doctest less clear and/or concise.
    - These doctests simply have a comment like `# // Prevent leaks for Miri.` above the added line that removes the memory leak.
    - [^2]Some of these would perhaps be better as part of the public documentation part of the doctest, to clarify that a memory leak can happen if it is not otherwise mentioned explicitly in the documentation  (specifically the ones in `(A)Rc::increment_strong_count(_in)`).
2. "Focused" memory leaks that are intentional and documented, and/or are possibly fragile to remove.
    - These doctests have a `# // FIXME` comment above the line that removes the memory leak, with a note that once `-Zmiri-disable-leak-check` can be applied at test granularity, these tests should be "un-unleakified" and have `-Zmiri-disable-leak-check` enabled.
    - Some of these are possibly fragile (e.g. unleaking the result of `Vec::leak`) and thus should definitely not be made part of the documentation.

This should be all of the leaks currently in `core` and `alloc`. I only found one leak in `std`, and it was in the first category (excluding the modules `@RalfJung` mentioned in rust-lang#126067 , and reducing the number of iterations of [one test](https://github.com/rust-lang/rust/blob/master/library/std/src/sync/once_lock.rs#L49-L94) from 1000 to 10)

[^1]: assuming [^2] is not added
[^2]: backlink
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-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement C-proposal Category: a proposal for something we might want to do, or maybe not; details still being worked out
Projects
None yet
Development

No branches or pull requests

2 participants