std: add platform support for hexagon-unknown-qurt#152084
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
791d71f to
3201966
Compare
|
This PR depends on a newer version of the |
This comment has been minimized.
This comment has been minimized.
You need |
|
@rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
The commit I need - rust-lang/libc@9934c04 is not yet tagged/released. Or rather there is a |
This comment has been minimized.
This comment has been minimized.
3201966 to
5adcafb
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Ah - I see now that it appears in |
This comment has been minimized.
This comment has been minimized.
5adcafb to
caab9c9
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
caab9c9 to
3749f50
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I've updated the PR description like so:
|
| @@ -114,7 +114,7 @@ pub(crate) fn get_result_from_exit_code( | |||
| Some(TR_OK) => TestResult::TrOk, | |||
| #[cfg(windows)] | |||
| Some(STATUS_FAIL_FAST_EXCEPTION) => TestResult::TrFailed, | |||
| #[cfg(unix)] | |||
| #[cfg(all(unix, not(target_os = "qurt")))] | |||
There was a problem hiding this comment.
So, is qurt unix or not for cfg purposes? This seems to not be consistent throughout the PR
There was a problem hiding this comment.
Sorry, I'll make it consistent.
| @@ -1,6 +1,11 @@ | |||
| #![forbid(unsafe_op_in_unsafe_fn)] | |||
|
|
|||
| cfg_select! { | |||
| // QuRT uses unsupported pipes since it doesn't support process spawning | |||
| target_os = "qurt" => { | |||
There was a problem hiding this comment.
This is no longer needed since qurt is no longer unix.
|
The conclusion from today's libs meeting was that this target is (given what we're seeing) Not Unix, but properly defining what is unix is best left for a refactor of the cfgs we expose |
aa6be17 to
45894d0
Compare
This comment has been minimized.
This comment has been minimized.
45894d0 to
2db6d1f
Compare
This comment has been minimized.
This comment has been minimized.
2db6d1f to
091e154
Compare
| - **`thread::yield_now()`**: Calls `sched_yield` which is stubbed to `abort`; | ||
| use `thread::sleep(Duration::from_millis(0))` as an alternative |
There was a problem hiding this comment.
Couldn't std do that? yield_now aborting seems bad, even having it be a noop is better.
| @@ -227,6 +231,7 @@ impl<'a> AsRawFd for io::StdinLock<'a> { | |||
| } | |||
|
|
|||
| #[stable(feature = "asraw_stdio_locks", since = "1.35.0")] | |||
| #[cfg(not(target_os = "qurt"))] | |||
There was a problem hiding this comment.
But stdio is supported, right? Why wouldn't this be available?
There was a problem hiding this comment.
This isn't used anymore, right? If so, then could you remove this change please?
| @@ -3,6 +3,10 @@ use crate::fmt; | |||
|
|
|||
| pub struct Args {} | |||
|
|
|||
| pub unsafe fn init(_argc: isize, _argv: *const *const u8) { | |||
There was a problem hiding this comment.
This should be gated for QuRT, otherwise other unsupported targets will complain about the unused function.
| @@ -124,6 +129,7 @@ pub fn set_permissions(path: &Path, perm: FilePermissions) -> io::Result<()> { | |||
| pub fn set_permissions_nofollow(path: &Path, perm: crate::fs::Permissions) -> io::Result<()> { | |||
| use crate::fs::OpenOptions; | |||
|
|
|||
| #[cfg_attr(any(target_os = "espidf", target_os = "horizon"), allow(unused_mut))] | |||
There was a problem hiding this comment.
Thanks! I suggest #[expect] instead of #[allow], it means we'll notice if the attribute becomes unnecessary.
There was a problem hiding this comment.
Please undo this change, this is no longer needed.
There was a problem hiding this comment.
Insert the config here instead of adding a manual fallback.
| #[cfg(target_os = "qurt")] | ||
| pub fn set_name(_name: &CStr) { | ||
| // QuRT doesn't support pthread_setname_np | ||
| } | ||
|
|
There was a problem hiding this comment.
| #[cfg(target_os = "qurt")] | |
| pub fn set_name(_name: &CStr) { | |
| // QuRT doesn't support pthread_setname_np | |
| } |
| -C "link-args=${QURTLIB}/crt1.o ${HEXLIB}/crt0.o ${HEXLIB}/init.o ${QURTLIB}/debugmon.o" \ | ||
| -C "link-args=-Wl,--section-start=.start=0x40000" \ | ||
| -C "link-args=-Wl,--defsym=sched_yield=abort" \ | ||
| -C "link-args=-Wl,--defsym=unsetenv=abort" \ |
There was a problem hiding this comment.
Making remove_env panic seems like a much nicer way of handling this.
| @@ -31,9 +31,9 @@ cfg_select! { | |||
| unix, | |||
| windows, | |||
| target_os = "psp", | |||
| target_os = "qurt", | |||
There was a problem hiding this comment.
You might be able to get rid of the weird symbol redefinitions by not linking to them here and just aborting in panic_unwind instead.
There are a number of other targets that don't support unwinding either, I suggest you have a look at the Hermit implementation in particular.
This comment has been minimized.
This comment has been minimized.
Remove unix family from hexagon-unknown-qurt target spec and instead explicitly opt into unix implementations where applicable, following the pattern used by similar unix-adjacent targets. The target uses the unix PAL and explicitly selects unix implementations for the supported features. Modules that remain unsupported for qurt: process spawning, networking and pipes.
Update documentation with details about how to leverage qurt, now that there's some level of libstd support
Remove spurious #[allow(unused_imports)] added before `use super::platform::fs::MetadataExt as _` in os/unix/fs.rs. This import is used and the annotation is not needed.
Remove #[cfg(not(target_os = "qurt"))] guard on getppid() in sys/process/unix/common.rs. QuRT's libc provides getpid/getppid, so no special-casing is needed.
Remove target_os = "qurt" from the libunwind branch in library/unwind/src/lib.rs. QuRT's _Unwind_* symbols are provided by libc_eh (linked via build.rs), not by Rust's in-tree libunwind. panic_unwind for qurt falls through to dummy.rs which aborts, so no libunwind symbols are needed from this crate.
Move qurt's set_name no-op from thread/unix.rs into thread/mod.rs by adding target_os = "qurt" to the cfg lists that select unsupported::set_name. This is the canonical pattern used by other targets (newlib, l4re, wasi, etc.) that don't support thread naming, rather than adding a per-OS stub function in unix.rs.
Make unsetenv panic for qurt rather than calling libc::unsetenv, which is not available. This causes std::env::remove_var to panic on qurt, which is the correct behaviour for an unsupported operation. The linker stub --defsym=unsetenv=abort is no longer needed.
Remove qurt exclusions from AsRawFd impls for Stdin/Stdout/Stderr and their lock types in os/fd/raw.rs. QuRT's libc defines STDIN_FILENO, STDOUT_FILENO and STDERR_FILENO, and stdio is functional on this platform, so these impls should be available.
Gate the no-op args::init in sys/args/unsupported.rs with #[cfg(target_os = "qurt")]. Without this, all other targets using the unsupported args module would see an unused function warning. Only qurt needs it there because it uses the unix PAL (which calls args::init) but is not in the unix family and so falls through to the unsupported args module.
Change #[allow(unused_mut)] to #[expect(unused_mut)] for the set_permissions_nofollow options binding, so the compiler will warn if the attribute becomes unnecessary in the future.
Remove as_raw_stat() from qurt's MetadataExt trait and its raw::stat struct definition. as_raw_stat() has been deprecated since 1.8.0 in favour of the individual accessor methods; adding it to a brand-new platform only to immediately deprecate it is unnecessary. This follows the pattern of newer platforms like AIX that only expose the metadata_ext2 accessors.
Update docs to reflect the sys-layer fixes: - remove_var now panics rather than silently failing (unsetenv is not available; the panic happens at the sys layer so no linker stub needed) - yield_now is implemented as sleep(Duration::ZERO); sched_yield linker stub no longer needed; remove the known-limitation entry entirely - Remove the yield_now and unsetenv --defsym stubs from both the build.rs and rustc direct-invocation examples
Fix compilation after rebase: - ffi.rs: migrate sealed traits to pub impl(self) syntax - fd/unix.rs: add u8 type parameter to BorrowedCursor - fs/unix.rs: remove redundant cfg guard on lock() methods - unwind/lib.rs: explicitly list qurt for libunwind (no longer in unix family)
091e154 to
d605089
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
View all comments
QuRT is not a unix target but provides POSIX-compatible threading, file I/O, and synchronization APIs. This implementation does not declare unix as a target family. Instead, it explicitly opts into the unix PAL and selects unix-based implementations for individual subsystems where QuRT's POSIX layer is sufficient.
Supported: alloc, threading, mutex, condvar, rwlock, once, thread parking, TLS, file system, file descriptors, stdio, I/O slices, I/O errors, is_terminal, env, paths, time, exit, personality/unwinding, backtrace
Stubbed; returns Err: process spawning, networking, pipes, hostname
Stubbed; returns empty: command-line args
Stubbed; panics: random
Platform modules: