-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Add Cosmopolitan Libc target #142609
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
base: master
Are you sure you want to change the base?
Add Cosmopolitan Libc target #142609
Conversation
This comment has been minimized.
This comment has been minimized.
The job Click to see the possible cause of the failure (guessed by this bot)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, my patch isn’t perfect. It can’t handle some anonymous structs,
enum
s,const int
s, or amazing things like usingSIGTERM
as an array index within an initializer. Rare compiler crashes may still occur in some weird static initializations, or if you try stuffingSIGTERM
into astatic const int8_t
. But I’ve spent a good chunk of time removing obvious counterexamples, and a lot of popular software builds seamlessly. A stringent testing setup will reveal more things to improve.
If that's "seamlessly", then I would hate to know what is "with some difficulty".
I do not think we really want to add a target where none of libc is allowed in const eval, certainly not like this. This is Rust, not C++, and CTFE is a rule when in const {}
or any other const context, not a suggestion like it is in constexpr
. Many crates do something like this:
const NOT_FOUND: u32 = libc::ENOENT as _;
and then we can't evaluate that, so you're not getting much to begin with. It isn't very common for any one libc symbol, but when considering all the constants that are redefined, it adds up to quite a lot.
And that's assuming that those symbols are even sound, which may be an overly generous assumption.
x if x == libc::E2BIG => ArgumentListTooLong, | ||
x if x == libc::EADDRINUSE => AddrInUse, | ||
x if x == libc::EADDRNOTAVAIL => AddrNotAvailable, | ||
x if x == libc::EBUSY => ResourceBusy, | ||
x if x == libc::ECONNABORTED => ConnectionAborted, | ||
x if x == libc::ECONNREFUSED => ConnectionRefused, | ||
x if x == libc::ECONNRESET => ConnectionReset, | ||
x if x == libc::EDEADLK => Deadlock, | ||
x if x == libc::EDQUOT => QuotaExceeded, | ||
x if x == libc::EEXIST => AlreadyExists, | ||
x if x == libc::EFBIG => FileTooLarge, | ||
x if x == libc::EHOSTUNREACH => HostUnreachable, | ||
x if x == libc::EINTR => Interrupted, | ||
x if x == libc::EINVAL => InvalidInput, | ||
x if x == libc::EISDIR => IsADirectory, | ||
x if x == libc::ELOOP => FilesystemLoop, | ||
x if x == libc::ENOENT => NotFound, | ||
x if x == libc::ENOMEM => OutOfMemory, | ||
x if x == libc::ENOSPC => StorageFull, | ||
x if x == libc::ENOSYS => Unsupported, | ||
x if x == libc::EMLINK => TooManyLinks, | ||
x if x == libc::ENAMETOOLONG => InvalidFilename, | ||
x if x == libc::ENETDOWN => NetworkDown, | ||
x if x == libc::ENETUNREACH => NetworkUnreachable, | ||
x if x == libc::ENOTCONN => NotConnected, | ||
x if x == libc::ENOTDIR => NotADirectory, | ||
#[cfg(not(target_os = "aix"))] | ||
libc::ENOTEMPTY => DirectoryNotEmpty, | ||
libc::EPIPE => BrokenPipe, | ||
libc::EROFS => ReadOnlyFilesystem, | ||
libc::ESPIPE => NotSeekable, | ||
libc::ESTALE => StaleNetworkFileHandle, | ||
libc::ETIMEDOUT => TimedOut, | ||
libc::ETXTBSY => ExecutableFileBusy, | ||
libc::EXDEV => CrossesDevices, | ||
libc::EINPROGRESS => InProgress, | ||
libc::EOPNOTSUPP => Unsupported, | ||
|
||
libc::EACCES | libc::EPERM => PermissionDenied, | ||
x if x == libc::ENOTEMPTY => DirectoryNotEmpty, | ||
x if x == libc::EPIPE => BrokenPipe, | ||
x if x == libc::EROFS => ReadOnlyFilesystem, | ||
x if x == libc::ESPIPE => NotSeekable, | ||
x if x == libc::ESTALE => StaleNetworkFileHandle, | ||
x if x == libc::ETIMEDOUT => TimedOut, | ||
x if x == libc::ETXTBSY => ExecutableFileBusy, | ||
x if x == libc::EXDEV => CrossesDevices, | ||
x if x == libc::EINPROGRESS => InProgress, | ||
x if x == libc::EOPNOTSUPP => Unsupported, | ||
|
||
x if x == libc::EACCES || x == libc::EPERM => PermissionDenied, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I think not.
The correctness losses here are unacceptable, because changing these to be symbolic, even if it could be done automatically, means that we no longer can benefit from exhaustiveness checking. Thus we cannot do things such as determining if the current match arm is covered by a previous one, or etc., requiring manual rechecking on every single review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This need not be added for new targets.
let program = self.get_program_cstr(); | ||
let argv = self.get_argv().as_ptr(); | ||
#[cfg(target_os = "cosmo")] | ||
if libc::IsWindows() && !program.to_bytes().ends_with(b".exe") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think we want more single-target-specific functions inside the libc
crate, either.
no_default_libraries: true, | ||
max_atomic_width: Some(64), | ||
linker_flavor: LinkerFlavor::Gnu(Cc::Yes, Lld::No), | ||
stack_probes: StackProbeType::None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't this use inline stack probes?
metadata: TargetMetadata { | ||
description: Some("64-bit Cosmopolitan Libc".into()), | ||
tier: Some(3), | ||
host_tools: Some(true), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
host_tools: Some(true), | |
host_tools: Some(false), |
I'm pretty sure rustc can't be compiled for this based on what @workingjubilee already noted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stray file
debug_assert_eq!(res, 0); | ||
cfg_if::cfg_if! { | ||
if #[cfg(target_os = "cosmo")] { | ||
let _ = res; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change?
@rustbot concern libc-system-constants-no-longer-const I concur with @workingjubilee, loosing the constness of
If gcc didn't wanted it, then it's not really a solution. I would very much want to see a proper and accepted solution in C before attempting any port in Rust. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(picking a random file to get an inline comment) new targets should land as no_std
first. Once we have that to indicate acceptance in rustc, other ecosystem updates can be considered.
Though maybe, if accepted, this is just easier to support as a no_std target indefinitely? That sidesteps any concerns with libc
or hacks in std
. It would be possible to publish a cosmo-std
crate that drops into the std
APIs that are well supported.
This PR adds support for Cosmopolitan Libc, building on prior work from @ahgamut. It depends on support in the libc crate.
Cosmopolitan is a unique target, because some of the system constants (e.g.
EINVAL
,MAP_ANONYMOUS
,O_CLOEXEC
) are actually global variables that are set to the correct values at runtime based on the detected OS. This means unfortunately any pattern matching on them likelibc::EINVAL => {}
must be changed to useif
guards instead likee if e == libc::EINVAL => {}
. For switch statements in C, the solution was a gcc patch that wasn't upstreamed, so I'm not sure if it makes sense for Rust to upstream this, but I figured I would submit a PR anyway just to make it easier to find.The code is also a bit messy right now, but it works surprisingly well in its current state.
Testing
Building tests
I built the library tests with
Then converted the test runners from ELF executables into APE executables with
apelink
apelink -o test.com -l $COSMO/.cosmocc/3.9.2/bin/ape-x86_64.elf std-04db952a3d57c5d1.com.dbg
Then copied the APE test runners to different supported platforms and ran them.
Current test results
Caution
Concerns (1 active)
Managed by
@rustbot
—see help for details.