Fix non-intentional memory leaks in analysis/test
#681
Assignees
Comments
This was referenced Sep 25, 2022
kkysen
added a commit
that referenced
this issue
Sep 28, 2022
Fixes #681. This fixes all of the memory leaks in `analysis/test`. There was one that was intentional (`no_owner(0)`), but I freed that in `main_0` immediately after the `no_owner(0)` call instead.
kkysen
added a commit
that referenced
this issue
Oct 10, 2022
Fixes #685. This makes `analysis/test` `miri`-compatible when running with `--features miri`. This is done by using a monomorphic `printf` shim, since `miri` can't handle variadic functions like `printf`. Since all uses of `printf` in `analysis/test` are monomorphic (they all have the same format string), we can substitute a `fn printf` that is non-`extern "C"`, non-variadic (and thus `miri`-compatible), and that still has the same behavior for its call sites. Then we add a test in `c2rust-pdg` that runs `miri` on `analysis/test` to ensure it stays UB-free. However, we don't yet run this test by default (it's `#[ignore]`d for now) as there are issues with running `miri` in CI (it installs `xargo` every time and I'm getting a permission denied error (not sure from quite what exactly), and it'd be better to install `xargo` upfront, not on every run). Thus, I'm `#[ignore]`ing it for now in 4152d34. We can get it to run in CI correctly later in another PR, but I want to merge this now and avoid over-complicating it here. The test can still be manually run with `cargo test -p c2rust-pdg -- --ignored analysis_test_miri`. See #698 for the tracking issue to re-enable this test by default. The new test passing is blocked on: * #683 (f ixes #680) * #684 (f ixes #681) It would also be nice to f ix #682, but that's not completely necessary for this (though it would create a much less noisy output).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This is related to #680 (fixing UB). There are also a bunch of non-intentional memory leaks in
analysis/test. These are not UB, so they shouldn't be causing any unexpected behavior, but there's no good reason to have them, and they clutter up themirioutput when looking for UB. We should only keep the intentional leaks that are there for testing how we handle leaks.The text was updated successfully, but these errors were encountered: