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

cargo-valgrind reports "possibly lost" and "still reachable" as errors #105

Open
lnicola opened this issue Nov 28, 2024 · 3 comments
Open

Comments

@lnicola
Copy link

lnicola commented Nov 28, 2024

..while valgrind doesn't unless you use --leak-check=full. This caused a bit of kerfuffle wrt. rust-lang/rust#133574.

PS: valgrind --xml=yes --gen-suppressions=all doesn't seem to output suppressions which makes it really painful when you need to generate suppressions within cargo valgrind test.

@jfrimmel
Copy link
Owner

Thank you for your report. Yes, currently this tool always sums the number of reported blocks. I understand, that this might not be, what all users want.

Ho would you like the output of the tool to be? Or do you suggest additional command line flags to toggle some behavior?

@lnicola
Copy link
Author

lnicola commented Nov 28, 2024

So the XML has elements like <kind>Leak_PossiblyLost</kind>, which can be used to filter out the different types. I think it would be good to match the default Valgrind behavior (which only reports "definitely lost", I think?), and to have a flag.

Ideally, VALGRINDFLAGS="--leak-check=full" would work to enable these back, but you can see why it won't.

@workingjubilee
Copy link

workingjubilee commented Nov 28, 2024

cargo valgrind could pass --errors-for-leak-kinds=definite,indirect, for instance, or expose the ability to control which categories are errors.

jfrimmel added a commit that referenced this issue Nov 30, 2024
Unfortunately, some new versions may change behavior, e.g. clippy may
add lints, which break the build. Therefore it makes sense, to pin
certain error prone CI-jobs to some specific versions. They should be
bumped regularly, though.

Currently, the following jobs are pinned:
1. the `ci/test`-job: as shown in #105, Rust 1.83 has introduced a valid
   "leak", which fails our tests for now. Therefore this job is pinned
   to 1.82, where this issue doesn't occur. A proper fix is necessary in
   the future (#107).
2. `clippy`: the latest [PR broke the clippy job][clippy-job] due to new
   lints being fired. Therefore the CI is pinned to 1.83 with the issues
   fixed.

[clippy-job]: https://github.com/jfrimmel/cargo-valgrind/actions/runs/12095283463/job/33728289241
jfrimmel added a commit that referenced this issue Nov 30, 2024
Unfortunately, some new versions may change behavior, e.g. clippy may
add lints, which break the build. Therefore it makes sense, to pin
certain error prone CI-jobs to some specific versions. They should be
bumped regularly, though.

Currently, the following jobs are pinned:
1. the `ci/test`-job: as shown in #105, Rust 1.83 has introduced a valid
   "leak", which fails our tests for now. Therefore this job is pinned
   to 1.82, where this issue doesn't occur. A proper fix is necessary in
   the future (#107).
2. `clippy`: the latest [PR broke the clippy job][clippy-job] due to new
   lints being fired. Therefore the CI is pinned to 1.83 with the issues
   fixed.

[clippy-job]: https://github.com/jfrimmel/cargo-valgrind/actions/runs/12095283463/job/33728289241
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants