Upgrade to musl 1.1.24 in CI #1588

oxalica · 2019-11-10T12:39:58Z

Required by #1577

Note that in musl 1.1.24, struct sched_param from sched.h has changed and some fields became reserved. So these fields are outdated. I'm not sure if we should rename them, since they are in public API.

I simply skip struct sched_param from the test now.

Here's the diff between musl 1.1.23 and 1.1.24

diff --git a/include/sched.h b/include/sched.h
index 05d40b1e..7e470d3a 100644
--- a/include/sched.h
+++ b/include/sched.h
@@ -18,10 +18,12 @@ extern "C" {
 
 struct sched_param {
        int sched_priority;
-       int sched_ss_low_priority;
-       struct timespec sched_ss_repl_period;
-       struct timespec sched_ss_init_budget;
-       int sched_ss_max_repl;
+       int __reserved1;
+       struct {
+               time_t __reserved1;
+               long __reserved2;
+       } __reserved2[2];
+       int __reserved3;
 };

rust-highfive · 2019-11-10T12:40:02Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @gnzlbg (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

oxalica · 2019-11-12T05:20:47Z

There's another change that field __seq of struct ipc_perm on musl-aarch64 becomes int from unsigned short after musl 1.1.24
It's also part of public API.

bors · 2019-11-19T02:28:48Z

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

oxalica · 2019-11-20T06:33:08Z

ping? @gnzlbg

gnzlbg

This looks really good @oxalica !

cc @alexcrichton @pietroalbini @mati865 : bumping the musl version for the API exposed by libc looks in general fine to me, but since apparently musl version 1.1 is slightly ABI incompatible with musl 1.0.x we might want to apply the same change to rust-lang/rust to keep everything in sync. As long as musl is statically linked, it should be fine. But some distros like Alpine, dynamically link musl, and dynamically linking an slightly ABI incompatible version might be troublesome. This PR does not apply these ABI incompatible changes to libc, but if we make the upgrade it becomes easy for new APIs that get added to follow the musl 1.1 ABI instead of the 1.0 ABI because that's what we test here against. I'm not a musl expert, so feel free to cc others here as well.

gnzlbg · 2019-11-20T07:50:48Z

libc-test/build.rs

+            "sched_ss_init_budget",
+            "sched_ss_max_repl",
+        ].contains(&field) && musl) ||
+        // FIXME: After musl 1.1.24, the type becomes `int` instead of `unsigned short`.


This means that musl 1.1 has ABI breaking changes with respect to musl 1.0.x ?

@gnzlbg Oh, sorry. It may not be an ABI breaking change. The struct layout is kept and just some fields are renamed to reserved.

Does that refer to the change in type of ipc_perm.__seq field from unsigned short to int on aarch64 ? Do you have a musl diff for this change?

IIUC it was unsupported bminor/musl@827aa8f
But still it's breaking change between 1.1.23 and 1.1.24
Edit: replied to wrong comment

Relevant commit for this change bminor/musl@319b2d0

gnzlbg · 2019-11-20T07:51:35Z

libc-test/build.rs

-                                           field == "ssi_arch"))
+                                           field == "ssi_arch")) ||
+        // FIXME: After musl 1.1.24, it have only one field `sched_priority`,
+        // while other fields become reserved.


Can you elaborate here in a comment on what this means? Did the fields of this struct change? Some fields became "private" ?

I placed the diff of musl in the initial comment

Thanks, there are functions for mapping Rust field names to C field names in ctest, e.g., https://docs.rs/ctest/0.2.22/ctest/struct.TestGenerator.html#method.field_name . So maybe we can just translate the Rust names to the new __reservedX names for now ?

@gnzlbg

So maybe we can just translate the Rust names to the new __reservedX names for now ?

This does not work for struct { time_t __reserved1; long __reserved2; } __reserved2[2];.
The anonymous struct does not match struct timespec. Build log

We still need to skip these fields at least.

Yes, you are right.

oxalica · 2019-11-20T08:24:58Z

@gnzlbg

The struct icp_perm on aarch64 is in arch/aarch64/bits/ipc.h of musl 1.1.23.

struct ipc_perm {
        key_t __ipc_perm_key;
        uid_t uid;
        gid_t gid;
        uid_t cuid;
        gid_t cgid;
        mode_t mode;
        unsigned short __ipc_perm_seq;

        unsigned long __pad1;
        unsigned long __pad2;
};

#define IPC_64 0

But the file is removed in musl 1.1.24. It seems to fallback to arch/generic/bits/ipc.h.

struct ipc_perm {
        key_t __ipc_perm_key;
        uid_t uid;
        gid_t gid;
        uid_t cuid;
        gid_t cgid;
        mode_t mode;
        int __ipc_perm_seq;
        long __pad1;
        long __pad2;
};

Note that only the field __ipc_perm_seq changed to int from unsigned short.
Due to the padding, the size and field offsets of the struct are unchanged.
But it will still lead to problem due to incompatible type (interpret the padding as a part of int).

alexcrichton · 2019-11-20T15:38:28Z

