ARM: Don't define _SYS_AUXV_H in sysdep.h

sysdep.h was defining _SYS_AUXV_H in order to avoid an include guard check
in hwcap.h. Unfortunately it didn't undefine it so it could leak out into
code and caused a build failure with -Wimplicit-function-declaration
building tst-auxv on ARM.

ChangeLog:

2014-09-11  Will Newton  <will.newton@linaro.org>

	* sysdeps/unix/sysv/linux/arm/bits/hwcap.h: Check for
	_LINUX_ARM_SYSDEP_H include guard too.
	* sysdeps/unix/sysv/linux/arm/sysdep.h (_SYS_AUXV_H): Remove
	define.
---
 sysdeps/unix/sysv/linux/arm/bits/hwcap.h | 2 +-
 sysdeps/unix/sysv/linux/arm/sysdep.h     | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

On Fri, 12 Sep 2014, Will Newton wrote:

> sysdep.h was defining _SYS_AUXV_H in order to avoid an include guard check
> in hwcap.h. Unfortunately it didn't undefine it so it could leak out into
> code and caused a build failure with -Wimplicit-function-declaration
> building tst-auxv on ARM.
> 
> ChangeLog:
> 
> 2014-09-11  Will Newton  <will.newton@linaro.org>
> 
> 	* sysdeps/unix/sysv/linux/arm/bits/hwcap.h: Check for
> 	_LINUX_ARM_SYSDEP_H include guard too.

Rather than referring to a guard for an internal header in an installed 
header, I think it would be better to test for _LIBC there.

On 12 September 2014 10:04, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Fri, 12 Sep 2014, Will Newton wrote:
>
>> sysdep.h was defining _SYS_AUXV_H in order to avoid an include guard check
>> in hwcap.h. Unfortunately it didn't undefine it so it could leak out into
>> code and caused a build failure with -Wimplicit-function-declaration
>> building tst-auxv on ARM.
>>
>> ChangeLog:
>>
>> 2014-09-11  Will Newton  <will.newton@linaro.org>
>>
>>       * sysdeps/unix/sysv/linux/arm/bits/hwcap.h: Check for
>>       _LINUX_ARM_SYSDEP_H include guard too.
>
> Rather than referring to a guard for an internal header in an installed
> header, I think it would be better to test for _LIBC there.

This approach (testing for the sysdep.h guard) is used in sparc and
powerpc at present. Testing _LIBC would work but
stdio-common/scanf15.c and stdio-common/scanf17.c both #undef _LIBC
and break things.

On Tue, 16 Sep 2014, Will Newton wrote:

> On 12 September 2014 10:04, Joseph S. Myers <joseph@codesourcery.com> wrote:
> > On Fri, 12 Sep 2014, Will Newton wrote:
> >
> >> sysdep.h was defining _SYS_AUXV_H in order to avoid an include guard check
> >> in hwcap.h. Unfortunately it didn't undefine it so it could leak out into
> >> code and caused a build failure with -Wimplicit-function-declaration
> >> building tst-auxv on ARM.
> >>
> >> ChangeLog:
> >>
> >> 2014-09-11  Will Newton  <will.newton@linaro.org>
> >>
> >>       * sysdeps/unix/sysv/linux/arm/bits/hwcap.h: Check for
> >>       _LINUX_ARM_SYSDEP_H include guard too.
> >
> > Rather than referring to a guard for an internal header in an installed
> > header, I think it would be better to test for _LIBC there.
> 
> This approach (testing for the sysdep.h guard) is used in sparc and
> powerpc at present. Testing _LIBC would work but

OK for the original patch then, but please file a bug in Bugzilla to 
eliminate such ad hoc conditionals from installed headers.  In general 
installed headers should have as little as possible that's not actually 
relevant for use of the installed library, and the particular set of 
conditionals used in installed headers in such cases should be minimal 
(_LIBC and _LIBC_TEST?).

> stdio-common/scanf15.c and stdio-common/scanf17.c both #undef _LIBC
> and break things.

Why does sysdep.h get included from these testcases anyway?  As well as 
moving away from defining _LIBC (and other internal defines) for tests 
unless needed for a particular test, it would also be good to move away 
from tests using the internal includes (if such an include is responsible 
here), instead making them use the versions of headers that actually get 
installed before looking for any other headers in the source tree or 
sysdeps directories.

