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/[email protected]/
[2] https://inbox.sourceware.org/libc-alpha/[email protected]/

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants