[PATCH] Add test for linking against most static libraries

[PATCH] Add test for linking against most static libraries

[Florian Weimer]

2016-10-28  Florian Weimer  <fweimer@redhat.com>

	* elf/tst-linkall-static.c: New file.
	* elf/Makefile (tests-static): Add tst-linkall-static.
	(tst-linkall-static): Link against static libraries.

diff --git a/elf/Makefile b/elf/Makefile
index caffd92..82c7e05 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -122,7 +122,8 @@ tests = tst-tls1 tst-tls2 tst-tls9 tst-leaks1 \
 tests-static = tst-tls1-static tst-tls2-static tst-stackguard1-static \
 	       tst-leaks1-static tst-array1-static tst-array5-static \
 	       tst-ptrguard1-static tst-dl-iter-static \
-	       tst-tlsalign-static tst-tlsalign-extern-static
+	       tst-tlsalign-static tst-tlsalign-extern-static \
+	       tst-linkall-static
 ifeq (yes,$(build-shared))
 tests-static += tst-tls9-static
 tst-tls9-static-ENV = \
@@ -1278,3 +1279,13 @@ $(objpfx)tst-ldconfig-X.out : tst-ldconfig-X.sh $(objpfx)ldconfig
 	$(evaluate-test)
 
 $(objpfx)tst-dlsym-error: $(libdl)
+
+$(objpfx)tst-linkall-static: \
+  $(common-objpfx)math/libm.a \
+  $(common-objpfx)crypt/libcrypt.a \
+  $(common-objpfx)resolv/libresolv.a \
+  $(common-objpfx)dlfcn/libdl.a \
+  $(common-objpfx)login/libutil.a \
+  $(common-objpfx)rt/librt.a \
+  $(common-objpfx)resolv/libanl.a \
+  $(static-thread-library) \
diff --git a/elf/tst-linkall-static.c b/elf/tst-linkall-static.c
new file mode 100644
index 0000000..7a4aacc
--- /dev/null
+++ b/elf/tst-linkall-static.c
@@ -0,0 +1,45 @@
+/* Test static linking against multiple libraries, to find symbol conflicts.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public License as
+   published by the Free Software Foundation; either version 2.1 of the
+   License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; see the file COPYING.LIB.  If
+   not, see <http://www.gnu.org/licenses/>.  */
+
+#include <math.h>
+#include <pthread.h>
+#include <crypt.h>
+#include <resolv.h>
+#include <dlfcn.h>
+#include <utmp.h>
+#include <aio.h>
+#include <netdb.h>
+
+/* These references force linking the executable against central
+   functions in the static libraries, pulling significant parts of
+   each library into the link.  */
+void *references[] =
+  {
+    &pow,                       /* libm */
+    &pthread_create,            /* libpthread */
+    &crypt,                     /* libcrypt */
+    &res_send,                  /* libresolv */
+    &dlopen,                    /* libdl */
+    &login,                     /* libutil */
+    &aio_init,                  /* librt */
+    &getaddrinfo_a,             /* libanl */
+  };
+
+/* This is a link-time test.  There is nothing to run here.  */
+#define TEST_FUNCTION 0
+#include "../test-skeleton.c"

Re: [PATCH] Add test for linking against most static libraries

