Alias all LFS64 symbols to their non-LFS64 counterparts on musl

[bossmc]

As per #2934 the LFS64 symbols on musl-libc are simply aliases to the non-LFS64 symbols. Currently this is done both in the header files (as #define entries) and in the library (as aliasing symbols). There is a desire in musl to drop the ABI compatibility shims (the symbol aliases) - currently the libc crate exports the LFS64 symbols by extern-ing the compatibility shims which will fail if musl removes them.

This changes the musl build of libc to replicate the aliasing that's in the C header files (with pub use xxx as xxx64) so the API from the libc crate is unchanged, but the crate is now compatible with the upcoming musl release.

I've also checked and all the LFS64 types (e.g. off64_t) are already the same as their non-LFS64 equivalents.

This is an annoying change to test, libc-test seems expect to build against musl 1.1.24 (in fact, does Rust even support musl 1.2? It's not obvious that it does... e.g. see

libc/src/unix/linux_like/linux/musl/mod.rs

Lines 288 to 292 in d99c37d

	// FIXME: musl added paddings and adjusted
	// layout in 1.2.0 but our CI is still 1.1.24.
	// So, I'm leaving some fields as comments for now.
	// ref. https://github.com/bminor/musl/commit/
	// 1e7f0fcd7ff2096904fd93a2ee6d12a2392be392

)

[rust-highfive]

r? @Amanieu

(rust-highfive has picked a reviewer for you, use r? to override)

[JohnTitor]

This is a breaking change, we have to deprecate them on musl first otherwise user code depending on them would be broken.

[bossmc]

Unless I've missed something, this isn't a breaking change (at a crate API level at least) - all the types and functions that used to exist still do and still have the same name/arguments/structure/alignment etc. The only difference is that when rust code calls xxx64(*const xxx64_t) it will actually call the xxx function in the musl library (which it was already doing since the xxx64 symbol in the library was a link-time alias to the xxx symbol).

I've done a bit more work on this (ensuring that the complex types are rust-level aliases to each other on musl so they can be passed to either function without failing type checks), though I'm not sure it's complete (I've not done this for the structs in musl's b64 or b32 modules yet - and I'm unsure what needs to happen in the b32 case).

[JohnTitor]

This could be moved to test, I guess?

[bossmc]

Could be, though this is merely a compile-time check that the interchangeable types are aliases (rather than identically defined separate types) it's not a test in the classical sense.

An alternative might be to #[cfg(test)] this function?

[JohnTitor]

Ah, I see! Thanks for clarifying. Could you fix the style failure and squash commits?

This is an annoying change to test, libc-test seems expect to build against musl 1.1.24 (in fact, does Rust even support musl 1.2

No, we don't support musl 1.2.0 or higher as it contains the time64 change. We've tried to update and the discussion is ongoing: #2088
I'm not sure how it's related to this change though, does it matter?

[bossmc]

No, we don't support musl 1.2.0 or higher as it contains the time64 change. We've tried to update, and the discussion is ongoing: #2088 I'm not sure how it's related to this change though, does it matter?

There's a proposed patch available today to remove the symbols from the musl library on the 1.2.3 branch, to test this change has sufficiently removed the references to the LFS64 symbols, I plan to use that patched library. Unfortunately, this also entails making a few changes to the libc crate (e.g. fixing up that linked struct definition) to get the tests passing which are changes that don't want to go into the crate while it's targeting 1.1.

Maybe this PR should be deferred until @wesleywiser's 1.2 support lands - since these symbols won't be disappearing in the 1.1.x releases.

[bors]

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

[colincross]

I tested this against ToT musl that includes the LFS64 symbol removal (https://git.musl-libc.org/cgit/musl/commit/?id=246f1c811448f37a44b41cd8df8d0ef9736d95f4), and it required the attached addition to the second patch:
dirent64.patch.txt.

[bossmc]

Thanks @colincross I'd not noticed the extra-traits feature. Apparently there's a catchup merge/rebase needed too now 💯

[Amanieu]

Style check is failing, but otherwise this looks good to merge. It is not a breaking change.

[bossmc]

From the discussion in #3248 this probably needs some markups. In that discussion @joshtriplett would prefer the libc crate to expose LFS64 symbols even on Musl 1.2.4 so cross-libc-targeting crates don't need to use #[cfg] to find the 64-bit compatible file operations. This seems reasonable (if a little unusual, not following the model used elsewhere in the libc crate) but it exposes another few questions about how the libc crate exposes the aliased symbols.

Currently the implementation exposes aliased types with type <foo>64 = <foo> and aliased functions with use <foo> as <foo>64 this has a few impacts:

• The two types are not distinct, so (e.g.) trait impls on them overlap where they wouldn't for e.g. glibc
• The function type signatures are "wrong", e.g. the <foo>64 function takes *mut <bar> not *mut <bar>64 arguments
• The aliased functions are "the same" function (e.g. casting them to raw pointers will give the same value)

The use of use is also fairly messy, use pulls the named symbol into the current module in every namespace (value, function, type) unless there's already a symbol of that name defined directly in the current module. This, combined with the name clash in stat()/stat64() vs struct stat/struct stat64 means that it's not immediately obvious where symbols are coming from and if they're correct.

An alternative approach would be for the musl backend to define each LFS64-affected type twice and to define shim functions of the form:

fn stat64(path: *const c_char, buf: *mut stat64) -> c_int {
  return stat(path, buf.cast());
}

Which assumes:

• The inliner will do the right thing in release builds
• The two structures are .cast()-able to each other which would want a regression test (or deterministic codegen?)

Thoughts?

[Amanieu]

I believe there was some confusion in the discussion in #3248. @joshtriplett was talking about time64, not LFS64.

You're right that changing the *64 types would be a breaking change due to possibe impls, and the alternative solution would be better.

[bossmc]

Ok, I've added the set of shim functions (in .../musl/lfs64.rs) and restored the duplicate definitions of the structs. I think this is now the correct shape for review!

[bors]

⌛ Testing commit 75ac488 with merge 1e8c55c...

[bors]

☀️ Test successful - checks-actions, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13, checks-cirrus-freebsd-14
Approved by: Amanieu
Pushing 1e8c55c to main...