| Message ID | 047d7b6d8c8c4d13600522a1ffa5@google.com |
|---|---|
| State | New |
| Headers | show |
On 10/21/2015 09:00 PM, Doug Evans wrote: > I happened to notice local prefixes not working with musl. > Fixes thusly. > Index: config/linux.h > =================================================================== > --- config/linux.h (revision 229130) > +++ config/linux.h (working copy) > @@ -174,6 +174,7 @@ > #define INCLUDE_DEFAULTS \ > { \ > INCLUDE_DEFAULTS_MUSL_GPP \ > + INCLUDE_DEFAULTS_MUSL_LOCAL \ > INCLUDE_DEFAULTS_MUSL_PREFIX \ > INCLUDE_DEFAULTS_MUSL_CROSS \ > INCLUDE_DEFAULTS_MUSL_TOOL \ Looks pretty obvious given that the macro isn't otherwise used AFAICT. However, I have no idea whether the order is right, since the purpose of all this code here is apparently only to provide a different order than the default. So, someone who worked on the original musl patches should comment. I would also like to know precisely which ordering change over the default is required, and that it be documented. Ideally we'd come up with a solution that makes us not duplicate all this stuff in linux.h. Bernd
On Fri, Oct 23, 2015 at 10:08 AM, Bernd Schmidt <bschmidt@redhat.com> wrote: > > On 10/21/2015 09:00 PM, Doug Evans wrote: >> >> I happened to notice local prefixes not working with musl. >> Fixes thusly. > > >> Index: config/linux.h >> =================================================================== >> --- config/linux.h (revision 229130) >> +++ config/linux.h (working copy) >> @@ -174,6 +174,7 @@ >> #define INCLUDE_DEFAULTS \ >> { \ >> INCLUDE_DEFAULTS_MUSL_GPP \ >> + INCLUDE_DEFAULTS_MUSL_LOCAL \ >> INCLUDE_DEFAULTS_MUSL_PREFIX \ >> INCLUDE_DEFAULTS_MUSL_CROSS \ >> INCLUDE_DEFAULTS_MUSL_TOOL \ > > > Looks pretty obvious given that the macro isn't otherwise used AFAICT. However, I have no idea whether the order is right, since the purpose of all this code here is apparently only to provide a different order than the default. > > So, someone who worked on the original musl patches should comment. I would also like to know precisely which ordering change over the default is required, and that it be documented. Ideally we'd come up with a solution that makes us not duplicate all this stuff in linux.h. > > > Bernd Crap, sorry for the resend. Grrrr gmail ... The only significant different AFAICT is that GCC_INCLUDE_DIR is moved to later (last). Why this is is briefly described in the intro comment: config/linux.h: /* musl avoids problematic includes by rearranging the include directories. * Unfortunately, this is mostly duplicated from cppdefault.c */ I've put LOCAL in the same place as the default (as defined by cppdefault.c), so one could separate the issues here ... 1) Where does LOCAL go for musl? 2) More clarification as to why musl doesn't use the default. ... and thus not block this patch on a more global musl rethink. OTOH I'm in no rush. :-)
On 10/23/2015 07:36 PM, Doug Evans wrote: > The only significant different AFAICT is that GCC_INCLUDE_DIR is moved > to later (last). > Why this is is briefly described in the intro comment: > > config/linux.h: > > /* musl avoids problematic includes by rearranging the include > directories. > * Unfortunately, this is mostly duplicated from cppdefault.c */ I saw that, but it's not a helpful comment - it has insufficient definitions of "problematic" and "rearranging". > I've put LOCAL in the same place as the default (as defined by > cppdefault.c), > so one could separate the issues here ... > > 1) Where does LOCAL go for musl? > 2) More clarification as to why musl doesn't use the default. > > ... and thus not block this patch on a more global musl rethink. Yeah, that part wasn't addressed at you. If the musl folks are happy with your choice of order your patch can go in, but I'd ask them to strongly consider a different approach. Bernd
On 23/10/15 18:39, Doug Evans wrote: > On Fri, Oct 23, 2015 at 10:08 AM, Bernd Schmidt <bschmidt@redhat.com> wrote: >> >> On 10/21/2015 09:00 PM, Doug Evans wrote: >>> >>> I happened to notice local prefixes not working with musl. >>> Fixes thusly. >> >> >>> Index: config/linux.h >>> =================================================================== >>> --- config/linux.h (revision 229130) >>> +++ config/linux.h (working copy) >>> @@ -174,6 +174,7 @@ >>> #define INCLUDE_DEFAULTS \ >>> { \ >>> INCLUDE_DEFAULTS_MUSL_GPP \ >>> + INCLUDE_DEFAULTS_MUSL_LOCAL \ >>> INCLUDE_DEFAULTS_MUSL_PREFIX \ >>> INCLUDE_DEFAULTS_MUSL_CROSS \ >>> INCLUDE_DEFAULTS_MUSL_TOOL \ >> >> >> Looks pretty obvious given that the macro isn't otherwise used AFAICT. However, I have no idea whether the order is right, since the purpose of all this code here is apparently only to provide a different order than the default. >> >> So, someone who worked on the original musl patches should comment. I would also like to know precisely which ordering change over the default is required, and that it be documented. Ideally we'd come up with a solution that makes us not duplicate all this stuff in linux.h. >> >> >> Bernd > > Crap, sorry for the resend. Grrrr gmail ... > > The only significant different AFAICT is that GCC_INCLUDE_DIR is moved > to later (last). > Why this is is briefly described in the intro comment: > > config/linux.h: > /* musl avoids problematic includes by rearranging the include > directories. > * Unfortunately, this is mostly duplicated from cppdefault.c */ > > I've put LOCAL in the same place as the default (as defined by cppdefault.c), > so one could separate the issues here ... > > 1) Where does LOCAL go for musl? LOCAL should go the same place as in cppdefault.c so the patch is ok. > 2) More clarification as to why musl doesn't use the default. > ... and thus not block this patch on a more global musl rethink. > OTOH I'm in no rush. :-) > musl moves gcc includes after libc includes, that's the only change. i think bsd libcs do the same, compiler headers interfering with libc headers is problematic (e.g. FLT_ROUNDS is wrong in gcc float.h, applications shouldn't see that), i'm not sure why glibc wants the current default order, but i do see some include_next magic dance between glibc and gcc headers.. i think that should be fixed but most likely there is some reason for it...
On Fri, 23 Oct 2015, Szabolcs Nagy wrote: > i think bsd libcs do the same, compiler headers interfering > with libc headers is problematic (e.g. FLT_ROUNDS is wrong > in gcc float.h, applications shouldn't see that), i'm not sure FLT_ROUNDS is an ordinary compiler bug (bug 30569), should be fixable reasonably straightforwardly as outlined at <https://gcc.gnu.org/ml/gcc/2013-11/msg00317.html>, probably within say a week's work if most architecture-specific changes are left to architecture maintainers.
On 23/10/15 21:20, Joseph Myers wrote: > On Fri, 23 Oct 2015, Szabolcs Nagy wrote: > >> i think bsd libcs do the same, compiler headers interfering >> with libc headers is problematic (e.g. FLT_ROUNDS is wrong >> in gcc float.h, applications shouldn't see that), i'm not sure > > FLT_ROUNDS is an ordinary compiler bug (bug 30569), should be fixable > reasonably straightforwardly as outlined at > <https://gcc.gnu.org/ml/gcc/2013-11/msg00317.html>, probably within say a > week's work if most architecture-specific changes are left to architecture > maintainers. musl tries to support old compilers in general (it can be built with gcc 3.x, and it should be possible to use with a wider range of compilers with reasonably consistent semantics, so fixing that bug in gcc does not help much.) a better example would be stddef.h (it has incompatible definition of NULL, max_align_t etc, the ifdefs in gcc are fragile and none of the __need_FOO patterns match the ones musl use). i think in general the higher level layer should come first (c++ first, then libc, then compiler include paths), so the one closer to the user gets a chance to override the ones after it, stdc-predef.h was a good step toward that.
On Mon, Oct 26, 2015 at 12:32:01PM +0000, Szabolcs Nagy wrote: > On 23/10/15 21:20, Joseph Myers wrote: > >On Fri, 23 Oct 2015, Szabolcs Nagy wrote: > > > >>i think bsd libcs do the same, compiler headers interfering > >>with libc headers is problematic (e.g. FLT_ROUNDS is wrong > >>in gcc float.h, applications shouldn't see that), i'm not sure > > > >FLT_ROUNDS is an ordinary compiler bug (bug 30569), should be fixable > >reasonably straightforwardly as outlined at > ><https://gcc.gnu.org/ml/gcc/2013-11/msg00317.html>, probably within say a > >week's work if most architecture-specific changes are left to architecture > >maintainers. > > musl tries to support old compilers in general (it can be built > with gcc 3.x, and it should be possible to use with a wider range > of compilers with reasonably consistent semantics, so fixing that > bug in gcc does not help much.) > > a better example would be stddef.h (it has incompatible definition > of NULL, max_align_t etc, the ifdefs in gcc are fragile and none > of the __need_FOO patterns match the ones musl use). > > i think in general the higher level layer should come first > (c++ first, then libc, then compiler include paths), so the one > closer to the user gets a chance to override the ones after it, > stdc-predef.h was a good step toward that. musl explicitly does not support using a mix of libc headers and compiler-provided freestanding headers. While there may be circumstances under which no effective breakage occurs, this is merely by chance and is not a supported usage. No effort is made by musl to interact with the gcc headers (e.g. defining the macros they use to prevent multiple definitions or control multiple inclusion, or testing for whether they have already been included are made). It is necessary to use the BSD-style header order, so that the libc headers get used instead of the compiler-provided ones, to have a reliable musl target. Rich
On Fri, Oct 23, 2015 at 11:34 AM, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > On 23/10/15 18:39, Doug Evans wrote: >> >> On Fri, Oct 23, 2015 at 10:08 AM, Bernd Schmidt <bschmidt@redhat.com> >> wrote: >>> >>> >>> On 10/21/2015 09:00 PM, Doug Evans wrote: >>>> >>>> >>>> I happened to notice local prefixes not working with musl. >>>> Fixes thusly. >>> >>> >>> >>>> Index: config/linux.h >>>> =================================================================== >>>> --- config/linux.h (revision 229130) >>>> +++ config/linux.h (working copy) >>>> @@ -174,6 +174,7 @@ >>>> #define INCLUDE_DEFAULTS \ >>>> { \ >>>> INCLUDE_DEFAULTS_MUSL_GPP \ >>>> + INCLUDE_DEFAULTS_MUSL_LOCAL \ >>>> INCLUDE_DEFAULTS_MUSL_PREFIX \ >>>> INCLUDE_DEFAULTS_MUSL_CROSS \ >>>> INCLUDE_DEFAULTS_MUSL_TOOL \ >>> >>> >>> >>> Looks pretty obvious given that the macro isn't otherwise used AFAICT. >>> However, I have no idea whether the order is right, since the purpose of all >>> this code here is apparently only to provide a different order than the >>> default. >>> >>> So, someone who worked on the original musl patches should comment. I >>> would also like to know precisely which ordering change over the default is >>> required, and that it be documented. Ideally we'd come up with a solution >>> that makes us not duplicate all this stuff in linux.h. >>> >>> >>> Bernd >> >> >> Crap, sorry for the resend. Grrrr gmail ... >> >> The only significant different AFAICT is that GCC_INCLUDE_DIR is moved >> to later (last). >> Why this is is briefly described in the intro comment: >> >> config/linux.h: >> /* musl avoids problematic includes by rearranging the include >> directories. >> * Unfortunately, this is mostly duplicated from cppdefault.c */ >> >> I've put LOCAL in the same place as the default (as defined by >> cppdefault.c), >> so one could separate the issues here ... >> >> 1) Where does LOCAL go for musl? > > > LOCAL should go the same place as in cppdefault.c > so the patch is ok. Committed. Thanks.
On Mon, 26 Oct 2015, Szabolcs Nagy wrote: > > FLT_ROUNDS is an ordinary compiler bug (bug 30569), should be fixable > > reasonably straightforwardly as outlined at > > <https://gcc.gnu.org/ml/gcc/2013-11/msg00317.html>, probably within say a > > week's work if most architecture-specific changes are left to architecture > > maintainers. > > musl tries to support old compilers in general (it can be built > with gcc 3.x, and it should be possible to use with a wider range > of compilers with reasonably consistent semantics, so fixing that > bug in gcc does not help much.) Well, the general expectation in the GNU system is that GCC and glibc may work around each other's issues if the one doing the working around is responsible for the interface that needs the workaround - but also that interfaces required for freestanding implementations are GCC's responsibility while interfaces involving library functions are the C library's responsibility. GCC fixincludes doesn't try to fix library issues not relevant for GCC and its tests unless they actually break use of a header with GCC, and glibc doesn't try to fix issues with headers provided by GCC. (There may be the odd deviation from that starting point - GCC provides stdatomic.h because it's so closely linked to the compiler despite not being required of freestanding implementations, and GCC would not start to provide libm in future if adopting TS 18661-1 despite it requiring more library functionality for freestanding implementations.)
On Mon, 26 Oct 2015, Rich Felker wrote: > musl explicitly does not support using a mix of libc headers and > compiler-provided freestanding headers. While there may be In that case the GCC ports for musl should define USER_H = $(EXTRA_HEADERS) like t-openbsd does. (Of course that won't work for multilib builds supporting different C libraries with different multilibs.)
On Mon, Oct 26, 2015 at 11:42:37PM +0000, Joseph Myers wrote: > On Mon, 26 Oct 2015, Rich Felker wrote: > > > musl explicitly does not support using a mix of libc headers and > > compiler-provided freestanding headers. While there may be > > In that case the GCC ports for musl should define USER_H = > $(EXTRA_HEADERS) like t-openbsd does. (Of course that won't work for > multilib builds supporting different C libraries with different > multilibs.) This sounds interesting. Are there practical ways it's a better solution than what linux.h is doing now for musl? Inability to support multilib well is something I've wondered if we could improve on, but from what you said it sounds like this wouldn't help. Rich
On Mon, 26 Oct 2015, Rich Felker wrote: > On Mon, Oct 26, 2015 at 11:42:37PM +0000, Joseph Myers wrote: > > On Mon, 26 Oct 2015, Rich Felker wrote: > > > > > musl explicitly does not support using a mix of libc headers and > > > compiler-provided freestanding headers. While there may be > > > > In that case the GCC ports for musl should define USER_H = > > $(EXTRA_HEADERS) like t-openbsd does. (Of course that won't work for > > multilib builds supporting different C libraries with different > > multilibs.) > > This sounds interesting. Are there practical ways it's a better > solution than what linux.h is doing now for musl? Inability to support Well, it ensures the installed compiler can't find the freestanding headers at all, because they're not installed (other than architecture-specific intrinsics headers).
On Tue, Oct 27, 2015 at 12:16:16AM +0000, Joseph Myers wrote: > On Mon, 26 Oct 2015, Rich Felker wrote: > > > On Mon, Oct 26, 2015 at 11:42:37PM +0000, Joseph Myers wrote: > > > On Mon, 26 Oct 2015, Rich Felker wrote: > > > > > > > musl explicitly does not support using a mix of libc headers and > > > > compiler-provided freestanding headers. While there may be > > > > > > In that case the GCC ports for musl should define USER_H = > > > $(EXTRA_HEADERS) like t-openbsd does. (Of course that won't work for > > > multilib builds supporting different C libraries with different > > > multilibs.) > > > > This sounds interesting. Are there practical ways it's a better > > solution than what linux.h is doing now for musl? Inability to support > > Well, it ensures the installed compiler can't find the freestanding > headers at all, because they're not installed (other than > architecture-specific intrinsics headers). Oh. I think that breaks Linux kernel builds, which use -nostdinc then add back the gcc include directory or something like that. I don't remember the details. Rich
Index: config/linux.h =================================================================== --- config/linux.h (revision 229130) +++ config/linux.h (working copy) @@ -174,6 +174,7 @@ #define INCLUDE_DEFAULTS \ { \ INCLUDE_DEFAULTS_MUSL_GPP \ + INCLUDE_DEFAULTS_MUSL_LOCAL \ INCLUDE_DEFAULTS_MUSL_PREFIX \ INCLUDE_DEFAULTS_MUSL_CROSS \ INCLUDE_DEFAULTS_MUSL_TOOL \