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

c2rust-analyze: Add a cargo wrapper #1036

Merged
merged 6 commits into from
Nov 3, 2023
Merged

Conversation

kkysen
Copy link
Contributor

@kkysen kkysen commented Nov 1, 2023

This adds an MVP of a cargo wrapper for c2rust-analyze so that it can be run on a whole crate like cargo. This copies the approach from c2rust-instrument, with some minor adjustments:

  • We don't care about the instrumentation and metadata c2rust-instrument need, so that code is gone.
  • We still allow c2rust-analyze to be called as a rustc_wrapper directly. More specifically, the cargo wrapper is supposed to set RUST_SYSROOT. c2rust-instrument requires this, while c2rust-analyze will re-calculate it if it wasn't.

This allows us to keep using the rustc wrapper in tests,
as the tests are all set up as single files meant to be compiled with rustc directly.
We can change this, but that'll come later. The exception is the lighttpd_minimal and with_pdg_file tests, as those run on full crates. I converted lighttpd_minimal to use the cargo wrapper as a proof of concept to show it works. I tried to do the same for with_pdg_file, but it updated some of the hashes or something so now the PDG binary has out-of-date DefPathHashes, and I'm not sure how to regenerate a new one.

@kkysen
Copy link
Contributor Author

kkysen commented Nov 1, 2023

@fw-immunant, how'd you generate the reference_pdg.bc? I'd like to regenerate it so it uses up-to-date DefPathHashes, as I think dependency hashes got updated from the ones hardcoded in the test.

@kkysen kkysen force-pushed the kkysen/analyze-cargo-wrapper branch from d62b187 to afd03d3 Compare November 1, 2023 10:35
Copy link
Contributor

@aneksteind aneksteind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once comments are addressed. Where there is ambiguity I would prefer the documentation be changed, not just PR comments to clarify.

Base automatically changed from kkysen/analyze-separate-main-mod to master November 1, 2023 16:38
@kkysen kkysen force-pushed the kkysen/analyze-cargo-wrapper branch from afd03d3 to c05209b Compare November 3, 2023 06:35
It was tricky to get exactly the right ones with the right MSRV.
…rgo` on a whole crate.

This copies the approach from `c2rust-instrument`.

The main difference are:
* We don't care about the instrumentation and metadata `c2rust-instrument` need.
* We still allow `c2rust-analyze` to be called as a `rustc_wrapper` directly.
  More specifically, the `cargo` wrapper is supposed to set `RUST_SYSROOT`.
  `c2rust-instrument` requires this, while `c2rust-analyze` will re-calculate it if it wasn't.

This allows us to keep using the `rustc` wrapper in tests,
as the tests are all set up as single files meant to be compiled with `rustc` directly.
We can change this, but that'll come later.
For full crates we test like `lighttpd-minimal`, we'll definitely switch to the `cargo` wrapper,
at the very least for a proof of concept.
…e the `cargo` wrapper.

This is done mostly as a proof of concept for now.
Ideally we'd want to add it as a method on `Analyze`/`FileCheck` and handle error reporting better.

Now that `lighttpd-minimal` is run with `cargo`, we don't need the `extern crate libc;` anymore.
@kkysen kkysen force-pushed the kkysen/analyze-cargo-wrapper branch from c05209b to f071ea7 Compare November 3, 2023 06:44
@kkysen kkysen merged commit 431847a into master Nov 3, 2023
7 of 9 checks passed
@kkysen kkysen deleted the kkysen/analyze-cargo-wrapper branch November 3, 2023 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants