Skip to content

Conversation

@celinval
Copy link
Contributor

Description of changes:

Refactor how we were creating the rlib files in the link stage. We cannot invoke rustc's linker directly for the native types since they will call the native linker that will fail. Instead, we just manually create an rlib file that includes only a metadata file.

I cleaned up our archive builder since we no longer need to conform to rustc's interface.

Resolved issues:

Resolves #1915

Related RFC:

Call-outs:

Testing:

  • How is this change tested? Modified tests

  • Is this a refactor change? Kinda

Checklist

  • Each commit message has a non-empty body, explaining why the change was made
  • Methods or procedures are documented
  • Regression or unit tests are included, or existing tests cover the modified code
  • My PR is restricted to a single feature or bugfix

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.

@celinval celinval requested a review from a team as a code owner December 23, 2022 20:13
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens in the case of (false, true)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For (false, true), if verbose, we will print that the current target was skipped due to unsupported type. Otherwise, we will silently ignore the target.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that the behavior hasn't changed here and it is still covered by this test: https://github.com/model-checking/kani/tree/main/tests/cargo-ui/unsupported-lib-types/proc-macro

Copy link
Contributor

Choose a reason for hiding this comment

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

In my todo list I have a link to https://github.com/rust-lang/rust/pull/97485/files

I haven't spent the time looking, but while you're here, it looked like maybe there was now a reusable archive.rs in codegen_ssa, I was hoping we could delete this file. Got a chance to look?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't have access to this yet, since we are still using an older version of rustc. But once we do update to a newer version, I agree that we should be able to completely remove this file and just use ArArchiveBuilder directly.

Refactor how we were creating the `rlib` files in the link stage. We
cannot invoke rustc's linker directly for the native types since they
will call the native linker that will fail. Instead, we just manually
create an `rlib` file that includes only a `metadata` file.

I cleaned up our archive builder since we no longer need to conform to
rustc's interface.
One library target can have multiple crate-types, but they should only
be aggregated in one compilation target.
@celinval celinval merged commit a6c21c4 into model-checking:main Jan 4, 2023
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

Successfully merging this pull request may close these issues.

Add support to libraries with cdylib type

3 participants