[Carlos O'Donell]

On 10/28/2016 08:57 AM, Florian Weimer wrote:
> 2016-10-28  Florian Weimer  <fweimer@redhat.com>
> 
> 	* elf/tst-linkall-static.c: New file.
> 	* elf/Makefile (tests-static): Add tst-linkall-static.
> 	(tst-linkall-static): Link against static libraries.

LGTM. Thanks for increasing test coverage.

-- 
Cheers,
Carlos.

Re: [PATCH] Add test for linking against most static libraries

[Carlos O'Donell]

On 10/28/2016 10:51 AM, Carlos O'Donell wrote:
> On 10/28/2016 08:57 AM, Florian Weimer wrote:
>> 2016-10-28  Florian Weimer  <fweimer@redhat.com>
>>
>> 	* elf/tst-linkall-static.c: New file.
>> 	* elf/Makefile (tests-static): Add tst-linkall-static.
>> 	(tst-linkall-static): Link against static libraries.
> 
> LGTM. Thanks for increasing test coverage.
> 

This breaks my i486 -Os build with a test build failures because
it can't find crypt.h.

I don't see that any of the sysdep selection mechanisms add
crypt to the list, therefore you can't access crypt headers.

However, the crypt/ tests themselves seem to get an absolute
path added explicitly, so they work.

The only other reference is from locale/Makefile which also
adds -I../crypt via vpath and CFLAGS-*.

I'm kinda baffled by this. We don't set it in config-sysdirs.

I expect you have a local patch that hid this.

I'm going to commit the following immediate fix:

2016-10-28  Carlos O'Donell  <carlos@redhat.com>

	* elf/Makefile (CFALGS-tst-linkall-static): Define.

diff --git a/elf/Makefile b/elf/Makefile
index 82c7e05..a8d2bfe 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -1280,6 +1280,7 @@ $(objpfx)tst-ldconfig-X.out : tst-ldconfig-X.sh $(objpfx)ldconfig
 
 $(objpfx)tst-dlsym-error: $(libdl)
 
+CFLAGS-tst-linkall-static = -I../crypt
 $(objpfx)tst-linkall-static: \
   $(common-objpfx)math/libm.a \
   $(common-objpfx)crypt/libcrypt.a \
---

-- 
Cheers,
Carlos.

Re: [PATCH] Add test for linking against most static libraries

[Joseph Myers]

On Fri, 28 Oct 2016, Carlos O'Donell wrote:

> I'm kinda baffled by this. We don't set it in config-sysdirs.
> 
> I expect you have a local patch that hid this.

I expect he happened to have a compiler that searched a system include 
directory with crypt.h.

> I'm going to commit the following immediate fix:
> 
> 2016-10-28  Carlos O'Donell  <carlos@redhat.com>
> 
> 	* elf/Makefile (CFALGS-tst-linkall-static): Define.

That's not the right fix.  The right way to address such issues is a 
wrapper header in include/.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Add test for linking against most static libraries

[Florian Weimer]

On 10/28/2016 07:33 PM, Joseph Myers wrote:

>> I'm going to commit the following immediate fix:
>>
>> 2016-10-28  Carlos O'Donell  <carlos@redhat.com>
>>
>> 	* elf/Makefile (CFALGS-tst-linkall-static): Define.
>
> That's not the right fix.  The right way to address such issues is a
> wrapper header in include/.

Is this for installed headers only?

I've been using the <crypt/crypt.h> style for internal headers.

Florian

Re: [PATCH] Add test for linking against most static libraries

[Joseph Myers]

On Fri, 28 Oct 2016, Florian Weimer wrote:

> On 10/28/2016 07:33 PM, Joseph Myers wrote:
> 
> > > I'm going to commit the following immediate fix:
> > > 
> > > 2016-10-28  Carlos O'Donell  <carlos@redhat.com>
> > > 
> > > 	* elf/Makefile (CFALGS-tst-linkall-static): Define.
> > 
> > That's not the right fix.  The right way to address such issues is a
> > wrapper header in include/.
> 
> Is this for installed headers only?

<crypt.h> is an installed header.  We want glibc source files and tests to 
include installed headers via the same paths a normal program built with 
installed headers would, so #include <crypt.h> not #include 
<crypt/crypt.h>.  The way we achieve this is through the wrappers in 
include/, some of them trivial (e.g. cpio.h, an example of a wrapper added 
for testcase use for a similar issue), some less trivial.

> I've been using the <crypt/crypt.h> style for internal headers.

That may be reasonable if a file in one directory wants to include an 
internal header from another (though I'd think an internal header used in 
more than one directory should move to include/), but this isn't a case of 
an internal header.  It should have a wrapper that does #include 
<crypt/crypt.h>, so nothing other than the wrapper needs to use that path.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Add test for linking against most static libraries

[Carlos O'Donell]

On 10/28/2016 01:29 PM, Carlos O'Donell wrote:
> On 10/28/2016 10:51 AM, Carlos O'Donell wrote:
>> On 10/28/2016 08:57 AM, Florian Weimer wrote:
>>> 2016-10-28  Florian Weimer  <fweimer@redhat.com>
>>>
>>> 	* elf/tst-linkall-static.c: New file.
>>> 	* elf/Makefile (tests-static): Add tst-linkall-static.
>>> 	(tst-linkall-static): Link against static libraries.
>>
>> LGTM. Thanks for increasing test coverage.
>>
> 
> This breaks my i486 -Os build with a test build failures because
> it can't find crypt.h.
> 
> I don't see that any of the sysdep selection mechanisms add
> crypt to the list, therefore you can't access crypt headers.
> 
> However, the crypt/ tests themselves seem to get an absolute
> path added explicitly, so they work.
> 
> The only other reference is from locale/Makefile which also
> adds -I../crypt via vpath and CFLAGS-*.
> 
> I'm kinda baffled by this. We don't set it in config-sysdirs.
> 
> I expect you have a local patch that hid this.
> 
> I'm going to commit the following immediate fix:
> 
> 2016-10-28  Carlos O'Donell  <carlos@redhat.com>
> 
> 	* elf/Makefile (CFALGS-tst-linkall-static): Define.
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index 82c7e05..a8d2bfe 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -1280,6 +1280,7 @@ $(objpfx)tst-ldconfig-X.out : tst-ldconfig-X.sh $(objpfx)ldconfig
>  
>  $(objpfx)tst-dlsym-error: $(libdl)
>  
> +CFLAGS-tst-linkall-static = -I../crypt
>  $(objpfx)tst-linkall-static: \
>    $(common-objpfx)math/libm.a \
>    $(common-objpfx)crypt/libcrypt.a \
> ---

Without the typo (missing '.c' in CFLAGS rule).

-- 
Cheers,
Carlos.

Re: [PATCH] Add test for linking against most static libraries

[Carlos O'Donell]

On 10/28/2016 01:33 PM, Joseph Myers wrote:
> On Fri, 28 Oct 2016, Carlos O'Donell wrote:
> 
>> I'm kinda baffled by this. We don't set it in config-sysdirs.
>>
>> I expect you have a local patch that hid this.
> 
> I expect he happened to have a compiler that searched a system include 
> directory with crypt.h.
> 
>> I'm going to commit the following immediate fix:
>>
>> 2016-10-28  Carlos O'Donell  <carlos@redhat.com>
>>
>> 	* elf/Makefile (CFALGS-tst-linkall-static): Define.
> 
> That's not the right fix.  The right way to address such issues is a 
> wrapper header in include/.
 
OK, I'll fix it that way right now.

-- 
Cheers,
Carlos.

Re: [PATCH] Add test for linking against most static libraries

[Florian Weimer]

On 10/28/2016 07:33 PM, Joseph Myers wrote:
> On Fri, 28 Oct 2016, Carlos O'Donell wrote:
>
>> I'm kinda baffled by this. We don't set it in config-sysdirs.
>>
>> I expect you have a local patch that hid this.
>
> I expect he happened to have a compiler that searched a system include
> directory with crypt.h.

This is what seems to have happened, at least according to 
elf/tst-linkall-static.o.d.  It happened on all systems I tested this.

I suspect the only way to guard against this is to parse the default 
compiler search paths from gcc -v and replace the variants of 
/usr/include found therein with a carefully prepared directory tree of 
installed headers and some linked-in kernel headers.

Thanks,
Florian

Re: [PATCH] Add test for linking against most static libraries

[Joseph Myers]

On Fri, 28 Oct 2016, Florian Weimer wrote:

> I suspect the only way to guard against this is to parse the default compiler
> search paths from gcc -v and replace the variants of /usr/include found
> therein with a carefully prepared directory tree of installed headers and some
> linked-in kernel headers.

Building glibc needs compiler headers (include and include-fixed 
directories - but while we need limits.h from include-fixed, there may be 
other headers in include-fixed that we don't want) and kernel headers.  
But in some cases it may require other installed system headers as well.  
E.g. memusagestat requires installed GD headers.  Systemtap support 
requires installed sys/sdt.h.  I don't know what might require other 
installed headers as well, especially for the non-Linux ports.

So while avoiding /usr/include is desirable, doing so while keeping all 
the cases that properly use installed headers working may be tricky.  
(I'd argue that the memusagestat case should be solved by moving it to a 
separate package - maybe released along with glibc, but built with 
installed libraries only rather than as part of a glibc build.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Add test for linking against most static libraries

[Carlos O'Donell]

On 10/28/2016 02:30 PM, Carlos O'Donell wrote:
> On 10/28/2016 01:33 PM, Joseph Myers wrote:
>> On Fri, 28 Oct 2016, Carlos O'Donell wrote:
>>
>>> I'm kinda baffled by this. We don't set it in config-sysdirs.
>>>
>>> I expect you have a local patch that hid this.
>>
>> I expect he happened to have a compiler that searched a system include 
>> directory with crypt.h.
>>
>>> I'm going to commit the following immediate fix:
>>>
>>> 2016-10-28  Carlos O'Donell  <carlos@redhat.com>
>>>
>>> 	* elf/Makefile (CFALGS-tst-linkall-static): Define.
>>
>> That's not the right fix.  The right way to address such issues is a 
>> wrapper header in include/.
>  
> OK, I'll fix it that way right now.

Given that crypt.h is the only installed header in crypt/ that should
fix all such issues.

No regressions on x86.

OK?

2016-10-28  Carlos O'Donell  <carlos@redhat.com>

        * elf/Makefile (CFALGS-tst-linkall-static): Remove.
        * include/crypt.h: New file.

diff --git a/elf/Makefile b/elf/Makefile
index 057a19f..82c7e05 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -1280,7 +1280,6 @@ $(objpfx)tst-ldconfig-X.out : tst-ldconfig-X.sh $(objpfx)ldconfig
 
 $(objpfx)tst-dlsym-error: $(libdl)
 
-CFLAGS-tst-linkall-static.c = -I../crypt
 $(objpfx)tst-linkall-static: \
   $(common-objpfx)math/libm.a \
   $(common-objpfx)crypt/libcrypt.a \
diff --git a/include/crypt.h b/include/crypt.h
new file mode 100644
index 0000000..544551d
--- /dev/null
+++ b/include/crypt.h
@@ -0,0 +1,3 @@
+#ifndef _CRYPT_H
+# include <crypt/crypt.h>
+#endif /* _CRYPT_H */
---

-- 
Cheers,
Carlos.

Re: [PATCH] Add test for linking against most static libraries

[Carlos O'Donell]

On 10/28/2016 02:55 PM, Florian Weimer wrote:
> On 10/28/2016 07:33 PM, Joseph Myers wrote:
>> On Fri, 28 Oct 2016, Carlos O'Donell wrote:
>> 
>>> I'm kinda baffled by this. We don't set it in config-sysdirs.
>>> 
>>> I expect you have a local patch that hid this.
>> 
>> I expect he happened to have a compiler that searched a system
>> include directory with crypt.h.
> 
> This is what seems to have happened, at least according to
> elf/tst-linkall-static.o.d.  It happened on all systems I tested
> this.
> 
> I suspect the only way to guard against this is to parse the default
> compiler search paths from gcc -v and replace the variants of
> /usr/include found therein with a carefully prepared directory tree
> of installed headers and some linked-in kernel headers.

That's what my build private scripts do :-)

I rebuild a sysroot out of a few headers and an install of master
kernel headers to detect immediate UAPI breakage as it goes in upstream.

-- 
Cheers,
Carlos.

Re: [PATCH] Add test for linking against most static libraries

[Joseph Myers]

On Fri, 28 Oct 2016, Carlos O'Donell wrote:

> Given that crypt.h is the only installed header in crypt/ that should
> fix all such issues.
> 
> No regressions on x86.
> 
> OK?

OK.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Add test for linking against most static libraries

[Florian Weimer]

On 10/28/2016 10:07 PM, Joseph Myers wrote:
> On Fri, 28 Oct 2016, Florian Weimer wrote:
>
>> I suspect the only way to guard against this is to parse the default compiler
>> search paths from gcc -v and replace the variants of /usr/include found
>> therein with a carefully prepared directory tree of installed headers and some
>> linked-in kernel headers.
>
> Building glibc needs compiler headers (include and include-fixed
> directories - but while we need limits.h from include-fixed, there may be
> other headers in include-fixed that we don't want) and kernel headers.

Are you certain about include-fixed part?

Are there any distributions which actually use fixincludes?  What would 
be found there?

We need quite a few headers from GCC (even more with C++), but I'd 
suggest to rewrite the default include path to strip /usr/include, 
/usr/local/include and the multi-arch header directories if applicable, 
and leave the GCC include directories in place.  The glibc installed 
header directory would take the place where /usr/include originally was.

> But in some cases it may require other installed system headers as well.
> E.g. memusagestat requires installed GD headers.  Systemtap support
> requires installed sys/sdt.h.  I don't know what might require other
> installed headers as well, especially for the non-Linux ports.

True.  NSS is another problematic case.

> So while avoiding /usr/include is desirable, doing so while keeping all
> the cases that properly use installed headers working may be tricky.

Yes, but a cleaner separation seems desirable to me.

> (I'd argue that the memusagestat case should be solved by moving it to a
> separate package - maybe released along with glibc, but built with
> installed libraries only rather than as part of a glibc build.)

Seems reasonable.

I'm also trying to phase out NSS-based libcrypt at Red Hat, or 
alternatively, convince a real cryptographic library to build the 
library for us.

Thanks,
Florian

Re: [PATCH] Add test for linking against most static libraries

[Joseph Myers]

On Fri, 28 Oct 2016, Florian Weimer wrote:

> > Building glibc needs compiler headers (include and include-fixed
> > directories - but while we need limits.h from include-fixed, there may be
> > other headers in include-fixed that we don't want) and kernel headers.
> 
> Are you certain about include-fixed part?

It has limits.h as a wrapper round the libc limits.h.  Other headers, 
fixed from libc or kernel or other system headers, wouldn't be wanted for 
building glibc - but might well be present (and must not be searched until 
after glibc's own headers - whereas for limits.h either order is OK).

> Are there any distributions which actually use fixincludes?  What would be

If someone has built their own compiler for building glibc I'd expect any 
fixed versions of glibc or kernel headers to be there as it would take 
special measures to remove them after building GCC.

(I tried before unsuccessfully to get the "#ifdef linux" in linux/a.out.h 
which results in it being fixincluded removed.  
<https://lkml.org/lkml/2006/10/6/201> 
<https://lkml.org/lkml/2011/10/11/210>.  Other fixed headers may vary 
depending on GCC / glibc / Linux versions.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Add test for linking against most static libraries

[Carlos O'Donell]

On 10/28/2016 04:21 PM, Joseph Myers wrote:
> On Fri, 28 Oct 2016, Carlos O'Donell wrote:
> 
>> Given that crypt.h is the only installed header in crypt/ that should
>> fix all such issues.
>>
>> No regressions on x86.
>>
>> OK?
> 
> OK.
> 

Fixed.

-- 
Cheers,
Carlos.