musl and kernel headers [was Re: system-images 1.4.2: od is broken; bzip2 is missing]

musl and kernel headers [was Re: system-images 1.4.2: od is broken; bzip2 is missing]

[Rich Felker]

On Mon, Oct 05, 2015 at 10:08:10AM -0500, Rob Landley wrote:
> > Re musl: two cases of breakage:
> > 
> >   #include <netinet/in.h>
> >   #include <linux/if_bridge.h>
> > results in:
> >   In file included from /usr/include/linux/if_bridge.h:18,
> >                    from networking/brctl.c:67:
> >   /usr/include/linux/in6.h:32: error: redefinition of 'struct in6_addr'
> >   /usr/include/linux/in6.h:49: error: redefinition of 'struct sockaddr_in6'
> >   /usr/include/linux/in6.h:59: error: redefinition of 'struct ipv6_mreq'
> > 
> > and
> > 
> >   #include <linux/ethtool.h>
> >   #include <net/ethernet.h>
> > results in:
> >   In file included from /usr/include/net/ethernet.h:10,
> >                    from networking/ifplugd.c:41:
> >   /usr/include/netinet/if_ether.h:96: error: redefinition of 'struct ethhdr'
> > 
> 
> That I leave to Rich. :)

Including kernel headers is just really problematic. These days they
try to make it work on glibc with a complex dance between glibc's
headers and the kernel headers. You're likely to have the best luck by
including the libc headers first. I think Sabotage and possibly some
other musl-based dists are patching the kernel headers to make them
behave better; see:

https://github.com/sabotage-linux/sabotage/blob/master/pkg/kernel-headers

But it might be possible that we can just #define whatever macro the
kernel headers expect to suppress redefinitions, if it's in a reserved
namespace. This would only fix the case where the kernel headers are
included _after_ the libc/userspace ones, but it's not too ugly. I
probably won't get around to it myself but if anyone wants to check
what macros we'd need to define (and where), let's discuss it (cc'ing
musl list).

The cleaner approach is just avoiding including both the kernel
headers and libc/userspace headers for the same things in the same
file. In theory this may be hard in some cases, but I find that I can
almost always fix these sorts of errors during a build by commenting
out one or two #include lines.

Rich

Re: musl and kernel headers [was Re: system-images 1.4.2: od is broken; bzip2 is missing]

[Rob Landley]

On 10/05/2015 08:44 PM, Rich Felker wrote:
> On Mon, Oct 05, 2015 at 10:08:10AM -0500, Rob Landley wrote:
> The cleaner approach is just avoiding including both the kernel
> headers and libc/userspace headers for the same things in the same
> file. In theory this may be hard in some cases, but I find that I can
> almost always fix these sorts of errors during a build by commenting
> out one or two #include lines.

