Skip to content

Be #1472

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Be #1472

wants to merge 6 commits into from

Conversation

WorksButNotTested
Copy link
Contributor

  • Configure rustix to use the linux_raw backend on armeb-unknown-linux-gnueabi and aarch64_be-unknown-linux-gnu targets. (Both of these are tier 3 rust platforms and hence no public binaries are provided).
  • Since rustix uses tempfile for its unit test, which in turn uses rustix as a dependency we have a cyclic dependency. Change the Cargo.toml so that the version of rustix used to satisfy this tempfile dependency is the working copy.

@WorksButNotTested
Copy link
Contributor Author

@sunfishcode Grateful for your thoughts on this.

@WorksButNotTested
Copy link
Contributor Author

Seems the freebsd test failure issue is unrelated? This workflow appears to have run successfully.
https://github.com/bytecodealliance/rustix/actions/runs/15474936766

A few of the other commits also adress issues caused by the release of new crates which require a version of rust higher than the MSRV.

Lastly, there is a commit to fix an issue whereby constants guarded by the stdio feature are used within the tests of the fs feature although there is no dependency between those features.

# the same consistent version. We therefore need to use a version of tempfile
# for which its rustix dependency is a semver match for our current version.
[patch.crates-io]
rustix = { path = "." }
Copy link
Contributor

Choose a reason for hiding this comment

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

This will make it impossible to do a semver incompatible release of rustix as there is no tempfile version that has a semver compatible dependency on rustix yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't really see a good solution to this problem. I guess the real problem is that there is a cyclic dependency, but the two projects are both under separate ownership. The only clean solution I can see is to remove that dependency and trty to find an alternative crate. The change is outside the scope of my original PR really, I just wanted to try and get the CI working to ensure I didn't introduce any regressions. But it seems unfortunately that new releases of other crates change the result of the CI. Happy to take advice on how to proceed here.

Copy link
Contributor

Choose a reason for hiding this comment

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

One option would be to keep the big-endian CI disabled for now and only enable it once there is a new rustix release.

Copy link
Member

Choose a reason for hiding this comment

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

tempfile's use of rustix is not part of rustix's testsuite. We don't need to make tempfile's version of rustix match the version of rustix using tempfile for tests. They can differ and it's fine.

Copy link
Member

Choose a reason for hiding this comment

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

So to be clear, I think it's best to omit this part of the patch.

@@ -286,11 +286,11 @@ pub(crate) const SIGSYS: c_int = linux_raw_sys::general::SIGSYS as _;
))]
pub(crate) const SIGEMT: c_int = linux_raw_sys::general::SIGEMT as _;

#[cfg(feature = "stdio")]
#[cfg(any(test, feature = "stdio"))]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These constants are used in the tests here and here. Not related to the purpose of my change, but trying to get the CI into life to check for any regressions.

Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding cfg(test) here, please either put the test code that uses STDIN_FILENO under cfg(feature = "stdio"), or define STDIN_FILENO here also when cfg(feature = "fs").

@WorksButNotTested
Copy link
Contributor Author

@bjorn3 Thanks for the feedback. Just let me know how best to proceed and I'm happy to make any changes.

@sunfishcode
Copy link
Member

How much testing have you done with the linux_raw backend on these targets? There are some places in the Linux syscall ABI which are sensitive to endianness, such as when 32-bit values are split into two halves, so it'd be good to have some assurance that this code is working before enabling it on these targets.

@WorksButNotTested
Copy link
Contributor Author

WorksButNotTested commented Jun 20, 2025

When I ran the tests locally, there were a handful of failures. But ideally I'd like to add CI for these targets, the only thing is that it requires:

  • An armbe-linux-gnueabi- and aarch64_be-linux-gnu- cross compiler (since we would need a linker) and neither the standard linux distros, nor ARM themselves provide a pre-built one ready to use.
  • Either:
    • A build of qemu-system supporting the relevant targets with accompanying initrd and kernel.
    • A build of qemu-user supporting the relevant targets.

I have already made most of these for some other projects, but wondered what if anything would be the best approach.

  • There are containers with cross-compilers as part of frida's docker-images repo for armbe and aarch64_be. These could either be used as is, or duplicated into the repo.
  • There is also a container with qemu-system, kernel and initrd here. Again this could be either used as is, or duplicated. The workflow for running this sort of setup is a little clunky and would look a bit like this.
  • Building qemu from source to support qemu-armbe and qemu-aarch64_be isn't too hard, so again a container could be made for that, or it could be built and installed as part of a CI job (though likely some form of caching would be useful to speed up this job).

The consideration is in deciding whether to use resources from other projects or whether to replicate the same functionality to give more control. Then of course there is the additional cost in terms of resources and time for the CI.

I'm happy to go away and start working on a solution if you think it would be beneficial, but would be grateful if you have some advice on which direction to take.

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.

3 participants