[PATCH] Add missing INCLUDE_DEFAULTS_MUSL_LOCAL

[PATCH] Add missing INCLUDE_DEFAULTS_MUSL_LOCAL

[Doug Evans]

Heya.

I happened to notice local prefixes not working with musl.
Fixes thusly.

2015-10-21  Doug Evans  <dje@google.com>

	* config/linux.h (INCLUDE_DEFAULTS): Add INCLUDE_DEFAULTS_MUSL_LOCAL.

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				\

Re: [PATCH] Add missing INCLUDE_DEFAULTS_MUSL_LOCAL

[Bernd Schmidt]

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

Re: [PATCH] Add missing INCLUDE_DEFAULTS_MUSL_LOCAL

[Doug Evans]

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. :-)

Re: [PATCH] Add missing INCLUDE_DEFAULTS_MUSL_LOCAL

[Bernd Schmidt]

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

Re: [PATCH] Add missing INCLUDE_DEFAULTS_MUSL_LOCAL

[Szabolcs Nagy]

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...

Re: [PATCH] Add missing INCLUDE_DEFAULTS_MUSL_LOCAL

[Joseph Myers]

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.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Add missing INCLUDE_DEFAULTS_MUSL_LOCAL

[Szabolcs Nagy]

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.

Re: [PATCH] Add missing INCLUDE_DEFAULTS_MUSL_LOCAL

[Rich Felker]

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

Re: [PATCH] Add missing INCLUDE_DEFAULTS_MUSL_LOCAL

[Doug Evans]

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.

Re: [PATCH] Add missing INCLUDE_DEFAULTS_MUSL_LOCAL

[Joseph Myers]

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.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Add missing INCLUDE_DEFAULTS_MUSL_LOCAL

[Joseph Myers]

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.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Add missing INCLUDE_DEFAULTS_MUSL_LOCAL

[Rich Felker]

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

Re: [PATCH] Add missing INCLUDE_DEFAULTS_MUSL_LOCAL

[Joseph Myers]

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).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Add missing INCLUDE_DEFAULTS_MUSL_LOCAL

[Rich Felker]

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