I am _deeply_ curious how you'd get linux/loop.h on a platform where you
need the 32 bit loopback structure definition without including the
kernel header. I struggled with that one a lot back in the busybox days.
(For the 64 bit version you can block copy the structure out of the
header into your program, just like you can block copy everything out of
headers and never actually #include anything, in the name of portability!)

Sadly, the kernel headers are exported for a _reason_. If I need to
syscall something you haven't wrapped, I need the _NR_ and it varies per
target. It's a thing.

Rob

Re: Re: musl and kernel headers [was Re: system-images 1.4.2: od is broken; bzip2 is missing]

[Szabolcs Nagy]

* Rob Landley <rob@landley.net> [2015-10-05 21:24:01 -0500]:
> On 10/05/2015 08:44 PM, Rich Felker wrote:
> > On Mon, Oct 05, 2015 at 10:08:10AM -0500, Rob Landley wrote:
> > The cleaner approach is just avoiding including both the kernel
> > headers and libc/userspace headers for the same things in the same
> > file. In theory this may be hard in some cases, but I find that I can
> > almost always fix these sorts of errors during a build by commenting
> > out one or two #include lines.
> 
> I am _deeply_ curious how you'd get linux/loop.h on a platform where you
> need the 32 bit loopback structure definition without including the
> kernel header. I struggled with that one a lot back in the busybox days.

in theory you can put all usage of the structure in a
separate translation unit where you don't include any
libc headers.

(if you need libc api you can declare prototypes e.g.
int ioctl(int, int,...); but if you need libc types
there then you are in trouble).

> (For the 64 bit version you can block copy the structure out of the
> header into your program, just like you can block copy everything out of
> headers and never actually #include anything, in the name of portability!)
> 
> Sadly, the kernel headers are exported for a _reason_. If I need to
> syscall something you haven't wrapped, I need the _NR_ and it varies per
> target. It's a thing.
> 
> Rob

Re: musl and kernel headers [was Re: system-images 1.4.2: od is broken; bzip2 is missing]

[Rich Felker]

On Mon, Oct 05, 2015 at 09:24:01PM -0500, Rob Landley wrote:
> On 10/05/2015 08:44 PM, Rich Felker wrote:
> > On Mon, Oct 05, 2015 at 10:08:10AM -0500, Rob Landley wrote:
> > The cleaner approach is just avoiding including both the kernel
> > headers and libc/userspace headers for the same things in the same
> > file. In theory this may be hard in some cases, but I find that I can
> > almost always fix these sorts of errors during a build by commenting
> > out one or two #include lines.
> 
> I am _deeply_ curious how you'd get linux/loop.h on a platform where you
> need the 32 bit loopback structure definition without including the
> kernel header.

Sorry I wasn't clear on this; the meaning I intended to convey was
including both kernel and libc/userspace headers for the same things.
Network is the main area affected here. The kernel headers have fixed
up all the gratuitous conflicts with userspace, but the big remaining
ones are places where they want to define types with the exact same
names, which mostly happens in network. So what I was trying to say is
that programs using kernel network headers (legitimately for
linux-specific stuff) are going to best avoid the risk of clashes by
not including libc network headers in the same files.

> Sadly, the kernel headers are exported for a _reason_. If I need to
> syscall something you haven't wrapped, I need the _NR_ and it varies per
> target. It's a thing.

For syscalls we export all SYS_*/__NR_* macros (sys/syscall.h), but
you still might need macro and struct definitions for arguments to the
syscalls or ioctls.

Rich

Re: musl and kernel headers [was Re: system-images 1.4.2: od is broken; bzip2 is missing]

[Denys Vlasenko]

On Tue, Oct 6, 2015 at 4:30 PM, Rich Felker <dalias@aerifal.cx> wrote:
> On Mon, Oct 05, 2015 at 09:24:01PM -0500, Rob Landley wrote:
>> On 10/05/2015 08:44 PM, Rich Felker wrote:
>> > The cleaner approach is just avoiding including both the kernel
>> > headers and libc/userspace headers for the same things in the same
>> > file. In theory this may be hard in some cases, but I find that I can
>> > almost always fix these sorts of errors during a build by commenting
>> > out one or two #include lines.
>>
>> I am _deeply_ curious how you'd get linux/loop.h on a platform where you
>> need the 32 bit loopback structure definition without including the
>> kernel header.
>
> Sorry I wasn't clear on this; the meaning I intended to convey was
> including both kernel and libc/userspace headers for the same things.
> Network is the main area affected here. The kernel headers have fixed
> up all the gratuitous conflicts with userspace, but the big remaining
> ones are places where they want to define types with the exact same
> names, which mostly happens in network. So what I was trying to say is
> that programs using kernel network headers (legitimately for
> linux-specific stuff) are going to best avoid the risk of clashes by
> not including libc network headers in the same files.

For me as a libc user, the gist of the story is:
I switch to musl, things break.

Apart from linux/* includes, utmp/wtmp also broke.

Re: musl and kernel headers [was Re: system-images 1.4.2: od is broken; bzip2 is missing]

[Denys Vlasenko]

On Tue, Oct 6, 2015 at 3:44 AM, Rich Felker <dalias@aerifal.cx> wrote:
>> >
>> >   #include <netinet/in.h>
>> >   #include <linux/if_bridge.h>
>> > results in:
>> >   In file included from /usr/include/linux/if_bridge.h:18,
>> >                    from networking/brctl.c:67:
>> >   /usr/include/linux/in6.h:32: error: redefinition of 'struct in6_addr'
>> >   /usr/include/linux/in6.h:49: error: redefinition of 'struct sockaddr_in6'
>> >   /usr/include/linux/in6.h:59: error: redefinition of 'struct ipv6_mreq'
>> >
>> > and
>> >
>> >   #include <linux/ethtool.h>
>> >   #include <net/ethernet.h>
>> > results in:
>> >   In file included from /usr/include/net/ethernet.h:10,
>> >                    from networking/ifplugd.c:41:
>> >   /usr/include/netinet/if_ether.h:96: error: redefinition of 'struct ethhdr'
>> >
>>
>> That I leave to Rich. :)
>
> Including kernel headers is just really problematic. These days they
> try to make it work on glibc with a complex dance between glibc's
> headers and the kernel headers. You're likely to have the best luck by
> including the libc headers first.

brctl.c  was including <linux/if_bridge.h> after <netinet/in.h>

Re: Re: musl and kernel headers [was Re: system-images 1.4.2: od is broken; bzip2 is missing]

[Rich Felker]

On Tue, Oct 06, 2015 at 06:09:15PM +0200, Denys Vlasenko wrote:
> On Tue, Oct 6, 2015 at 3:44 AM, Rich Felker <dalias@aerifal.cx> wrote:
> >> >
> >> >   #include <netinet/in.h>
> >> >   #include <linux/if_bridge.h>
> >> > results in:
> >> >   In file included from /usr/include/linux/if_bridge.h:18,
> >> >                    from networking/brctl.c:67:
> >> >   /usr/include/linux/in6.h:32: error: redefinition of 'struct in6_addr'
> >> >   /usr/include/linux/in6.h:49: error: redefinition of 'struct sockaddr_in6'
> >> >   /usr/include/linux/in6.h:59: error: redefinition of 'struct ipv6_mreq'
> >> >
> >> > and
> >> >
> >> >   #include <linux/ethtool.h>
> >> >   #include <net/ethernet.h>
> >> > results in:
> >> >   In file included from /usr/include/net/ethernet.h:10,
> >> >                    from networking/ifplugd.c:41:
> >> >   /usr/include/netinet/if_ether.h:96: error: redefinition of 'struct ethhdr'
> >> >
> >>
> >> That I leave to Rich. :)
> >
> > Including kernel headers is just really problematic. These days they
> > try to make it work on glibc with a complex dance between glibc's
> > headers and the kernel headers. You're likely to have the best luck by
> > including the libc headers first.
> 
> brctl.c  was including <linux/if_bridge.h> after <netinet/in.h>

The problem is linux/libc-compat.h, which should fix this, only works
on glibc, by design. See:

#ifndef _LIBC_COMPAT_H
#define _LIBC_COMPAT_H

/* We have included glibc headers... */
#if defined(__GLIBC__)

/* Coordinate with glibc netinet/in.h header. */
#if defined(_NETINET_IN_H)

If you patch it like this:

-#if defined(__GLIBC__)
+#if 1

then it should mostly work but it's still all a big hack. I think
that's what distros are doing. The problem is that the same header is
trying to do two different things:

1. Provide extra linux-kernel-API stuff that's not in the
   libc/userspace headers.

2. Provide definitions of the standard types and constants for uClibc
   and klibc, which don't have complete libc headers and rely on the
   kernel headers for definitions.

These two uses really should be separated out into separate headers so
that the latter only get included explicitly by uClibc and klibc and
otherwise remain completely unused. But that would require coordinated
changes/upgrades which are unlikely to happen. :(

Rich

Re: Re: musl and kernel headers [was Re: system-images 1.4.2: od is broken; bzip2 is missing]

[Denys Vlasenko]

On Thu, Oct 8, 2015 at 6:58 PM, Rich Felker <dalias@aerifal.cx> wrote:
>> > Including kernel headers is just really problematic. These days they
>> > try to make it work on glibc with a complex dance between glibc's
>> > headers and the kernel headers. You're likely to have the best luck by
>> > including the libc headers first.
>>
>> brctl.c  was including <linux/if_bridge.h> after <netinet/in.h>
>
> The problem is linux/libc-compat.h, which should fix this, only works
> on glibc, by design. See:
>
> #ifndef _LIBC_COMPAT_H
> #define _LIBC_COMPAT_H
>
> /* We have included glibc headers... */
> #if defined(__GLIBC__)
>
> /* Coordinate with glibc netinet/in.h header. */
> #if defined(_NETINET_IN_H)
>
> If you patch it like this:
>
> -#if defined(__GLIBC__)
> +#if 1
>
> then it should mostly work but it's still all a big hack. I think
> that's what distros are doing. The problem is that the same header is
> trying to do two different things:
>
> 1. Provide extra linux-kernel-API stuff that's not in the
>    libc/userspace headers.
>
> 2. Provide definitions of the standard types and constants for uClibc
>    and klibc, which don't have complete libc headers and rely on the
>    kernel headers for definitions.
>
> These two uses really should be separated out into separate headers so
> that the latter only get included explicitly by uClibc and klibc and
> otherwise remain completely unused. But that would require coordinated
> changes/upgrades which are unlikely to happen. :(

Looking at kernel's libc-compat.h, it looks like you can get away
with using __UAPI_DEF_foo's like this?


#if  defined(__UAPI_DEF_SOCKADDR_IN) && __UAPI_DEF_SOCKADDR_IN == 1
/* kernel already defined the struct, do nothing */
#else
struct sockaddr_in {
        ...
};
#undef __UAPI_DEF_SOCKADDR_IN
/* tell kernel to not define the struct */
#define __UAPI_DEF_SOCKADDR_IN 0
#endif

Re: Re: musl and kernel headers [was Re: system-images 1.4.2: od is broken; bzip2 is missing]

[Rich Felker]

On Fri, Oct 09, 2015 at 09:11:08PM +0200, Denys Vlasenko wrote:
> On Thu, Oct 8, 2015 at 6:58 PM, Rich Felker <dalias@aerifal.cx> wrote:
> >> > Including kernel headers is just really problematic. These days they
> >> > try to make it work on glibc with a complex dance between glibc's
> >> > headers and the kernel headers. You're likely to have the best luck by
> >> > including the libc headers first.
> >>
> >> brctl.c  was including <linux/if_bridge.h> after <netinet/in.h>
> >
> > The problem is linux/libc-compat.h, which should fix this, only works
> > on glibc, by design. See:
> >
> > #ifndef _LIBC_COMPAT_H
> > #define _LIBC_COMPAT_H
> >
> > /* We have included glibc headers... */
> > #if defined(__GLIBC__)
> >
> > /* Coordinate with glibc netinet/in.h header. */
> > #if defined(_NETINET_IN_H)
> >
> > If you patch it like this:
> >
> > -#if defined(__GLIBC__)
> > +#if 1
> >
> > then it should mostly work but it's still all a big hack. I think
> > that's what distros are doing. The problem is that the same header is
> > trying to do two different things:
> >
> > 1. Provide extra linux-kernel-API stuff that's not in the
> >    libc/userspace headers.
> >
> > 2. Provide definitions of the standard types and constants for uClibc
> >    and klibc, which don't have complete libc headers and rely on the
> >    kernel headers for definitions.
> >
> > These two uses really should be separated out into separate headers so
> > that the latter only get included explicitly by uClibc and klibc and
> > otherwise remain completely unused. But that would require coordinated
> > changes/upgrades which are unlikely to happen. :(
> 
> Looking at kernel's libc-compat.h, it looks like you can get away
> with using __UAPI_DEF_foo's like this?
> 
> 
> #if  defined(__UAPI_DEF_SOCKADDR_IN) && __UAPI_DEF_SOCKADDR_IN == 1
> /* kernel already defined the struct, do nothing */
> #else
> struct sockaddr_in {
>         ...
> };

This would address the case where the kernel header is included first,
but it's not a case I or most of the musl community wants to support,
because there's no guarantee that the kernel's definitions of these
structures will actually be compatible with use elsewhere in the libc
headers, etc.

The other direction, suppressing kernel headers' definition of the
structs, is what we want to work, but they've restricted their logic
for that to only work when __GLIBC__ is defined. :(

> #undef __UAPI_DEF_SOCKADDR_IN
> /* tell kernel to not define the struct */
> #define __UAPI_DEF_SOCKADDR_IN 0
> #endif

We could do something like this but then we would need to keep up with
the list of all the __UAPI defines we need to suppress unwanted kernel
definitions.

What if we could get the kernel to change the #if defined(__GLIBC__)
to #if defined(__GLIBC__) || defined(__UAPI_DONTNEED_DEFS) or similar,
so that there would only be one macro we need to define, and the
kernel would then use the same logic it uses with glibc to suppress
all of these.

Rich

Re: Re: musl and kernel headers [was Re: system-images 1.4.2: od is broken; bzip2 is missing]

[Rob Landley]

On 10/09/2015 02:46 PM, Rich Felker wrote:
> On Fri, Oct 09, 2015 at 09:11:08PM +0200, Denys Vlasenko wrote:
>> #undef __UAPI_DEF_SOCKADDR_IN
>> /* tell kernel to not define the struct */
>> #define __UAPI_DEF_SOCKADDR_IN 0
>> #endif
> 
> We could do something like this but then we would need to keep up with
> the list of all the __UAPI defines we need to suppress unwanted kernel
> definitions.

You have a bug report. This would fix it. Then wait for more bug reports?

Rob

Re: Re: musl and kernel headers [was Re: system-images 1.4.2: od is broken; bzip2 is missing]

[Denys Vlasenko]

On Fri, Oct 9, 2015 at 9:46 PM, Rich Felker <dalias@aerifal.cx> wrote:
>> Looking at kernel's libc-compat.h, it looks like you can get away
>> with using __UAPI_DEF_foo's like this?
>>
>>
>> #if  defined(__UAPI_DEF_SOCKADDR_IN) && __UAPI_DEF_SOCKADDR_IN == 1
>> /* kernel already defined the struct, do nothing */
>> #else
>> struct sockaddr_in {
>>         ...
>> };
>
> This would address the case where the kernel header is included first,
> but it's not a case I or most of the musl community wants to support,
> because there's no guarantee that the kernel's definitions of these
> structures will actually be compatible with use elsewhere in the libc
> headers, etc.

If kernel's definition does not match yours, there is a much
bigger problem than "includes do not compile":
kernel and userspace definitions of these structs *must* match
(modulo harmless things like different typedef names for field types).

So in this case either kernel or libc would need to be fixed.

> The other direction, suppressing kernel headers' definition of the
> structs, is what we want to work, but they've restricted their logic
> for that to only work when __GLIBC__ is defined. :(

Yes, you will have to do by hand the thing which kernel
automagically does for glibc - namely, define to 0:

>> #undef __UAPI_DEF_SOCKADDR_IN
>> /* tell kernel to not define the struct */
>> #define __UAPI_DEF_SOCKADDR_IN 0
>> #endif

> We could do something like this but then we would need to keep up with
> the list of all the __UAPI defines we need to suppress unwanted kernel
> definitions.

Looking at libc-compat.h, this list is at the moment only about
13 defines long:

#define __UAPI_DEF_IN_ADDR              0
#define __UAPI_DEF_IN_IPPROTO           0
#define __UAPI_DEF_IN_PKTINFO           0
#define __UAPI_DEF_IP_MREQ              0
#define __UAPI_DEF_SOCKADDR_IN          0
#define __UAPI_DEF_IN_CLASS             0
#define __UAPI_DEF_IN6_ADDR             0
#define __UAPI_DEF_SOCKADDR_IN6         0
#define __UAPI_DEF_IPV6_MREQ            0
#define __UAPI_DEF_IPPROTO_V6           0
#define __UAPI_DEF_IPV6_OPTIONS         0
#define __UAPI_DEF_IN6_PKTINFO          0
#define __UAPI_DEF_IP6_MTUINFO          0


> What if we could get the kernel to change the #if defined(__GLIBC__)
> to #if defined(__GLIBC__) || defined(__UAPI_DONTNEED_DEFS) or similar,
> so that there would only be one macro we need to define, and the
> kernel would then use the same logic it uses with glibc to suppress
> all of these.

Or ask kernel to remove "define to 0" glibc hack and ask glibc to
do its own job. Why one libc should have preferential treatment?

Or ask kernel to stop using structures with userspace names.
This should not be that hard:

struct __kernel_sockaddr_in {...}
#if __KERNEL__
# define sockaddr_in __kernel_sockaddr_in
#endif

Re: Re: musl and kernel headers [was Re: system-images 1.4.2: od is broken; bzip2 is missing]

[Ruben Winistörfer]

[-- Attachment #1: Type: text/plain, Size: 3375 bytes --]

I just have modified the Alpine Linux kernel header patches to use them 
with the 4.2.3 version.
(Not sure if that is a good idea, but why not try it...)

So I am wondering right now, if they haven't solved the problem already 
you're discussing about.

http://git.alpinelinux.org/cgit/aports/tree/main/linux-headers

There is a patch for libc-compat.h...

Maybe it helps, otherwise ignore my "interruption". ;-)


Ruben



Denys Vlasenko schrieb:
> On Fri, Oct 9, 2015 at 9:46 PM, Rich Felker<dalias@aerifal.cx>  wrote:
>>> Looking at kernel's libc-compat.h, it looks like you can get away
>>> with using __UAPI_DEF_foo's like this?
>>>
>>>
>>> #if  defined(__UAPI_DEF_SOCKADDR_IN)&&  __UAPI_DEF_SOCKADDR_IN == 1
>>> /* kernel already defined the struct, do nothing */
>>> #else
>>> struct sockaddr_in {
>>>          ...
>>> };
>> This would address the case where the kernel header is included first,
>> but it's not a case I or most of the musl community wants to support,
>> because there's no guarantee that the kernel's definitions of these
>> structures will actually be compatible with use elsewhere in the libc
>> headers, etc.
>
> If kernel's definition does not match yours, there is a much
> bigger problem than "includes do not compile":
> kernel and userspace definitions of these structs *must* match
> (modulo harmless things like different typedef names for field types).
>
> So in this case either kernel or libc would need to be fixed.
>
>> The other direction, suppressing kernel headers' definition of the
>> structs, is what we want to work, but they've restricted their logic
>> for that to only work when __GLIBC__ is defined. :(
>
> Yes, you will have to do by hand the thing which kernel
> automagically does for glibc - namely, define to 0:
>
>>> #undef __UAPI_DEF_SOCKADDR_IN
>>> /* tell kernel to not define the struct */
>>> #define __UAPI_DEF_SOCKADDR_IN 0
>>> #endif
>
>> We could do something like this but then we would need to keep up with
>> the list of all the __UAPI defines we need to suppress unwanted kernel
>> definitions.
>
> Looking at libc-compat.h, this list is at the moment only about
> 13 defines long:
>
> #define __UAPI_DEF_IN_ADDR              0
> #define __UAPI_DEF_IN_IPPROTO           0
> #define __UAPI_DEF_IN_PKTINFO           0
> #define __UAPI_DEF_IP_MREQ              0
> #define __UAPI_DEF_SOCKADDR_IN          0
> #define __UAPI_DEF_IN_CLASS             0
> #define __UAPI_DEF_IN6_ADDR             0
> #define __UAPI_DEF_SOCKADDR_IN6         0
> #define __UAPI_DEF_IPV6_MREQ            0
> #define __UAPI_DEF_IPPROTO_V6           0
> #define __UAPI_DEF_IPV6_OPTIONS         0
> #define __UAPI_DEF_IN6_PKTINFO          0
> #define __UAPI_DEF_IP6_MTUINFO          0
>
>
>> What if we could get the kernel to change the #if defined(__GLIBC__)
>> to #if defined(__GLIBC__) || defined(__UAPI_DONTNEED_DEFS) or similar,
>> so that there would only be one macro we need to define, and the
>> kernel would then use the same logic it uses with glibc to suppress
>> all of these.
>
> Or ask kernel to remove "define to 0" glibc hack and ask glibc to
> do its own job. Why one libc should have preferential treatment?
>
> Or ask kernel to stop using structures with userspace names.
> This should not be that hard:
>
> struct __kernel_sockaddr_in {...}
> #if __KERNEL__
> # define sockaddr_in __kernel_sockaddr_in
> #endif


[-- Attachment #2: Type: text/html, Size: 4291 bytes --]

Re: Re: musl and kernel headers [was Re: system-images 1.4.2: od is broken; bzip2 is missing]

[Szabolcs Nagy]

* Denys Vlasenko <vda.linux@googlemail.com> [2015-10-13 14:10:24 +0200]:
> On Fri, Oct 9, 2015 at 9:46 PM, Rich Felker <dalias@aerifal.cx> wrote:
> >> Looking at kernel's libc-compat.h, it looks like you can get away
> >> with using __UAPI_DEF_foo's like this?
> >>
> >>
> >> #if  defined(__UAPI_DEF_SOCKADDR_IN) && __UAPI_DEF_SOCKADDR_IN == 1
> >> /* kernel already defined the struct, do nothing */
> >> #else
> >> struct sockaddr_in {
> >>         ...
> >> };
> >
> > This would address the case where the kernel header is included first,
> > but it's not a case I or most of the musl community wants to support,
> > because there's no guarantee that the kernel's definitions of these
> > structures will actually be compatible with use elsewhere in the libc
> > headers, etc.
> 
> If kernel's definition does not match yours, there is a much
> bigger problem than "includes do not compile":
> kernel and userspace definitions of these structs *must* match
> (modulo harmless things like different typedef names for field types).
> 
> So in this case either kernel or libc would need to be fixed.
> 

why?

in practice most types are c abi compatible with the kernel
because translating the types at the syscall boundary is
painful/impossible.

but even with compatible binary representation there is
plenty space for disagreement between kernel and libc on
the source level. (of course code that includes both libc
and kernel headers might not care about posix namespace
violations or undefined behaviour in kernel headers..)

and libc-compat does not cover all conflicting cases
(i assume they just add workarouds when somebody hits
a conflict), e.g. sys/inotify.h and linux/inotify.h are
in conflict (and linux/inotify.h is not even standard c).

Re: Re: musl and kernel headers [was Re: system-images 1.4.2: od is broken; bzip2 is missing]

[Rich Felker]

On Tue, Oct 13, 2015 at 04:53:35PM +0200, Szabolcs Nagy wrote:
> * Denys Vlasenko <vda.linux@googlemail.com> [2015-10-13 14:10:24 +0200]:
> > On Fri, Oct 9, 2015 at 9:46 PM, Rich Felker <dalias@aerifal.cx> wrote:
> > >> Looking at kernel's libc-compat.h, it looks like you can get away
> > >> with using __UAPI_DEF_foo's like this?
> > >>
> > >>
> > >> #if  defined(__UAPI_DEF_SOCKADDR_IN) && __UAPI_DEF_SOCKADDR_IN == 1
> > >> /* kernel already defined the struct, do nothing */
> > >> #else
> > >> struct sockaddr_in {
> > >>         ...
> > >> };
> > >
> > > This would address the case where the kernel header is included first,
> > > but it's not a case I or most of the musl community wants to support,
> > > because there's no guarantee that the kernel's definitions of these
> > > structures will actually be compatible with use elsewhere in the libc
> > > headers, etc.
> > 
> > If kernel's definition does not match yours, there is a much
> > bigger problem than "includes do not compile":
> > kernel and userspace definitions of these structs *must* match
> > (modulo harmless things like different typedef names for field types).
> > 
> > So in this case either kernel or libc would need to be fixed.
> 
> why?
> 
> in practice most types are c abi compatible with the kernel
> because translating the types at the syscall boundary is
> painful/impossible.
> 
> but even with compatible binary representation there is
> plenty space for disagreement between kernel and libc on
> the source level. (of course code that includes both libc
> and kernel headers might not care about posix namespace
> violations or undefined behaviour in kernel headers..)
> 
> and libc-compat does not cover all conflicting cases
> (i assume they just add workarouds when somebody hits
> a conflict), e.g. sys/inotify.h and linux/inotify.h are
> in conflict (and linux/inotify.h is not even standard c).

Indeed the problem here is source compatibility, not binary
compatibility. Issues like names of types, choice of distinct types
that have the same size and representation but which are not
compatible types (which make problems if you take the address of the
member, including possibly aliasing problems which are real-world
bugs), etc. If static linking of libc is used along with LTO these
could spill deep into internals and result in broken codegen.

Rich

Re: Re: musl and kernel headers [was Re: system-images 1.4.2: od is broken; bzip2 is missing]

[Denys Vlasenko]

On Tue, Oct 13, 2015 at 5:05 PM, Rich Felker <dalias@aerifal.cx> wrote:
>> > > This would address the case where the kernel header is included first,
>> > > but it's not a case I or most of the musl community wants to support,
>> > > because there's no guarantee that the kernel's definitions of these
>> > > structures will actually be compatible with use elsewhere in the libc
>> > > headers, etc.
>> >
>> > If kernel's definition does not match yours, there is a much
>> > bigger problem than "includes do not compile":
>> > kernel and userspace definitions of these structs *must* match
>> > (modulo harmless things like different typedef names for field types).
>> >
>> > So in this case either kernel or libc would need to be fixed.
>>
>> why?
>>
>> in practice most types are c abi compatible with the kernel
>> because translating the types at the syscall boundary is
>> painful/impossible.
>>
>> but even with compatible binary representation there is
>> plenty space for disagreement between kernel and libc on
>> the source level. (of course code that includes both libc
>> and kernel headers might not care about posix namespace
>> violations or undefined behaviour in kernel headers..)
>>
>> and libc-compat does not cover all conflicting cases
>> (i assume they just add workarouds when somebody hits
>> a conflict), e.g. sys/inotify.h and linux/inotify.h are
>> in conflict (and linux/inotify.h is not even standard c).
>
> Indeed the problem here is source compatibility, not binary
> compatibility. Issues like names of types, choice of distinct types
> that have the same size and representation but which are not
> compatible types (which make problems if you take the address of the
> member, including possibly aliasing problems which are real-world
> bugs), etc.

It's not that bad in practice.

In C, typedefs are not new types. uint16_t, u16 and __u16
are all just aliases to "unsigned short".

You won't get a conflict because different typedefs
were used to declare a struct member whose address you are
taking and passing to some function.
If signedness and width match, then it will work without casts
or compiler errors.

Re: Re: musl and kernel headers [was Re: system-images 1.4.2: od is broken; bzip2 is missing]

[Rich Felker]

On Tue, Oct 13, 2015 at 08:02:47PM +0200, Denys Vlasenko wrote:
> On Tue, Oct 13, 2015 at 5:05 PM, Rich Felker <dalias@aerifal.cx> wrote:
> >> > > This would address the case where the kernel header is included first,
> >> > > but it's not a case I or most of the musl community wants to support,
> >> > > because there's no guarantee that the kernel's definitions of these
> >> > > structures will actually be compatible with use elsewhere in the libc
> >> > > headers, etc.
> >> >
> >> > If kernel's definition does not match yours, there is a much
> >> > bigger problem than "includes do not compile":
> >> > kernel and userspace definitions of these structs *must* match
> >> > (modulo harmless things like different typedef names for field types).
> >> >
> >> > So in this case either kernel or libc would need to be fixed.
> >>
> >> why?
> >>
> >> in practice most types are c abi compatible with the kernel
> >> because translating the types at the syscall boundary is
> >> painful/impossible.
> >>
> >> but even with compatible binary representation there is
> >> plenty space for disagreement between kernel and libc on
> >> the source level. (of course code that includes both libc
> >> and kernel headers might not care about posix namespace
> >> violations or undefined behaviour in kernel headers..)
> >>
> >> and libc-compat does not cover all conflicting cases
> >> (i assume they just add workarouds when somebody hits
> >> a conflict), e.g. sys/inotify.h and linux/inotify.h are
> >> in conflict (and linux/inotify.h is not even standard c).
> >
> > Indeed the problem here is source compatibility, not binary
> > compatibility. Issues like names of types, choice of distinct types
> > that have the same size and representation but which are not
> > compatible types (which make problems if you take the address of the
> > member, including possibly aliasing problems which are real-world
> > bugs), etc.
> 
> It's not that bad in practice.
> 
> In C, typedefs are not new types. uint16_t, u16 and __u16
> are all just aliases to "unsigned short".
> 
> You won't get a conflict because different typedefs
> were used to declare a struct member whose address you are
> taking and passing to some function.
> If signedness and width match, then it will work without casts
> or compiler errors.

The problem is that u32 might be unsigned int or unsigned long (either
possible on ILP32), u64 might be unsigned long or unsigned long long
(either possible on LP64), etc. Also structs with different tags are
distinct types. These are not ABI issues for machine-code object files
but they are source-level compatibility issues and linking (arguably
ABI) issues for LTO object files. They're also C++ issues; for
instance it's possible that one translation unit declares a function
taking a pointer to some network function without including the kernel
header, and another defines it while including the kernel header, and
the struct names or types mismatch (even in a layout-compatible way),
the name mangling will be different and the code won't link.

Rich

Re: Re: musl and kernel headers [was Re: system-images 1.4.2: od is broken; bzip2 is missing]

[Rich Felker]

On Tue, Oct 13, 2015 at 02:10:24PM +0200, Denys Vlasenko wrote:
> > The other direction, suppressing kernel headers' definition of the
> > structs, is what we want to work, but they've restricted their logic
> > for that to only work when __GLIBC__ is defined. :(
> 
> Yes, you will have to do by hand the thing which kernel
> automagically does for glibc - namely, define to 0:
> 
> >> #undef __UAPI_DEF_SOCKADDR_IN
> >> /* tell kernel to not define the struct */
> >> #define __UAPI_DEF_SOCKADDR_IN 0
> >> #endif
> 
> > We could do something like this but then we would need to keep up with
> > the list of all the __UAPI defines we need to suppress unwanted kernel
> > definitions.
> 
> Looking at libc-compat.h, this list is at the moment only about
> 13 defines long:
> 
> #define __UAPI_DEF_IN_ADDR              0
> #define __UAPI_DEF_IN_IPPROTO           0
> #define __UAPI_DEF_IN_PKTINFO           0
> #define __UAPI_DEF_IP_MREQ              0
> #define __UAPI_DEF_SOCKADDR_IN          0
> #define __UAPI_DEF_IN_CLASS             0
> #define __UAPI_DEF_IN6_ADDR             0
> #define __UAPI_DEF_SOCKADDR_IN6         0
> #define __UAPI_DEF_IPV6_MREQ            0
> #define __UAPI_DEF_IPPROTO_V6           0
> #define __UAPI_DEF_IPV6_OPTIONS         0
> #define __UAPI_DEF_IN6_PKTINFO          0
> #define __UAPI_DEF_IP6_MTUINFO          0

Thanks for making the list! For the time being I don't think it would
be too objectionable to simply define these in musl. Does anyone else
have an opinion on it?

> > What if we could get the kernel to change the #if defined(__GLIBC__)
> > to #if defined(__GLIBC__) || defined(__UAPI_DONTNEED_DEFS) or similar,
> > so that there would only be one macro we need to define, and the
> > kernel would then use the same logic it uses with glibc to suppress
> > all of these.
> 
> Or ask kernel to remove "define to 0" glibc hack and ask glibc to
> do its own job. Why one libc should have preferential treatment?

Well they don't want regressions; changing this would make old
versions of glibc break. Presumably before all this UAPI "cleanup"
(I'm not sure it can really be called "clean" though) there was some
other mechanism that prevented things from breaking when they were
included with glibc. In any case current versions of glibc would be
broken by a change now. This type of interdependency is part of why I
think the whole thing (trying to make kernel headers for stuf that's
already defined in libc usable from userspace) was a bad idea to begin
with.

> Or ask kernel to stop using structures with userspace names.
> This should not be that hard:
> 
> struct __kernel_sockaddr_in {...}
> #if __KERNEL__
> # define sockaddr_in __kernel_sockaddr_in
> #endif

Yes but then some patchup would be needed for uClibc and other libcs
that actually want the kernel to define the structs. This would work
better if the #define was in the other direction:

# define __kernel_sockaddr_in sockaddr_in

and done _before_ declaration of the structure, so that the actual
struct tag would end up being the desired name in this case (both for
kernel-internal and for uClibc, etc. use).

Rich

Re: Re: musl and kernel headers [was Re: system-images 1.4.2: od is broken; bzip2 is missing]

[Isaac Dunham]

On Tue, Oct 13, 2015 at 11:10:49AM -0400, Rich Felker wrote:
> On Tue, Oct 13, 2015 at 02:10:24PM +0200, Denys Vlasenko wrote:
> > > The other direction, suppressing kernel headers' definition of the
> > > structs, is what we want to work, but they've restricted their logic
> > > for that to only work when __GLIBC__ is defined. :(
> > 
> > Yes, you will have to do by hand the thing which kernel
> > automagically does for glibc - namely, define to 0:
> > 
> > >> #undef __UAPI_DEF_SOCKADDR_IN
> > >> /* tell kernel to not define the struct */
> > >> #define __UAPI_DEF_SOCKADDR_IN 0
> > >> #endif
> > 
> > > We could do something like this but then we would need to keep up with
> > > the list of all the __UAPI defines we need to suppress unwanted kernel
> > > definitions.
> > 
> > Looking at libc-compat.h, this list is at the moment only about
> > 13 defines long:
> > 
> > #define __UAPI_DEF_IN_ADDR              0
> > #define __UAPI_DEF_IN_IPPROTO           0
> > #define __UAPI_DEF_IN_PKTINFO           0
> > #define __UAPI_DEF_IP_MREQ              0
> > #define __UAPI_DEF_SOCKADDR_IN          0
> > #define __UAPI_DEF_IN_CLASS             0
> > #define __UAPI_DEF_IN6_ADDR             0
> > #define __UAPI_DEF_SOCKADDR_IN6         0
> > #define __UAPI_DEF_IPV6_MREQ            0
> > #define __UAPI_DEF_IPPROTO_V6           0
> > #define __UAPI_DEF_IPV6_OPTIONS         0
> > #define __UAPI_DEF_IN6_PKTINFO          0
> > #define __UAPI_DEF_IP6_MTUINFO          0
> 
> Thanks for making the list! For the time being I don't think it would
> be too objectionable to simply define these in musl. Does anyone else
> have an opinion on it?

I'd like to see it for the time being, though long-term I still assume
that the only real solution would be something like the proposed change
in libc-compat.h:
-#ifdef __GLIBC__
+#if defined(__GLIBC__) || defined(__UAPI_DONTNEED_DEFS)

Thanks,
Isaac Dunham