Automatically suppress Rust standard library leaks #121
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jfrimmel
force-pushed
the
rust-std-suppressions
branch
from
dc8a432 to
60470a0
Compare
As reported in #111 and #116, at least Rust Rust 1.83 and the upcoming Rust 1.86 versions have possible leaks in the standard library. Since those reported lints are not actionable by the user, they should be suppressed. This was requested in #107 as well. Thankfully, `valgrind` has support for suppression files, that even can be auto-generated with the `--gen-suppressions=yes`-flag on normal valgrind invocations, e.g. [like so][comment]: ```shell $ cargo new --lib some-tests $ cd some-tests $ cargo test --lib $ valgrind --leak-check=full --gen-suppressions=yes target/debug/deps/some_tests-$HASH ``` Those generated traces are the full backtrace of the application, which maybe need to be trimmed down to match certain functions, which will leak (as it is done in the Rust 1.86 case). This commit adds the suppression files necessary to support all Rust versions from the MSRV up to the current beta compiler (1.86). This was checked using the following script: ```bash for rust in $(seq -f '1.%g' 58 85) beta; do echo $rust VALGRINDFLAGS='--suppressions=../../suppressions/rust-1.83 --suppressions=../../suppressions/rust-1.86' cargo +$rust valgrind test --manifest-path tests/default-new-project/Cargo.toml -q done ``` [comment]: #116 (comment)
This commit allows to pass a list of paths to the `valgrind::execute()`- command, which is converted into arguments of `--suppressions=<path>` for each path. This allows to supply pre-defined suppression files to valgrind. Note, that the list is currently empty, since there is no way to embed the standard library suppressions yet.
Previously, the user needed to provide the suppression files on disk in
order to make them readable by `valgrind` (as it unfortunately can only
read them by file). This is unfortunate, as `cargo valgrind` should pro-
vide a list of standard library suppressions without the need for extern
resources. Therefore the files are embedded into the binary.
This is done by two co-working parts:
1. the build script, which reads all files in the `suppressions`-folder
and writes their contents to a constant slice `SUPPRESSIONS`.
2. the runtime code, which crates a temporary directory and writes the
suppression contents into on file each in that directory and making
use of them for `--suppressions`-options.
The code is rather simple when the I/O errors can be disregarded (as it
is done at the moment). I've opted to panic if the temporary directory
cannot be created, as this should almost never happen.
Previously, the Rust `valgrind`-module did write each file contents into a own file into the temporary directory. Due to the file names being lost in the build-script, the names were generated numbers and thus not helpful at all. Therefore this commit replaces those multiple files with a single file containing _all_ suppressions. This also allows to create a temporary file instead of a temporary directory and to only use a single suppression-option to valgrind.
Previously, the build-script generated a slice of strings, which the application needed to concatenate together at runtime. This is slow and entirely unnecessary, as the code generator can simply output the format required by the application. Therefore this commit replaced the list of strings with a single long string, that is embedded in the binary, removing the need for any run- time modifications of the string (it can be written directly to the tem- porary file).
jfrimmel
force-pushed
the
rust-std-suppressions
branch
from
60470a0 to
f42829d
Compare
|
Interestingly, the GitHub (online) rebase does not contain the commit signature of myself, hence I rebased locally again to have the commit verifier. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background
Valgrind may detect leaks in the Rust standard library as it has done in the past (e.g. here and here). Those "leaks" reported by Valgrind are not considered as leaks by the Rust team when they are small permanent allocation, which is not growing (as described here) and there is no guarantee, that the Rust standard library is free of memory leaks (as described here). Therefore some reports of Valgrind are entirely non-actionable by a user of this crate.
In order to solve this, this PR addds a list of suppressions for the Rust
std. Those are applied automatically, so that those "leaks" are never reported to the user. New suppressions should be added when leaks are detected in the periodic test against the beta compiler, but only after reporting them to the Rust team as documented in the newsuppressions/README.md.The implementation is conceptually simple, but a bit tricky in the implementation: the suppression files are added to the
suppressions-di- rectory and should be used bycargo-valgrindwhen running. But that directory is not available at runtime. Therefore the files need to be embedded into the binary. This is done by two co-working parts:suppressions-folder and writes their contents to a constant stringSUPPRESSIONSand--suppressions-option.Fixes #116.
Fixes #111.
Fixes #107.