On 16 September 2014 14:47, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Tue, 16 Sep 2014, Will Newton wrote:
>
>> On 12 September 2014 10:04, Joseph S. Myers <joseph@codesourcery.com> wrote:
>> > On Fri, 12 Sep 2014, Will Newton wrote:
>> >
>> >> sysdep.h was defining _SYS_AUXV_H in order to avoid an include guard check
>> >> in hwcap.h. Unfortunately it didn't undefine it so it could leak out into
>> >> code and caused a build failure with -Wimplicit-function-declaration
>> >> building tst-auxv on ARM.
>> >>
>> >> ChangeLog:
>> >>
>> >> 2014-09-11  Will Newton  <will.newton@linaro.org>
>> >>
>> >>       * sysdeps/unix/sysv/linux/arm/bits/hwcap.h: Check for
>> >>       _LINUX_ARM_SYSDEP_H include guard too.
>> >
>> > Rather than referring to a guard for an internal header in an installed
>> > header, I think it would be better to test for _LIBC there.
>>
>> This approach (testing for the sysdep.h guard) is used in sparc and
>> powerpc at present. Testing _LIBC would work but
>
> OK for the original patch then, but please file a bug in Bugzilla to
> eliminate such ad hoc conditionals from installed headers.  In general
> installed headers should have as little as possible that's not actually
> relevant for use of the installed library, and the particular set of
> conditionals used in installed headers in such cases should be minimal
> (_LIBC and _LIBC_TEST?).

I submitted a bugzilla entry here:

https://sourceware.org/bugzilla/show_bug.cgi?id=17399

I am not sure if I got the wording 100% correct. I also added a brief
documentation of _LIBC and _LIBC_TEST here:

https://sourceware.org/glibc/wiki/Conditional%20Code

>> stdio-common/scanf15.c and stdio-common/scanf17.c both #undef _LIBC
>> and break things.
>
> Why does sysdep.h get included from these testcases anyway?  As well as
> moving away from defining _LIBC (and other internal defines) for tests
> unless needed for a particular test, it would also be good to move away
> from tests using the internal includes (if such an include is responsible
> here), instead making them use the versions of headers that actually get
> installed before looking for any other headers in the source tree or
> sysdeps directories.

The full error is:

In file included from ../sysdeps/unix/sysv/linux/arm/sysdep.h:43:0,
                 from ../sysdeps/unix/sysv/linux/lowlevellock-futex.h:22,
                 from ../sysdeps/nptl/lowlevellock.h:23,
                 from ../sysdeps/nptl/bits/stdio-lock.h:23,
                 from ../libio/libio.h:149,
                 from ../libio/stdio.h:74,
                 from scanf15.c:10:
../sysdeps/unix/sysv/linux/arm/bits/hwcap.h:20:3: error: #error "Never
include <bits/hwcap.h> directly; use <sys/auxv.h> instead."

On Tue, Sep 16, 2014 at 09:47:49PM +0000, Joseph S. Myers wrote:
> Why does sysdep.h get included from these testcases anyway?  As well as 
> moving away from defining _LIBC (and other internal defines) for tests 
> unless needed for a particular test, it would also be good to move away 
> from tests using the internal includes (if such an include is responsible 
> here), instead making them use the versions of headers that actually get 
> installed before looking for any other headers in the source tree or 
> sysdeps directories.

I wonder if it would make sense to 'install' headers into a known
location in the build directory during build and use only that for the
tests.  That would make the tests completely isolated from the
internal headers.  There are some tests that explicitly do white box
testing; those can be treated as special cases.

Siddhesh

On Wed, 17 Sep 2014, Siddhesh Poyarekar wrote:

> On Tue, Sep 16, 2014 at 09:47:49PM +0000, Joseph S. Myers wrote:
> > Why does sysdep.h get included from these testcases anyway?  As well as 
> > moving away from defining _LIBC (and other internal defines) for tests 
> > unless needed for a particular test, it would also be good to move away 
> > from tests using the internal includes (if such an include is responsible 
> > here), instead making them use the versions of headers that actually get 
> > installed before looking for any other headers in the source tree or 
> > sysdeps directories.
> 
> I wonder if it would make sense to 'install' headers into a known
> location in the build directory during build and use only that for the
> tests.  That would make the tests completely isolated from the
> internal headers.  There are some tests that explicitly do white box
> testing; those can be treated as special cases.

Yes, headers should come from an installed staging directory for testing, 
but that's only part of the solution.

* Tests can quite reasonably have wrapper files for a given architecture 
in sysdeps, so you can't avoid the sysdeps search for everything.  (Most 
tests include at least one file that's not an installed header - 
test-skeleton.c.)  The installed headers should be searched before the 
sysdeps / other source tree headers, however.

* Tests should not include libc-symbols.h, or config.h, or get any of the 
defines from libc-symbols.h except _GNU_SOURCE (in particular, should not 
define _LIBC) unless there is a reason for them to do so.

* Such reasons may not be obvious simply from "test fails to build / work 
in a particular configuration".  E.g. tst-fanotify.c has a good reason to 
include config.h - it uses a configure test for whether linux/fanotify.h 
is available.  While it includes config.h explicitly, some tests might be 
relying on implicit includes.  So you need some way to search tests for 
references to config.h / libc-symbols.h symbols in #if conditions.

* Tests should also not get internal symbols defined in other ways.  The 
problem reported in the present case appears to be because of an 
_IO_MTSAFE_IO definition that resulted in various internal headers getting 
included.  That comes via CPPFLAGS += $(libio-mtsafe) in 
stdio-common/Makefile.