Skip to content

Conversation

tgross35
Copy link
Contributor

Cherry picked from #4695

@tgross35
Copy link
Contributor Author

tgross35 commented Sep 18, 2025

I assume the failures may be coming from:

@rustbot rustbot added the A-CI Area: CI-related items label Sep 18, 2025
@tgross35
Copy link
Contributor Author

The MS_NOUSER fix is easy. Baud rates I'm less sure about; our definitions match those from Linux https://github.com/torvalds/linux/blob/992d4e481e958c6898fe750afd429d1b585fff93/include/uapi/asm-generic/termbits-common.h but glibc doesn't match? https://github.com/bminor/glibc/blob/3fd794264e3f062bfbf0c8727cef82f16d51450b/bits/termios-baud.h. Guessing that the commit above made it so we're starting to see the glibc definitions.

@tgross35
Copy link
Contributor Author

tgross35 commented Sep 18, 2025

https://sourceware.org/bugzilla/show_bug.cgi?id=33340, totally missed the discussion here at #4692.

Comment on lines -2132 to +2133
pub const MS_NOUSER: c_ulong = 0xffffffff80000000;
// FIXME: should this be an int? The suffix is `U` not `UL`.
pub const MS_NOUSER: c_ulong = 1 << 31;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote the glibc patch. Sorry, I didn't expect breakage here.

Here is my rational from the commit message [1]:

Using gcc -Wshift-overflow=2 -Wsystem-headers to compile a file including <sys/mount.h> will cause a warning since 1 << 31 is undefined behavior on platforms where int is 32-bits. 

Technically unsigned integers aren't allowed in C enum types, it is a GCC extension that is widely supported. We decided it was safe to use since the extension is already required for EPOLLONESHOT and EPOLLET [2].

Regarding the FIXME, I am not super familiar with Rust, but the mount function uses unsigned long for the flags argument. Until C23 enums could not have a type other than int, otherwise UL probably would have made more sense. Hopefully that helps you a bit in your decision.

[1] https://inbox.sourceware.org/libc-alpha/20250217055647.493638-1-collin.funk1@gmail.com/
[2] https://inbox.sourceware.org/libc-alpha/875xjwvrhj.fsf@oldenburg.str.redhat.com/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your patch makes complete sense, I assume the intent was always to be 0x80000000 rather than the sign-extended 0xffffffff80000000. I just left that fixme as something to double check after the experimentation here; surprised you found this PR out of the blue, but thank you for saving me the work :)

I wrote the glibc patch. Sorry, I didn't expect breakage here.

Not at all a problem, there's nothing user-visible—just a test failure checking the equality of constants. Expected whenever the libraries we test against get updated.

@tgross35
Copy link
Contributor Author

Related to termio failures: llvm/llvm-project#137403

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment