Enable linknamespace testing for libdl and libcrypt

Message ID alpine.DEB.2.20.1611161905330.26593@digraph.polyomino.org.uk
State Committed
Headers

Commit Message

Joseph Myers Nov. 16, 2016, 7:06 p.m. UTC
  When I set up linknamespace testing, the lists of libraries that might
contain functions from various standards were based on the -l options
POSIX says may be required to find certain functions with the c99
utility.

glibc has some POSIX functions in the libdl and libcrypt libraries,
not mentioned in the definition of the c99 utility (so an
implementation of that utility using glibc would need to use -ldl
-lcrypt automatically).  This patch adds those libraries to the ones
considered in linknamespace testing for relevant standards.  (The
crypt functions are XSI only, present in XPG3 and above; the libdl
ones were added in UNIX98, then moved from XSI to BASE in the 2008
edition of POSIX.)

I intend to commit this once the fix for bug 20829 (which I tested in 
conjunction with this patch), crypt namespace issues, is in.

2016-11-16  Joseph Myers  <joseph@codesourcery.com>

	* conform/Makefile (linknamespace-libs): Rename to
	linknamespace-libs-thr.
	(linknamespace-libs-posix): New variable.
	(linknamespace-libs-xsi): Likewise.
	(linknamespace-libs-XPG3): Include libcrypt.a.
	(linknamespace-libs-XPG4): Use $(linknamespace-libs-XPG3).
	(linknamespace-libs-POSIX): Use $(linknamespace-libs-thr).
	(linknamespace-libs-UNIX98): Use $(linknamespace-libs-xsi).
	(linknamespace-libs-XOPEN2K): Likewise.
	(linknamespace-libs-XOPEN2K8): Likewise.
	(linknamespace-libs-POSIX2008): Use $(linknamespace-libs-posix).
  

Comments

Florian Weimer Nov. 16, 2016, 7:47 p.m. UTC | #1
On 11/16/2016 08:06 PM, Joseph Myers wrote:
> +linknamespace-libs-XPG3 = $(linknamespace-libs-isoc) \
> +			  $(common-objpfx)crypt/libcrypt.a

Does the linknamespace test perform a full link, and fails if it 
encounters undefined symbols?

I don't think anyone has the requisite static NSS libraries for linking 
libcrypt statically with --enable-nss-crypt.  So you may have to disable 
this for $(nss-crypt) configurations.

Carlos will submit a patch to do this for elf/tst-linkall-static.  We 
weren't aware of this issue until this new test went in.

Thanks,
Florian
  
Joseph Myers Nov. 16, 2016, 10:45 p.m. UTC | #2
On Wed, 16 Nov 2016, Florian Weimer wrote:

> On 11/16/2016 08:06 PM, Joseph Myers wrote:
> > +linknamespace-libs-XPG3 = $(linknamespace-libs-isoc) \
> > +			  $(common-objpfx)crypt/libcrypt.a
> 
> Does the linknamespace test perform a full link, and fails if it encounters
> undefined symbols?

No, it has its own emulation of linker logic, and ignores failure to find 
a definition of a symbol (for example, libgcc symbols - it doesn't look 
for libgcc.a / libgcc_eh.a, so it won't detect namespace issues there).  
(If an undefined symbol breaks namespace rules, that will still result in 
a test failure unless whitelisted in linknamespace.pl, whether or not a 
definition of that symbol was found.)
  
Florian Weimer Nov. 17, 2016, 9:08 a.m. UTC | #3
On 11/16/2016 11:45 PM, Joseph Myers wrote:
> On Wed, 16 Nov 2016, Florian Weimer wrote:
>
>> On 11/16/2016 08:06 PM, Joseph Myers wrote:
>>> +linknamespace-libs-XPG3 = $(linknamespace-libs-isoc) \
>>> +			  $(common-objpfx)crypt/libcrypt.a
>>
>> Does the linknamespace test perform a full link, and fails if it encounters
>> undefined symbols?
>
> No, it has its own emulation of linker logic, and ignores failure to find
> a definition of a symbol (for example, libgcc symbols - it doesn't look
> for libgcc.a / libgcc_eh.a, so it won't detect namespace issues there).
> (If an undefined symbol breaks namespace rules, that will still result in
> a test failure unless whitelisted in linknamespace.pl, whether or not a
> definition of that symbol was found.)

Understood.  Then the linknamespace test fails thusly:

[initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt -> 
[libcrypt.a(md5-crypt.o)] NSSLOWHASH_Begin
[initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt -> 
[libcrypt.a(md5-crypt.o)] NSSLOWHASH_Destroy
[initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt -> 
[libcrypt.a(md5-crypt.o)] NSSLOWHASH_End
[initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt -> 
[libcrypt.a(md5-crypt.o)] NSSLOWHASH_NewContext
[initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt -> 
[libcrypt.a(md5-crypt.o)] NSSLOWHASH_Update
[initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt -> 
[libcrypt.a(md5-crypt.o)] NSSLOW_Init
[initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt -> 
[libcrypt.a(md5-crypt.o)] NSSLOW_Shutdown

This is, of course, completely correct and expected.  I don't know what 
to do about it.  XFAIL it?

I'm trying to get rid of NSS-based crypt downstream, but it's going to 
take a while (it's already optional in Fedora).

Florian
  
Joseph Myers Nov. 17, 2016, 2:17 p.m. UTC | #4
On Thu, 17 Nov 2016, Florian Weimer wrote:

> Understood.  Then the linknamespace test fails thusly:
> 
> [initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt ->
> [libcrypt.a(md5-crypt.o)] NSSLOWHASH_Begin
> [initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt ->
> [libcrypt.a(md5-crypt.o)] NSSLOWHASH_Destroy
> [initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt ->
> [libcrypt.a(md5-crypt.o)] NSSLOWHASH_End
> [initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt ->
> [libcrypt.a(md5-crypt.o)] NSSLOWHASH_NewContext
> [initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt ->
> [libcrypt.a(md5-crypt.o)] NSSLOWHASH_Update
> [initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt ->
> [libcrypt.a(md5-crypt.o)] NSSLOW_Init
> [initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt ->
> [libcrypt.a(md5-crypt.o)] NSSLOW_Shutdown
> 
> This is, of course, completely correct and expected.  I don't know what to do
> about it.  XFAIL it?

Assuming there are no implementation-namespace exports of those functions 
that libcrypt could use instead, that indicates whitelisting in 
linknamespace.pl with a comment referencing a bug filed for nss-crypt 
namespace issues.
  
Florian Weimer Nov. 17, 2016, 2:22 p.m. UTC | #5
On 11/17/2016 03:17 PM, Joseph Myers wrote:
> On Thu, 17 Nov 2016, Florian Weimer wrote:
>
>> Understood.  Then the linknamespace test fails thusly:
>>
>> [initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt ->
>> [libcrypt.a(md5-crypt.o)] NSSLOWHASH_Begin
>> [initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt ->
>> [libcrypt.a(md5-crypt.o)] NSSLOWHASH_Destroy
>> [initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt ->
>> [libcrypt.a(md5-crypt.o)] NSSLOWHASH_End
>> [initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt ->
>> [libcrypt.a(md5-crypt.o)] NSSLOWHASH_NewContext
>> [initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt ->
>> [libcrypt.a(md5-crypt.o)] NSSLOWHASH_Update
>> [initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt ->
>> [libcrypt.a(md5-crypt.o)] NSSLOW_Init
>> [initial] crypt -> [libcrypt.a(crypt-entry.o)] __md5_crypt ->
>> [libcrypt.a(md5-crypt.o)] NSSLOW_Shutdown
>>
>> This is, of course, completely correct and expected.  I don't know what to do
>> about it.  XFAIL it?
>
> Assuming there are no implementation-namespace exports of those functions
> that libcrypt could use instead, that indicates whitelisting in
> linknamespace.pl with a comment referencing a bug filed for nss-crypt
> namespace issues.

Well, this is only relevant if we actually had a libfreebl.a which 
defines the functions above.  I don't think such a thing exists.  Fedora 
and downstreams don't have it, and neither does Debian jessie.

So it's really a “does it make a sound?”-type question.

Florian
  
Joseph Myers Nov. 17, 2016, 2:31 p.m. UTC | #6
On Thu, 17 Nov 2016, Florian Weimer wrote:

> > Assuming there are no implementation-namespace exports of those functions
> > that libcrypt could use instead, that indicates whitelisting in
> > linknamespace.pl with a comment referencing a bug filed for nss-crypt
> > namespace issues.
> 
> Well, this is only relevant if we actually had a libfreebl.a which defines the
> functions above.  I don't think such a thing exists.  Fedora and downstreams
> don't have it, and neither does Debian jessie.

As a cross-library namespace issue I'd expect it's just as relevant when 
shared libcrypt is referencing these functions from another shared 
library.  That is, this configuration involves a namespace bug, which is 
hard to fix so the names in question are whitelisted.
  
Florian Weimer Nov. 18, 2016, 2:36 p.m. UTC | #7
On 11/17/2016 03:31 PM, Joseph Myers wrote:
> On Thu, 17 Nov 2016, Florian Weimer wrote:
>
>>> Assuming there are no implementation-namespace exports of those functions
>>> that libcrypt could use instead, that indicates whitelisting in
>>> linknamespace.pl with a comment referencing a bug filed for nss-crypt
>>> namespace issues.
>>
>> Well, this is only relevant if we actually had a libfreebl.a which defines the
>> functions above.  I don't think such a thing exists.  Fedora and downstreams
>> don't have it, and neither does Debian jessie.
>
> As a cross-library namespace issue I'd expect it's just as relevant when
> shared libcrypt is referencing these functions from another shared
> library.

This is correct.  The library with the NSSLOW_* names is just a stub, 
and it attempts to dlopen the real thing (without RTLD_GLOBAL), to 
conserve the symbol footprint.  But that library uses quite a few 
functions which may have been interposed by the application, and 
RTLD_LOCAL does not guard against that.

Note that moving these libraries into the implementation namespace is 
counterproductive because we don't want non-toolchain libraries to be 
located there because then we don't own that space anymore and cannot 
argue that names in it are safe to use for us.  But such low-level 
libraries (libidn will be in the same boat soon) could certainly use 
glibc functionality under names in the implementation namespace without 
ill effect.

For the symbols they define, we need to make sure (at the distribution 
level) that these symbols are reasonably prefixed in some way.  Names 
like “mutex“, “buffer”, ”yyin” are really not a good idea.

> That is, this configuration involves a namespace bug,

Agreed.

> which is  hard to fix so the names in question are whitelisted.

Sorry, I don't understand this part.

Florian
  
Joseph Myers Nov. 18, 2016, 6:07 p.m. UTC | #8
On Fri, 18 Nov 2016, Florian Weimer wrote:

> > That is, this configuration involves a namespace bug,
> 
> Agreed.
> 
> > which is  hard to fix so the names in question are whitelisted.
> 
> Sorry, I don't understand this part.

The whitelist in conform/linknamespace.pl covers both false positives and 
genuine but hard-to-fix bugs.  So these names should be added there with 
an appropriate comment.

# The following whitelisted symbols are also allowed for now.
#
# * Bug 17576: stdin, stdout, stderr only reserved with external
# linkage when stdio.h included (and possibly not then), not
# generally.
#
# * Bug 18442: re_syntax_options wrongly brought in by regcomp and
# used by re_comp.
#
# * False positive: matherr only used conditionally.  matherrf/matherrl are used
# by IA64 too for the same reason.
#
@whitelist = qw(stdin stdout stderr re_syntax_options matherr matherrf
                matherrl);
  
Florian Weimer Nov. 21, 2016, 10:42 a.m. UTC | #9
On 11/18/2016 07:07 PM, Joseph Myers wrote:
> On Fri, 18 Nov 2016, Florian Weimer wrote:
>
>>> That is, this configuration involves a namespace bug,
>>
>> Agreed.
>>
>>> which is  hard to fix so the names in question are whitelisted.
>>
>> Sorry, I don't understand this part.
>
> The whitelist in conform/linknamespace.pl covers both false positives and
> genuine but hard-to-fix bugs.  So these names should be added there with
> an appropriate comment.
>
> # The following whitelisted symbols are also allowed for now.
> #
> # * Bug 17576: stdin, stdout, stderr only reserved with external
> # linkage when stdio.h included (and possibly not then), not
> # generally.
> #
> # * Bug 18442: re_syntax_options wrongly brought in by regcomp and
> # used by re_comp.
> #
> # * False positive: matherr only used conditionally.  matherrf/matherrl are used
> # by IA64 too for the same reason.
> #
> @whitelist = qw(stdin stdout stderr re_syntax_options matherr matherrf
>                 matherrl);

Okay, this approach looks fine to me.

Thanks,
Florian
  

Patch

diff --git a/conform/Makefile b/conform/Makefile
index 7883624..5862f70 100644
--- a/conform/Makefile
+++ b/conform/Makefile
@@ -209,18 +209,23 @@  $(linknamespace-symlists-tests): $(objpfx)symlist-%: list-header-symbols.pl
 	$(evaluate-test)
 
 linknamespace-libs-isoc = $(common-objpfx)libc.a $(common-objpfx)math/libm.a
-linknamespace-libs = $(linknamespace-libs-isoc) \
-		     $(common-objpfx)rt/librt.a $(static-thread-library)
+linknamespace-libs-thr = $(linknamespace-libs-isoc) \
+			 $(common-objpfx)rt/librt.a $(static-thread-library)
+linknamespace-libs-posix = $(linknamespace-libs-thr) \
+			   $(common-objpfx)dlfcn/libdl.a
+linknamespace-libs-xsi = $(linknamespace-libs-posix) \
+			 $(common-objpfx)crypt/libcrypt.a
 linknamespace-libs-ISO = $(linknamespace-libs-isoc)
 linknamespace-libs-ISO99 = $(linknamespace-libs-isoc)
 linknamespace-libs-ISO11 = $(linknamespace-libs-isoc)
-linknamespace-libs-XPG3 = $(linknamespace-libs-isoc)
-linknamespace-libs-XPG4 = $(linknamespace-libs-isoc)
-linknamespace-libs-POSIX = $(linknamespace-libs)
-linknamespace-libs-UNIX98 = $(linknamespace-libs)
-linknamespace-libs-XOPEN2K = $(linknamespace-libs)
-linknamespace-libs-POSIX2008 = $(linknamespace-libs)
-linknamespace-libs-XOPEN2K8 = $(linknamespace-libs)
+linknamespace-libs-XPG3 = $(linknamespace-libs-isoc) \
+			  $(common-objpfx)crypt/libcrypt.a
+linknamespace-libs-XPG4 = $(linknamespace-libs-XPG3)
+linknamespace-libs-POSIX = $(linknamespace-libs-thr)
+linknamespace-libs-UNIX98 = $(linknamespace-libs-xsi)
+linknamespace-libs-XOPEN2K = $(linknamespace-libs-xsi)
+linknamespace-libs-POSIX2008 = $(linknamespace-libs-posix)
+linknamespace-libs-XOPEN2K8 = $(linknamespace-libs-xsi)
 
 $(linknamespace-symlist-stdlibs-tests): $(objpfx)symlist-stdlibs-%: \
 					$(linknamespace-libs)