hexagon: decouple time64 types from musl symbol redirects#5040
Conversation
|
Should we run tests for hexagon in CI? It's a tier 3 target, but one that is now used in (aside: maybe it's time to upgrade it to tier 2?) |
I am fine with those - I can make the MCP if desired. |
|
I think that we can just add tests here for targets that we believe are useful. That is what we do on But separately at least to me it seems like hexagon is at a point where tier 2 feels right. |
|
I don't think there is anything specific about this target that couldn't apply to any newer 32-bit targets, so I'd prefer not to use The issue may be due to us conflating two distinct meanings of Also how is Hexagon configured in musl? Grepping for "hexagon" yields no results. |
Anyone maintaining a r-l repo that tests T3 targets needs to be prepared to disable those tests when things break; please do not consider libc to be blocking anything here. Regarding libc testing: I am not interested in adding anything beyond the current build check for T3 targets. We can't really feasibly maintain CI for anything that may or may not have active maintenance, which is what the target list is meant to indicate. If target maintainers have the need, interest, and available effort to stand behind more testing, a T2 bump is the right thing to indicate that (which I do think would be reasonable enough here). |
0383fea to
7f435df
Compare
Makes sense - updated w/this suggestion.
Yeah - https://github.com/quic/musl/tree/bcain/to-upstream is our fork. We'll keep working on upstreaming that. |
7f435df to
8910d50
Compare
|
I think I am encountering rust-lang/rust#154818 causing the CI failure in |
8910d50 to
3467c35
Compare
The `musl32_time64` cfg previously conflated two distinct concepts: 1. Type definitions: `time_t` is `i64`, `suseconds_t` is `i64`, `timespec` has padding — applies to all 32-bit musl v1.2.3+ targets including hexagon. 2. Symbol redirects: `clock_gettime` → `__clock_gettime64` etc., corresponding to musl's `_REDIR_TIME64` — applies only to arm, mips, powerpc, and x86. Hexagon was added to musl after the time64 transition and never had a 32-bit `time_t`, so its libc exports `clock_gettime` directly with no `__*_time64` symbols. Applying the link-name redirects caused undefined symbol errors at link time (rust-lang/rust#154686). Introduce a new `musl_redir_time64` cfg for the symbol redirects and restrict it to arches that define `_REDIR_TIME64`. Keep `musl32_time64` for the type/struct meaning, now set generically for all 32-bit musl v1.2.3+ targets (removing explicit `target_arch = "hexagon"` conditions).
3467c35 to
b86b191
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. |
|
Thanks for your help @tgross35 Since the regression I introduced w/the previous Of course, hexagon-unknown-linux-musl is a tier 3 target so there's no urgency. But I'm wondering what the roles are - can I / should I request a new |
|
Totally fine to poke me on a release, I'm planning to do one ~tomorrow (I'll ping you once it's out). Anybody can do the r-l/rust update, usually I don't but am happy to review :) |
The `musl32_time64` cfg previously conflated two distinct concepts: 1. Type definitions: `time_t` is `i64`, `suseconds_t` is `i64`, `timespec` has padding — applies to all 32-bit musl v1.2.3+ targets including hexagon. 2. Symbol redirects: `clock_gettime` → `__clock_gettime64` etc., corresponding to musl's `_REDIR_TIME64` — applies only to arm, mips, powerpc, and x86. Hexagon was added to musl after the time64 transition and never had a 32-bit `time_t`, so its libc exports `clock_gettime` directly with no `__*_time64` symbols. Applying the link-name redirects caused undefined symbol errors at link time (rust-lang/rust#154686). Introduce a new `musl_redir_time64` cfg for the symbol redirects and restrict it to arches that define `_REDIR_TIME64`. Keep `musl32_time64` for the type/struct meaning, now set generically for all 32-bit musl v1.2.3+ targets (removing explicit `target_arch = "hexagon"` conditions). (backport <rust-lang#5040>) (cherry picked from commit a6b660c)
The `musl32_time64` cfg previously conflated two distinct concepts: 1. Type definitions: `time_t` is `i64`, `suseconds_t` is `i64`, `timespec` has padding — applies to all 32-bit musl v1.2.3+ targets including hexagon. 2. Symbol redirects: `clock_gettime` → `__clock_gettime64` etc., corresponding to musl's `_REDIR_TIME64` — applies only to arm, mips, powerpc, and x86. Hexagon was added to musl after the time64 transition and never had a 32-bit `time_t`, so its libc exports `clock_gettime` directly with no `__*_time64` symbols. Applying the link-name redirects caused undefined symbol errors at link time (rust-lang/rust#154686). Introduce a new `musl_redir_time64` cfg for the symbol redirects and restrict it to arches that define `_REDIR_TIME64`. Keep `musl32_time64` for the type/struct meaning, now set generically for all 32-bit musl v1.2.3+ targets (removing explicit `target_arch = "hexagon"` conditions). (backport <#5040>) (cherry picked from commit a6b660c)
|
Just released this as 0.2.185, feel free to submit a PR for rustc https://github.com/rust-lang/libc/releases/tag/0.2.185. |
|
Thanks @folkertdev - sorry for not getting to it sooner |
Hexagon was added to musl after the time64 transition and never had a 32-bit time_t ABI, so its libc exports
clock_gettime(not__clock_gettime64) and has no__*time64redirect symbols. Remove hexagon from MUSL_REDIR_TIME64_ARCHES to stop applying ~45 link_name redirects that cause undefined symbol errors at link time.The correct 64-bit time_t/suseconds_t types and timespec padding are preserved via direct target_arch = "hexagon" conditions, and linux_time_bits64 is set separately for input_event layout.
Description
Sources
Checklist
libc-test/semverhave been updated*LASTor*MAXareincluded (see #3131)
cd libc-test && cargo test --target mytarget);especially relevant for platforms that may not be checked in CI