Rebase of "Upgrade musl supported version to 1.2.3" #3791

myl7 · 2024-08-05T04:33:03Z

Also discussed in GSoC Zulip. Related: GSoC Idea OSPP Idea OSPP Zulip.

This PR is mainly the rebase of #3068 . So closes #3068 fixes #1848 closes #2088 .
Some other changes are made:

Fix warnings in CI by setting several #[allow(unused_imports)].

For the checklist for review or CI, as the original PR should have passed it, this PR only makes sure ci/style.sh and cd libc-test && cargo test pass

rustbot · 2024-08-05T04:33:08Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @JohnTitor (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

rustbot · 2024-08-05T06:16:43Z

Some changes occurred in solarish module

cc @jclulow, @pfmooney

Some changes occurred in OpenBSD module

cc @semarie

Some changes occurred in OpenBSD module

cc @semarie

semarie · 2024-08-05T06:42:44Z

FYI, no problem on OpenBSD side

tgross35 · 2024-08-12T09:49:17Z

Could you please rebase once #3797 lands? (Hopefully ~1 hour). You will need to drop 2852809 and eb4c4b2.

bors · 2024-08-12T10:32:09Z

☔ The latest upstream changes (presumably #3797) made this pull request unmergeable. Please resolve the merge conflicts.

tgross35 · 2024-08-12T22:38:46Z

@rustbot author

musl 1.1 maintenance has for all practical purposes ended. Accordingly, there is no point in supporting both 1.1 and 1.2 in the libc crate, so follow the time_t type transition to 64-bit.

- add padding members for musl 1.2 - ensure the padding members have an appropriate type (always c_int)

Only some 32-bit targets use the time64 family of functions and that set will not change going forward (new 32-bit targets added will use Y2038 compliant syscalls and types by default). This patch introduces a cfg flag that controls when the time64 abi related changes are enabled and sets that flag from libc and libc-tests' build.rs files.

By using set_cfg() helper.

myl7 · 2024-08-17T04:05:42Z

Glad to see most pass!

The only failure of wasm32-unknown-emscripten is because some code generated by ctest2 does not add struct prefix to struct names.
However, it should be added according to:

libc/libc-test/build.rs

Line 2744 in fc28283

t if is_struct => format!("struct {}", t),

Not sure how to resolve it.

bors · 2024-08-27T19:23:47Z

☔ The latest upstream changes (presumably #3869) made this pull request unmergeable. Please resolve the merge conflicts.

tgross35 · 2024-08-29T00:14:06Z

Are you able to just skip the type with a fixme? I think we have emscripten-specific ignores already. Also not sure why this would be happening, but I am really hoping we don't need to block this on ctest changes.

tgross35 · 2024-08-29T05:18:31Z

Also @kaniini I think you had some ideas about how to do musl better here. If you get a chance, would you mind taking a look at this?

tgross35

I didn't check all the types in depth yet but this looks good at first glance. Please address the feedback (specifically linux_time_bits64) and then I think we can get this in soon.

tgross35 · 2024-11-20T03:55:20Z

build.rs

@@ -17,6 +17,7 @@ const ALLOWED_CFGS: &'static [&'static str] = &[
    "libc_const_extern_fn",
    "libc_const_extern_fn_unstable",
    "libc_deny_warnings",
+    "musl_time64_abi",


Could this be renamed to musl_redir_time64?

Musl uses _REDIR_TIME64 in their source, I think the name is a bit more intuitive because only 32-bit does these hacks (64-bit always has the 64-bit time interface).

tgross35 · 2024-11-20T04:02:50Z

build.rs

+fn is_musl_time64_abi() -> bool {
+    match env::var("TARGET") {
+        Ok(target) => match &target[..] {
+            "arm-unknown-linux-musleabi"
+            | "arm-unknown-linux-musleabihf"
+            | "armv5te-unknown-linux-musleabi"
+            | "armv7-unknown-linux-musleabi"
+            | "armv7-unknown-linux-musleabihf"
+            | "i586-unknown-linux-musl"
+            | "i686-unknown-linux-musl"
+            | "mips-unknown-linux-musl"
+            | "mipsel-unknown-linux-musl"
+            | "powerpc-unknown-linux-musl" => true,
+            _ => false,
+        },
+        Err(_) => false,
+    }
+}


We probably shouldn't match literal targets here since we only care about the prefix and the suffix. Instead could we:

Add const TARGET_PREFIXES_32BIT: &[&str] = &[/* ... */]; that contains arm-, i586-, etc

Add const MUSL_SUFFIXES : &[&str] = &[/* ... */]; with -musleabi, -musl, etc

Add fn target_is_32bit(target: &str) -> bool that checks if it starts if target starts with any of TARGET_PREFIXES_32BIT. Then we can reuse it with the GNU change.

Add fn target_is_musl(target: &str) -> bool that does the same with the suffixes

Add fn target_is_linux(target: &str) -> bool that checks if it contains -linux-

Check these three functions in order to set musl_redir_time64/musl_time64_abi

tgross35 · 2024-11-20T04:04:42Z

libc-test/build.rs

+    // Some targets use time64 symbols so set the musl_time64_abi appropriately.
+    // See #2088 and #1848 for more information.
+    match target {
+        "arm-unknown-linux-musleabi"
+        | "arm-unknown-linux-musleabihf"
+        | "armv5te-unknown-linux-musleabi"
+        | "armv7-unknown-linux-musleabi"
+        | "armv7-unknown-linux-musleabihf"
+        | "i586-unknown-linux-musl"
+        | "i686-unknown-linux-musl"
+        | "mips-unknown-linux-musl"
+        | "mipsel-unknown-linux-musl"
+        | "powerpc-unknown-linux-musl" => {
+            cfg.cfg("musl_time64_abi", None);
+        }
+        _ => {}
+    }


Why do we need this in build.rs if it's set automatically by our crate's build script?

tgross35 · 2024-11-20T04:11:17Z

build.rs

+    // Some ABIs need to redirect time related symbols to their time64
+    // equivalents. See #2088 and #1848 for more information.
+    if is_musl_time64_abi() {
+        set_cfg("musl_time64_abi");


We should set two separate configs here:

musl_redir_time64 (musl_time64_abi) which corresponds to musl's _REDIR_TIME64

linux_time_bits64 which corresponds to __USE_TIME_BITS64 in uapi

We need this to be split because musl and glibc share the kernel uapi

tgross35 · 2024-11-20T04:21:49Z

src/unix/linux_like/linux/mod.rs

+        #[cfg(musl_time64_abi)]
+        pub input_event_sec: ::c_ulong,
+        #[cfg(musl_time64_abi)]
+        pub input_event_usec: ::c_ulong,
+        #[cfg(not(musl_time64_abi))]
        pub time: ::timeval,


As mentioned above this should make use of linux_time_bits64. Specifically to match https://github.com/torvalds/linux/blob/bf9aa14fc523d2763fc9a10672a709224e8fcaf4/include/uapi/linux/input.h#L29 it could be:

#[cfg(any(target_pointer_width = "64", not(linux_time_bits64))] pub time: ::timeval, #[cfg(not(any(target_pointer_width = "64", not(linux_time_bits64)))] pub input_event_sec: ::c_ulong, #[cfg(all(not(any(target_pointer_width = "64", not(linux_time_bits64))), target_arch = "sparc64")] pub input_event_usec: ::c_uint, #[cfg(all(not(any(target_pointer_width = "64", not(linux_time_bits64))), target_arch = "sparc64")] pub __pad: ::c_uint, #[cfg(all(not(any(target_pointer_width = "64", not(linux_time_bits64))), not(target_arch = "sparc64"))] pub input_event_usec: ::c_ulong,

unfortunately a bit messy but I think that's accurate

tgross35 · 2024-11-20T04:25:05Z

src/unix/linux_like/linux/mod.rs

-            pub fn mq_open(name: *const ::c_char, oflag: ::c_int, ...) -> ::mqd_t;
-            pub fn mq_close(mqd: ::mqd_t) -> ::c_int;
-            pub fn mq_unlink(name: *const ::c_char) -> ::c_int;
-            pub fn mq_receive(
-                mqd: ::mqd_t,
-                msg_ptr: *mut ::c_char,
-                msg_len: ::size_t,
-                msg_prio: *mut ::c_uint,
-            ) -> ::ssize_t;
-            pub fn mq_timedreceive(
-                mqd: ::mqd_t,
-                msg_ptr: *mut ::c_char,
-                msg_len: ::size_t,
-                msg_prio: *mut ::c_uint,
-                abs_timeout: *const ::timespec,
-            ) -> ::ssize_t;
-            pub fn mq_send(
-                mqd: ::mqd_t,
-                msg_ptr: *const ::c_char,
-                msg_len: ::size_t,
-                msg_prio: ::c_uint,
-            ) -> ::c_int;
-            pub fn mq_timedsend(
-                mqd: ::mqd_t,
-                msg_ptr: *const ::c_char,
-                msg_len: ::size_t,
-                msg_prio: ::c_uint,
-                abs_timeout: *const ::timespec,
-            ) -> ::c_int;
-            pub fn mq_getattr(mqd: ::mqd_t, attr: *mut ::mq_attr) -> ::c_int;
-            pub fn mq_setattr(
-                mqd: ::mqd_t,
-                newattr: *const ::mq_attr,
-                oldattr: *mut ::mq_attr
-            ) -> ::c_int;
-
-            pub fn pthread_mutex_consistent(mutex: *mut pthread_mutex_t) -> ::c_int;


Did these functions move because they are now available on openharmony? If so, that should go in a different commit preferably.

rustbot assigned JohnTitor Aug 5, 2024

rustbot added the S-waiting-on-review label Aug 5, 2024

myl7 force-pushed the musl branch 2 times, most recently from c45a829 to 0185395 Compare August 5, 2024 11:43

myl7 changed the title ~~Draft: Modernize the libc crate~~ Rebase of "Upgrade musl supported version to 1.2.3" Aug 5, 2024

rustbot added S-waiting-on-author and removed S-waiting-on-review labels Aug 12, 2024

danielframpton and others added 14 commits August 17, 2024 11:33

ci: install-musl: upgrade to 1.2.3

c9bf9fc

musl: define time_t as i64 type.

06f4086

musl 1.1 maintenance has for all practical purposes ended. Accordingly, there is no point in supporting both 1.1 and 1.2 in the libc crate, so follow the time_t type transition to 64-bit.

musl: fix utmpx struct layout

76dc6a9

- add padding members for musl 1.2 - ensure the padding members have an appropriate type (always c_int)

Redirecting to musl time64 symbols

c8fa1a1

Update stat structures and redirect all stat functions.

030363e

Update x86 musl to 1.2.3

2120628

Update libc-test/build.rs testing for musl 1.2.3 symbols

b7f429e

Update arm to musl 1.2.3

06fd916

Update mips to musl 1.2.3

ccf9e57

Update mipsel to musl 1.2.3

b9c6bcd

Update x86_64 to musl 1.2.3

d4d8e01

Fix musl_time64_abi cfg flag in build script

e7daf4f

By using set_cfg() helper.

Allow unused imports for some statements to remove warnings

67d56c1

myl7 force-pushed the musl branch from eb4c4b2 to 67d56c1 Compare August 17, 2024 03:40

tgross35 mentioned this pull request Oct 13, 2024

libc: proposal to update minimum versions and release 0.3 rust-lang/libs-team#463

Closed

neuschaefer mentioned this pull request Oct 16, 2024

Enable statx on musl-libc #3976

Draft

3 tasks

tgross35 requested changes Nov 20, 2024

View reviewed changes

tgross35 mentioned this pull request Nov 20, 2024

Switch to 64 bit file and time APIs on GNU libc for 32bit systems #3175

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rebase of "Upgrade musl supported version to 1.2.3" #3791

Rebase of "Upgrade musl supported version to 1.2.3" #3791

myl7 commented Aug 5, 2024 •

edited

Loading

rustbot commented Aug 5, 2024

rustbot commented Aug 5, 2024

semarie commented Aug 5, 2024

tgross35 commented Aug 12, 2024 •

edited

Loading

bors commented Aug 12, 2024

tgross35 commented Aug 12, 2024

myl7 commented Aug 17, 2024

bors commented Aug 27, 2024

tgross35 commented Aug 29, 2024

tgross35 commented Aug 29, 2024

tgross35 left a comment

tgross35 Nov 20, 2024

tgross35 Nov 20, 2024

tgross35 Nov 20, 2024

tgross35 Nov 20, 2024

tgross35 Nov 20, 2024

tgross35 Nov 20, 2024

Rebase of "Upgrade musl supported version to 1.2.3" #3791

Are you sure you want to change the base?

Rebase of "Upgrade musl supported version to 1.2.3" #3791

Conversation

myl7 commented Aug 5, 2024 • edited Loading

rustbot commented Aug 5, 2024

rustbot commented Aug 5, 2024

semarie commented Aug 5, 2024

tgross35 commented Aug 12, 2024 • edited Loading

bors commented Aug 12, 2024

tgross35 commented Aug 12, 2024

myl7 commented Aug 17, 2024

bors commented Aug 27, 2024

tgross35 commented Aug 29, 2024

tgross35 commented Aug 29, 2024

tgross35 left a comment

Choose a reason for hiding this comment

tgross35 Nov 20, 2024

Choose a reason for hiding this comment

tgross35 Nov 20, 2024

Choose a reason for hiding this comment

tgross35 Nov 20, 2024

Choose a reason for hiding this comment

tgross35 Nov 20, 2024

Choose a reason for hiding this comment

tgross35 Nov 20, 2024

Choose a reason for hiding this comment

tgross35 Nov 20, 2024

Choose a reason for hiding this comment

myl7 commented Aug 5, 2024 •

edited

Loading

tgross35 commented Aug 12, 2024 •

edited

Loading