This seems fine to me to land, and it should also be fine to land an update in rust-lang/rust

gnzlbg · 2019-11-20T17:31:07Z

@bors: r+

bors · 2019-11-20T17:31:08Z

📌 Commit b14e947 has been approved by gnzlbg

bors · 2019-11-20T17:31:16Z

⌛ Testing commit b14e947 with merge ff27a33...

Upgrade to musl 1.1.24 in CI Required by #1577 Note that in musl 1.1.24, `struct sched_param` from `sched.h` has changed and some fields became reserved. So [these fields](https://github.com/rust-lang/libc/blob/13d4a5da2eafd82d3ebb2acb729d8b8c9148fb1f/src/unix/linux_like/mod.rs#L97) are outdated. I'm not sure if we should rename them, since they are in public API. I simply skip `struct sched_param` from the test now. Here's the diff between musl 1.1.23 and 1.1.24 ``` diff --git a/include/sched.h b/include/sched.h index 05d40b1e..7e470d3a 100644 --- a/include/sched.h +++ b/include/sched.h @@ -18,10 +18,12 @@ extern "C" { struct sched_param { int sched_priority; - int sched_ss_low_priority; - struct timespec sched_ss_repl_period; - struct timespec sched_ss_init_budget; - int sched_ss_max_repl; + int __reserved1; + struct { + time_t __reserved1; + long __reserved2; + } __reserved2[2]; + int __reserved3; }; ```

oxalica · 2019-11-20T17:55:39Z

Do we need to update the public API?

bors · 2019-11-20T19:20:58Z

💔 Test failed - status-azure

gnzlbg · 2019-11-21T18:25:02Z

@bors: retry

bors · 2019-11-21T20:34:45Z

⌛ Testing commit b14e947 with merge 2d94f3f...

Upgrade to musl 1.1.24 in CI Required by #1577 Note that in musl 1.1.24, `struct sched_param` from `sched.h` has changed and some fields became reserved. So [these fields](https://github.com/rust-lang/libc/blob/13d4a5da2eafd82d3ebb2acb729d8b8c9148fb1f/src/unix/linux_like/mod.rs#L97) are outdated. I'm not sure if we should rename them, since they are in public API. I simply skip `struct sched_param` from the test now. Here's the diff between musl 1.1.23 and 1.1.24 ``` diff --git a/include/sched.h b/include/sched.h index 05d40b1e..7e470d3a 100644 --- a/include/sched.h +++ b/include/sched.h @@ -18,10 +18,12 @@ extern "C" { struct sched_param { int sched_priority; - int sched_ss_low_priority; - struct timespec sched_ss_repl_period; - struct timespec sched_ss_init_budget; - int sched_ss_max_repl; + int __reserved1; + struct { + time_t __reserved1; + long __reserved2; + } __reserved2[2]; + int __reserved3; }; ```

bors · 2019-11-21T21:55:49Z

☀️ Test successful - checks-cirrus-freebsd-10, checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, status-azure
Approved by: gnzlbg
Pushing 2d94f3f to master...

rust-highfive assigned gnzlbg Nov 10, 2019

Upgrade to musl 1.1.24 in CI

b14e947

oxalica force-pushed the upgrade-musl branch from aea6412 to b14e947 Compare November 19, 2019 07:55

gnzlbg reviewed Nov 20, 2019

View reviewed changes

gnzlbg mentioned this pull request Nov 21, 2019

Provide SYS_statx for more target_arch/target_env #1577

Merged

bors merged commit b14e947 into rust-lang:master Nov 21, 2019

oxalica deleted the upgrade-musl branch November 22, 2019 05:45

nekopsykose mentioned this pull request Oct 19, 2023

linux_like: remove non-sched_priority fields from sched_param for musl #3395

Closed

Upgrade to musl 1.1.24 in CI #1588

Upgrade to musl 1.1.24 in CI #1588

Uh oh!

Conversation

oxalica commented Nov 10, 2019

Uh oh!

rust-highfive commented Nov 10, 2019

Uh oh!

oxalica commented Nov 12, 2019

Uh oh!

bors commented Nov 19, 2019

Uh oh!

oxalica commented Nov 20, 2019

Uh oh!

gnzlbg left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mati865 Nov 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gnzlbg Nov 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oxalica commented Nov 20, 2019

Uh oh!

alexcrichton commented Nov 20, 2019

Uh oh!

gnzlbg commented Nov 20, 2019

Uh oh!

bors commented Nov 20, 2019

Uh oh!

bors commented Nov 20, 2019

Uh oh!

oxalica commented Nov 20, 2019

Uh oh!

bors commented Nov 20, 2019

Uh oh!

gnzlbg commented Nov 21, 2019

Uh oh!

bors commented Nov 21, 2019

Uh oh!

bors commented Nov 21, 2019

Uh oh!

Uh oh!

gnzlbg left a comment •

edited

Loading

mati865 Nov 20, 2019 •

edited

Loading

gnzlbg Nov 20, 2019 •

edited

Loading