[PATCH 4/4] Add IS_IN (testsuite) and remaining fixes.

[Zack Weinberg]

On 02/20/2017 10:13 AM, Carlos O'Donell wrote:
> On 02/20/2017 08:03 AM, Zack Weinberg wrote:
>> This is the main change, adding a new build module called 'testsuite'.
>> IS_IN (testsuite) implies _ISOMAC, as do IS_IN_build and __cplusplus
>> (which means several ad-hoc tests for __cplusplus can go away).
>> libc-symbols.h now suppresses almost all of *itself* when _ISOMAC is
>> defined; in particular, _ISOMAC mode does not get config.h
>> automatically anymore.

I'm going through your low-level comments now; here are a few notes.
I'll post a revised patch later today, or tomorrow.

> - Fix tst-dladdr by removing DL_LOOKUP_ADDRESS usage and keeping it in
>   tests instead of tests-internal. I think this test should be as close
>   to a real application as possible.

Just to be sure that I understand the change you have in mind here: is
this right?

--- a/dlfcn/tst-dladdr.c
+++ b/dlfcn/tst-dladdr.c
@@ -24,8 +24,6 @@
 #include <stdlib.h>
 #include <string.h>

-#include <ldsodefs.h>
-

 #define TEST_FUNCTION do_test ()
 extern int do_test (void);
@@ -53,8 +51,6 @@ do_test (void)
   if (ret == 0)
     error (EXIT_FAILURE, 0, "dladdr failed");

-  printf ("address of ref1 = %lx\n",
-         (unsigned long int)  DL_LOOKUP_ADDRESS (sym));
   printf ("ret = %d\n", ret);
   printf ("info.dli_fname = %p (\"%s\")\n", info.dli_fname,
info.dli_fname);
   printf ("info.dli_fbase = %p\n", info.dli_fbase);


> - Please carry out a built artifact comparison to ensure the IS_IN
>   changes did not change any code generation in the library. Minimally
>   x86_64 and one other architecture of your choice.

Will do.

> - The include/stdio.h change needs a detailed comment about why we undef 
>   _IO_MTSAFE_IO.

Eegh, that's complicated.  Rather than add a comment, I am going to
simplify the situation.

_IO_MTSAFE_IO is only ever defined during the build of glibc itself, and
it does not change the public API or ABI.  There are two uses in
libio.h, which is a public header.  One changes the definitions of a
handful of internal-use-only _IO_* macros.  The other controls whether
libio.h defines _IO_lock_t itself (as an incomplete type) or leaves it
to stdio-lock.h (which is a non-installed header).  Unfortunately, some
versions of stdio-lock.h can only define _IO_lock_t as a typedef, so we
have to have the ability for libio.h not to do it at all.

What I'm going to do is remove the internal-use-only _IO_* macros to
include/libio.h, and invent a new macro _IO_lock_t_defined which all
versions of stdio-lock.h will define; libio.h will define the stub
_IO_lock_t if that macro is not defined, with a comment explaining that
this is only relevant when building glibc itself.  Then there will be no
uses of _IO_MTSAFE_IO in public headers, and it won't be necessary to
undefine it in include/stdio.h.

>> +    $(tests) $(tests-internal) $(xtests) $(test-srcs))): \
>> +	$(objpfx)libpthread.so \
>> +	$(objpfx)libpthread_nonshared.a
>
> Why doesn't this use $(shared-thread-library)?

It was that way when I got here, and I don't actually see any code that
*sets* $(shared-thread-library) anywhere in the Makefiles, so I can't
confirm that it'd be the same thing.

> - Why is tst-cancel-getpwuid_r in tests-internal? It was designed to be
>   a standalone cancellation test.

It calls __nss_configure_lookup.  I didn't look very hard when I saw
that - I see now that nss.h does count as a public header, so I'll
change it back.

> - New test stdlib/tst-strtod1i.c should use new support/ test infrastructure.
> 
> - New test stdlib/tst-strtod5i.c needs a copyright header and should use
>   new support/ test infrastructure.

Will do.  I am also going to make those changes to the tests they were
cloned from (tst-strtod.c and tst-strtod5.c).

zw