libc icon indicating copy to clipboard operation
libc copied to clipboard

uclibc/mips change `SA_*` to `uint`

Open tgross35 opened this issue 1 year ago • 5 comments

This comes from https://github.com/rust-lang/libc/pull/3211, I'm not positive what the context is. But we can do this at 1.0.

@cppcoffee do you have links to the relevant headers?

tgross35 avatar Aug 29 '24 11:08 tgross35

Hi @tgross35 , I'm really sorry, it's been a while and I've somewhat forgotten, but at the time I encountered a compilation error related to signal interfaces. Modifying it to u_long allowed the compilation to pass.

cppcoffee avatar Aug 29 '24 14:08 cppcoffee

Nothing to be sorry for - your PR has been open a very long time :) after https://github.com/rust-lang/libc/pull/3211 merges just submit another PR to correct these things, but I do need a link to headers to check against if you can find it.

(If headers aren't easy to find then it's not the end of the world since mips is tier 3 and I can take your word for it, just better to be certain if possible).

tgross35 avatar Aug 29 '24 18:08 tgross35

I reproduced it and the compilation error is as follows:

error[E0308]: mismatched types
   --> /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/nix-0.26.4/src/macros.rs:70:35
    |
70  |                       const $Flag = libc::$Flag $(as $cast)*;
    |                                     ^^^^^^^^^^^ expected `u32`, found `i32`
    |
   ::: /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/nix-0.26.4/src/sys/signal.rs:414:1
    |
414 | / libc_bitflags! {
415 | |     /// Controls the behavior of a [`SigAction`]
416 | |     #[cfg_attr(docsrs, doc(cfg(feature = "signal")))]
417 | |     pub struct SaFlags: SaFlags_t {
...   |
439 | |     }
440 | | }
    | |_- in this macro invocation
    |
    = note: this error originates in the macro `libc_bitflags` (in Nightly builds, run with -Z macro-backtrace for more info)

cppcoffee avatar Aug 30 '24 01:08 cppcoffee

Okay, tracing back: this error comes from https://github.com/nix-rust/nix/blame/ae338dfc2097a741d9dbff21391c737f9c95de93/src/sys/signal.rs#L413-L421, which seems to indicate that sigaction flags on ulibc are c_ulong. This was added here https://github.com/nix-rust/nix/pull/1603.

Looking at ulibc, this might not always be correct.

  • sa_flags is unsigned long by default https://github.com/kraj/uClibc/blob/ca1c74d67dd115d059a875150e10b8560a9c35a8/libc/sysdeps/linux/common/bits/sigaction.h#L39
  • unsigned long on ia64 https://github.com/kraj/uClibc/blob/ca1c74d67dd115d059a875150e10b8560a9c35a8/libc/sysdeps/linux/ia64/bits/sigaction.h#L35, which matches our x86 sigaction https://github.com/rust-lang/libc/blob/0e6afd534ed7a0406e9dfc9242c2c9370a5b8d05/src/unix/linux_like/linux/uclibc/x86_64/mod.rs#L136
  • unsigned long also on sparc https://github.com/kraj/uClibc/blob/ca1c74d67dd115d059a875150e10b8560a9c35a8/libc/sysdeps/linux/sparc/bits/sigaction.h#L35
  • unsigned on mips https://github.com/kraj/uClibc/blob/ca1c74d67dd115d059a875150e10b8560a9c35a8/libc/sysdeps/linux/mips/bits/sigaction.h#L26

Our struct sigaction reflects this, as do the flags.

So I think nix might have to be the one to change here - their config needs to check if ulibc and mips and make it c_uint rather than always c_ulong. ~~I think this might apply to gun/musl too and not just ulibc - see the mips-specific https://github.com/torvalds/linux/blob/20371ba120635d9ab7fc7670497105af8f33eb08/arch/mips/include/uapi/asm/signal.h#L94 in the kernel, as well as the other results.~~ (the last bit is not true, see below)

tgross35 avatar Aug 30 '24 03:08 tgross35

Ah, on glibc mips it's still int https://github.com/bminor/glibc/blob/3fc063dee01da4f80920a14b7db637c8501d6fd4/sysdeps/unix/sysv/linux/mips/bits/sigaction.h#L30 and musl looks like it's always int https://github.com/kraj/musl/blob/2c41ee96f60074fa8ac387e1fcdedba0654fb173/include/signal.h#L175

tgross35 avatar Aug 30 '24 03:08 tgross35