Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use linux/* headers for linux and android #235

Closed
kamalmarhubi opened this issue Mar 18, 2016 · 21 comments
Closed

Use linux/* headers for linux and android #235

kamalmarhubi opened this issue Mar 18, 2016 · 21 comments

Comments

@kamalmarhubi
Copy link
Contributor

This comes out of discussion on #233. Additions to Linux system APIs are included in headers like <linux/header.h> rather than <header.h>. We should use these headers for verification on Linux, in order to include constants that are not present in <header.h>.

@posborne provided some useful background:

Starting with kernel 3.7, there has been a push to separate the headers that are suitable for inclusion from userspace (or at least from libc) into the include/uapi and arch/$ARCH/include/uapi directories in the kernel. This is the case for the constants in this PR: http://lxr.free-electrons.com/source/include/uapi/linux/fcntl.h.

The idea has been around for awhile, but it recently got a bigger push.

As a specific example from #233:

$ printf '#define _GNU_SOURCE\n#include <fcntl.h>' | cpp -dM - | grep '#define F_
GET_SEAL'
$ printf '#define _GNU_SOURCE\n#include <linux/fcntl.h>' | cpp -dM - | grep '#def
ine F_GET_SEAL'
#define F_GET_SEALS (F_LINUX_SPECIFIC_BASE + 10)
kamalmarhubi added a commit to kamalmarhubi/libc that referenced this issue Mar 18, 2016
Also move F_DUPFD_CLOEXEC up a level as it is available on Android.

This commit leaves file sealing related fcntls and bitflag constants
out, as they are defined in `linux/fcntl.h` rather than `fcntl.h`. They
can be included once an approach for verification has been figured out.
See rust-lang#235 for more detail.
@kamalmarhubi
Copy link
Contributor Author

From reading a bit more, it appears that these uapi headers end up being installed under /usr/include/linux/. However, including that file results in conflicts with the original fcntl.h, which is included via several other headers. To get this to work, we'd have to make a bunch of changes to what headers get used in verification on Linux.
@kamalmarhubi on #233

@posborne
Copy link
Contributor

However, including that file results in conflicts with the original fcntl.h, which is included via several other headers

You Betcha (I'm feeling very Minnesotan today). This issue will be commonplace when using these headers. My initial thought on this is to have a separate test function with its own ctest::TestGenerator instance that includes the Linux UAPI headers and not ones from $libc. The problem here is that we end up with two sets of tests that don't play particularly nicely together.

If ctest were modified to support an inclusive approach (in addition to the exclusive model that exists currently), I think it might be sane to do the following:

  • Define the set of constants, structures, etc. that are to be verified using the Linux headers (possibly with some kind of pattern matching for constants to reduce duplication)
  • Exclude these from the linux/musl verification against libc compatability TestGenerator
  • Include these for the Linux compatibility TestGenerator

Since the complement of the ignored set from the $libc tests is covered by the linux test, we can have a level of confidence that we have good coverage.

@kamalmarhubi
Copy link
Contributor Author

@posborne do you know of any disagreements between the libc headers and the UAPI headers on values or signatures they both include? If not, we could just switch to using UAPI headers for verification on (non-musl) Linux, and the other headers on other platforms.

I may be taking a particular reading of the comment, but on #232 @alexcrichton said

The intention of the libc crate was basically to just provide the most up-to-date version of APIs that there are with the assumption that the old APIs are always binary compatible with the new ones (which they typically are). In terms of any fallback implementations, that was going to be left up to downstream crates. Downstream authors need to be vigilant to not use APIs that aren't available on a platform they're shipping on.

[...]

All libc is doing is defining what the APIs would be if they're there.
@alexcrichton on #232

(Emphasis mine.)

Providing these are part of the kernel's stable ABI, which I understand they are, we should be fine to do this.

kamalmarhubi added a commit to kamalmarhubi/libc that referenced this issue Mar 18, 2016
Also move F_DUPFD_CLOEXEC up a level as it is available on Android.

This commit leaves file sealing related fcntls and bitflag constants out
of "plain" Linux, as they are defined in `linux/fcntl.h` rather than
`fcntl.h`. They can be included once an approach for verification has
been figured out. Android and musl both include them in `fcntl.h`
however. See rust-lang#235 for more detail.
@alexcrichton
Copy link
Member

Yeah if we need to have two separate test suites here for Linux that's fine by me, also feel free to change whatever needs to be changed in ctest, it was basically created for this project :)

@kamalmarhubi
Copy link
Contributor Author

This came up in nix-rust/nix#354. @posborne @alexcrichton are there strong objections to just testing against UAPI headers on Linux?

@alexcrichton
Copy link
Member

Seems fine to me if they're explicitly intended for inclusion in user-space

@posborne
Copy link
Contributor

No objection from me.

@kamalmarhubi
Copy link
Contributor Author

@posborne so are the <linux/blah.h> headers definitely for user space use? It seems like yes, since they don't include constants guarded by #ifdef __KERNEL__.

@kamalmarhubi
Copy link
Contributor Author

This is proving a bit more complicated than I thought. For now I'm unclear on how to use both <fcntl.> (for open(2) and friends), and <linux/fcntl.h> (for file sealing fcntls) in one program.

Eg, this program does not compile.

#define _GNU_SOURCE
#include <fcntl.h>
#include <linux/fcntl.h>

int main(void) {
    int fd = open("/tmp/hi.c", O_RDONLY);
    return 0;
}

@posborne
Copy link
Contributor

@kamalmarhubi Yeah, that is the problem I have run into as well. Although the headers are suitable for use in use space in theory, many of them conflict with libc headers in practice (yuck!). That is why I expressed the thoughts about having a separate test generator in #235 (comment)

The linux exported uapi headers are far from complete, so I do not see an easy way to avoid going down the path of having two separate test suites.

@kamalmarhubi
Copy link
Contributor Author

I remember your comment but I guess I had to spend some time to properly understand why.

I also now see why you want to extend ctest to support inclusive rather than exclusive testing!

@joerg-krause
Copy link
Contributor

There is an FAQ on the musl wiki about mixing userspace and kernel headers:

Q: why am i getting "error: redefinition of struct ethhdr/tcphdr/etc" ?
A: The application you're trying to compile mixes userspace and kernel headers (/include/linux). The kernel headers are notoriously broken for userspace and clash with the definitions provided by musl. It only works (in certain cases) with GLIBC because the userspace headers just include the kernel headers and because the kernel headers have a hardcoded compatibility fix for GLIBC to prevent exactly this issue (linux/libc_compat.h).

@kamalmarhubi
Copy link
Contributor Author

Oh interesting! In most cases I've seen, the musl <foo.h> headers include all the definitions we'd want from the <linux/foo.h> headers. And we must be in one of those cases that falls outside of "in certain cases" because we get all kinds of errors if we try to mix them!

@joerg-krause
Copy link
Contributor

There are at least two possibilities to fix this issue:

  • factor out the code needing the kernel headers into a separate compilation unit which doesn't use any userspace headers
  • copy the required (struct) definitions into an internal header and rename them

@kamalmarhubi
Copy link
Contributor Author

kamalmarhubi commented Apr 22, 2016

Yeah, I think we're effectively looking at going with the separate compilation unit approach: a separate test program for testing against UAPI headers.

@joerg-krause
Copy link
Contributor

Yes, I think this is a good approach!

@raphaelcohn
Copy link
Contributor

Please be careful here. The <linux/foo.h> headers aren't part of the libc. It's actually extremely rare to use them; mostly, it's to get at #defined'd constants that haven't made it into the libc, or, in the past, networking structs. I maintain a custom linux distro, and, with each major kernel release, have found myself re-patching linux UAPI (mostly because upstream only takes patches to detect glibc and tend not to work with musl). I also end up occasionally patching BusyBox, as it uses them, although that's become far less frequent now it's got mainstream musl users Aalpine Linux). To my mind, the UAPI headers are nothing to do with libc, and should, if wanted, be wrapped in a separate crate.

@nathansizemore
Copy link

nathansizemore commented Sep 14, 2016

@raphaelcohn Even though linux/foo.h headers aren't part of libc, you can't actually build glibc without them. The two are too dependent on each other to leave a clear dividing line.

@raphaelcohn
Copy link
Contributor

raphaelcohn commented Sep 14, 2016

@nathansizemore Sure - but building glibc cleanly, from checkout, reliably and consistently, is an exercise in futility. There's a load of others things it typically ends up needing - not just headers. Musl, on the over hand, does draw a very distinct dividing line. In recent versions, it uses just linux/vt.h, linux/soundcard.h and linux/kd.h as far as I can tell (done quickly).

My point of view is very much coloured by the way I see glibc as yesterday's libc, and musl as tomorrow's, on linux. It's also hard to agree on a dividing line as libcs themselves are such a messy mix of unnamespaced POSIX APIs, C standard APIs and OS-specifics (including syscall code).

I think it would be nice to have bindings or some support for Linux's UAPI - but I don't feel it belongs in the libc crate. If it's wanted, then make it a dependency. It's effectively what it is.

Personally I hope in the long run Rust frees itself from the libc, primarily by replacing the functions the std libs need with syscalls and the like. It'll certainly open more opportunities for safety checks and optimisation. In that event, a binding to the UAPI would be very useful.

@valarauca
Copy link
Contributor

valarauca commented Oct 5, 2016

waitid and flags is exposed in the general headers. The issue was brought up was in #417 acaea91 with the ptraceflags.

:.:.:

Adding the Linux UAPI's to libc pretty clearly won't work. While yes building an alternate test-suite is possible. Travis-CI doesn't support rolling a custom kernel version which means libc will get stuck on one or two kernel versions for potentially years. The project would need to switch to a different testing platform as travel-ci currently only supports kernel 3.2 and 4.4 (and it took them years to move from 2.6 to 3.2).

I guess the project could pick up another test suite. But building a separate project that depends on libc seems like a far simpler solution. As this was already done with kernel-sys32 for Windows, and eventually will also have to be done for FreeBSD, OpenBSD, OSX, Solaris, etc. Rolling everything into libc may turn this project into libos.

@gnzlbg
Copy link
Contributor

gnzlbg commented May 24, 2019

This is fixed on master. libc provides user-space APIs, which not only include glibc APIs on linux, but also kernel user-space APIs that are not available in glibc (linux/...) . We test these by having multiple test suites to cover headers that are incompatible with each other.

We do not provide linux kernel-space in libc. That's IMO out-of-scope, these APIs are often incompatible with the user-space ones, and programs typically target either user-space or kernel-space, not both, so exposing those would be better done in a different crate.

Arguably, it would be cleaner for libc to only expose what glibc exposes, and move all Linux kernel APIs to a different crate (e.g. linux crate for user-space APIs, and linux_kernelspace for kernel-space APIs). Doing that would require deprecating all kernel APIs from libc, which at this point are a lot, and is probably not worth it. If someone cares enough about this, they can open a different issue.


The same applies to android, and pretty much all other operating system's. I just sent a PR (#1364 ) deprecating all MacOSX kernel APIs to fix #981 . The situation there is different because 1) we have a fully working mach crate exposing MacOSX kernel APIs that's already tested on multiple MacOSX versions, and 2) the amount of kernel APIs that were exposed in libc is minimal.

@gnzlbg gnzlbg closed this as completed May 24, 2019
danielverkamp pushed a commit to danielverkamp/libc that referenced this issue Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants