This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH roland/Wundef] Compile with -Wundef.


This adds -Wundef and fixes the most egregious cases that were warning on
just about every file in my x86_64-linux-gnu build.  There are many
warnings remaining, but I'll let others pursue all those.  Some of them
seem like they were masking real bugs.

In general, I think we want to get to a place where we have no #ifdef (or
"defined ...") tests at all, outside perhaps a few infrastructure headers
and the occasional special case (and installed headers, of course).  They
are inherently error-prone: you won't notice if you made a typo or failed
to #include the header that should define the macro, etc.  Eventually with
-Werror=undef and the style of always using "#if FOO", we'll get reliable
build errors for that sort of bug.

For similar cases, I think the include/stackinfo.h wrapper header is
probably a good model.  That is, something that makes the requirements on
the sysdeps header fairly simple, but enforces them strictly to catch bugs
quickly.  I first tried to do the same with tls.h, but ran into recursive
inclusion order hell and decided it was simpler for now to just change the
contract to require each sysdeps/.../tls.h file to define both macros
(there is nothing that enforces that exactly one of the two evaluates to
true, which would be ideal).  The mess of early include order that results
from tls.h has caused plenty of other headaches too.  We really should
figure out how to clean that up more sensibly at some point.

I'd be happy for this to go in now, but only if there are people who want
to commit to tracking down all the warnings and doing the cleanup in the
next couple of weeks.  As I mentioned before, I think the right way to go
about this is to choose one macro (or small set of closely related macros)
and fix up all warnings related to that one macro in one patch, not
including any other macros in the same patch.

It's a longer-term project to clean up all our #if uses to eliminate
#ifdef and its perils.


Thanks,
Roland


2014-02-28  Roland McGrath  <roland@hack.frob.com>

	* Makeconfig (+gccwarn): Add -Wundef.
	* include/errno.h [IS_IN_rtld] [!RTLD_PRIVATE_ERRNO]: #error to catch
	a dl-sysdep.h breaking its contract.
	[!IS_IN_rtld] (RTLD_PRIVATE_ERRNO): Define it to 0.
	* include/stackinfo.h: New file.
	* sysdeps/aarch64/nptl/tls.h (TLS_TCB_AT_TP): New macro.
	* sysdeps/alpha/nptl/tls.h (TLS_TCB_AT_TP): New macro.
	* sysdeps/arm/nptl/tls.h (TLS_TCB_AT_TP): New macro.
	* sysdeps/ia64/nptl/tls.h (TLS_TCB_AT_TP): New macro.
	* sysdeps/m68k/nptl/tls.h (TLS_TCB_AT_TP): New macro.
	* sysdeps/mach/hurd/i386/tls.h (TLS_TCB_AT_TP): New macro.
	* sysdeps/microblaze/nptl/tls.h (TLS_TCB_AT_TP): New macro.
	* sysdeps/mips/nptl/tls.h (TLS_TCB_AT_TP): New macro.
	* sysdeps/tile/nptl/tls.h (TLS_TCB_AT_TP): New macro.

nptl/
	* sysdeps/i386/tls.h (TLS_DTV_AT_TP): New macro.
	* sysdeps/powerpc/tls.h (TLS_TCB_AT_TP): New macro.
	* sysdeps/s390/tls.h (TLS_DTV_AT_TP): New macro.
	* sysdeps/sh/tls.h (TLS_TCB_AT_TP): New macro.
	* sysdeps/sparc/tls.h (TLS_DTV_AT_TP): New macro.
	* sysdeps/x86_64/tls.h (TLS_DTV_AT_TP): New macro.

ports/ChangeLog.hppa
	* sysdeps/hppa/nptl/tls.h (TLS_TCB_AT_TP): New macro.

--- a/Makeconfig
+++ b/Makeconfig
@@ -685,6 +685,7 @@ ifeq ($(all-warnings),yes)
 else
 +gccwarn := -Wall -Wwrite-strings -Winline
 endif
++gccwarn += -Wundef
 +gccwarn-c = -Wstrict-prototypes
 
 # We do not depend on the address of constants in different files to be
--- a/include/errno.h
+++ b/include/errno.h
@@ -6,6 +6,11 @@
 
 # ifdef IS_IN_rtld
 #  include <dl-sysdep.h>
+#  ifndef RTLD_PRIVATE_ERRNO
+#   error "dl-sysdep.h must define RTLD_PRIVATE_ERRNO!"
+#  endif
+# else
+#  define RTLD_PRIVATE_ERRNO	0
 # endif
 
 # if RTLD_PRIVATE_ERRNO
--- /dev/null
+++ b/include/stackinfo.h
@@ -0,0 +1,42 @@
+/* Details about the machine's stack: wrapper header.
+   Copyright (C) 2014 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; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _INCLUDE_STACKINFO_H
+#define _INCLUDE_STACKINFO_H	1
+
+/* A sysdeps/.../stackinfo.h file defines details for the CPU.
+   It is obliged to define either _STACK_GROWS_DOWN or _STACK_GROWS_UP.  */
+#include_next <stackinfo.h>
+
+#if defined _STACK_GROWS_DOWN && _STACK_GROWS_DOWN
+# ifdef _STACK_GROWS_UP
+#  error "stackinfo.h should not define both!"
+# else
+#  define _STACK_GROWS_UP	0
+# endif
+#elif defined _STACK_GROWS_UP && _STACK_GROWS_UP
+# ifdef _STACK_GROWS_DOWN
+#  error "stackinfo.h should not define both!"
+# else
+#  define _STACK_GROWS_DOWN	0
+# endif
+#else
+# error "stackinfo.h must define _STACK_GROWS_UP or _STACK_GROWS_DOWN!"
+#endif
+
+#endif  /* include/stackinfo.h */
--- a/nptl/sysdeps/i386/tls.h
+++ b/nptl/sysdeps/i386/tls.h
@@ -104,9 +104,6 @@ union user_desc_init
 };
 
 
-/* Get the thread descriptor definition.  */
-# include <nptl/descr.h>
-
 /* This is the size of the initial TCB.  Can't be just sizeof (tcbhead_t),
    because NPTL getpid, __libc_alloca_cutoff etc. need (almost) the whole
    struct pthread even when not linked with -lpthread.  */
@@ -124,6 +121,10 @@ union user_desc_init
 /* The TCB can have any size and the memory following the address the
    thread pointer points to is unspecified.  Allocate the TCB there.  */
 # define TLS_TCB_AT_TP	1
+# define TLS_DTV_AT_TP	0
+
+/* Get the thread descriptor definition.  */
+# include <nptl/descr.h>
 
 
 /* Install the dtv pointer.  The pointer passed is to the element with
--- a/nptl/sysdeps/powerpc/tls.h
+++ b/nptl/sysdeps/powerpc/tls.h
@@ -49,6 +49,7 @@ typedef union dtv
 
 /* The TP points to the start of the thread blocks.  */
 # define TLS_DTV_AT_TP	1
+# define TLS_TCB_AT_TP	0
 
 /* We use the multiple_threads field in the pthread struct */
 #define TLS_MULTIPLE_THREADS_IN_TCB	1
@@ -56,6 +57,7 @@ typedef union dtv
 /* Get the thread descriptor definition.  */
 # include <nptl/descr.h>
 
+
 /* The stack_guard is accessed directly by GCC -fstack-protector code,
    so it is a part of public ABI.  The dtv and pointer_guard fields
    are private.  */
--- a/nptl/sysdeps/s390/tls.h
+++ b/nptl/sysdeps/s390/tls.h
@@ -73,9 +73,6 @@ typedef struct
 /* Get system call information.  */
 # include <sysdep.h>
 
-/* Get the thread descriptor definition.  */
-# include <nptl/descr.h>
-
 /* This is the size of the initial TCB.  Can't be just sizeof (tcbhead_t),
    because NPTL getpid, __libc_alloca_cutoff etc. need (almost) the whole
    struct pthread even when not linked with -lpthread.  */
@@ -93,6 +90,10 @@ typedef struct
 /* The TCB can have any size and the memory following the address the
    thread pointer points to is unspecified.  Allocate the TCB there.  */
 # define TLS_TCB_AT_TP	1
+# define TLS_DTV_AT_TP	0
+
+/* Get the thread descriptor definition.  */
+# include <nptl/descr.h>
 
 
 /* Install the dtv pointer.  The pointer passed is to the element with
--- a/nptl/sysdeps/sh/tls.h
+++ b/nptl/sysdeps/sh/tls.h
@@ -76,6 +76,7 @@ typedef struct
 
 /* The TLS blocks start right after the TCB.  */
 # define TLS_DTV_AT_TP	1
+# define TLS_TCB_AT_TP	0
 
 /* Get the thread descriptor definition.  */
 # include <nptl/descr.h>
--- a/nptl/sysdeps/sparc/tls.h
+++ b/nptl/sysdeps/sparc/tls.h
@@ -69,9 +69,6 @@ typedef struct
 /* Get system call information.  */
 # include <sysdep.h>
 
-/* Get the thread descriptor definition.  */
-# include <nptl/descr.h>
-
 register struct pthread *__thread_self __asm__("%g7");
 
 /* This is the size of the initial TCB.  Can't be just sizeof (tcbhead_t),
@@ -91,6 +88,10 @@ register struct pthread *__thread_self __asm__("%g7");
 /* The TCB can have any size and the memory following the address the
    thread pointer points to is unspecified.  Allocate the TCB there.  */
 # define TLS_TCB_AT_TP	1
+# define TLS_DTV_AT_TP	0
+
+/* Get the thread descriptor definition.  */
+# include <nptl/descr.h>
 
 /* Install the dtv pointer.  The pointer passed is to the element with
    index -1 which contain the length.  */
--- a/nptl/sysdeps/x86_64/tls.h
+++ b/nptl/sysdeps/x86_64/tls.h
@@ -92,10 +92,6 @@ typedef struct
 /* Get system call information.  */
 # include <sysdep.h>
 
-
-/* Get the thread descriptor definition.  */
-# include <nptl/descr.h>
-
 #ifndef LOCK_PREFIX
 # ifdef UP
 #  define LOCK_PREFIX	/* nothing */
@@ -121,6 +117,10 @@ typedef struct
 /* The TCB can have any size and the memory following the address the
    thread pointer points to is unspecified.  Allocate the TCB there.  */
 # define TLS_TCB_AT_TP	1
+# define TLS_DTV_AT_TP	0
+
+/* Get the thread descriptor definition.  */
+# include <nptl/descr.h>
 
 
 /* Install the dtv pointer.  The pointer passed is to the element with
--- a/ports/sysdeps/hppa/nptl/tls.h
+++ b/ports/sysdeps/hppa/nptl/tls.h
@@ -51,6 +51,7 @@ typedef union dtv
 
 /* The TP points to the start of the thread blocks.  */
 # define TLS_DTV_AT_TP	1
+# define TLS_TCB_AT_TP	0
 
 /* Get the thread descriptor definition.  */
 # include <nptl/descr.h>
--- a/sysdeps/aarch64/nptl/tls.h
+++ b/sysdeps/aarch64/nptl/tls.h
@@ -48,6 +48,7 @@ typedef union dtv
 
 /* The TP points to the start of the thread blocks.  */
 # define TLS_DTV_AT_TP	1
+# define TLS_TCB_AT_TP	0
 
 /* Get the thread descriptor definition.  */
 # include <nptl/descr.h>
--- a/sysdeps/alpha/nptl/tls.h
+++ b/sysdeps/alpha/nptl/tls.h
@@ -42,6 +42,7 @@ typedef union dtv
 
 /* The TP points to the start of the thread blocks.  */
 # define TLS_DTV_AT_TP	1
+# define TLS_TCB_AT_TP	0
 
 /* Get the thread descriptor definition.  */
 # include <nptl/descr.h>
--- a/sysdeps/arm/nptl/tls.h
+++ b/sysdeps/arm/nptl/tls.h
@@ -49,6 +49,7 @@ typedef union dtv
 
 /* The TP points to the start of the thread blocks.  */
 # define TLS_DTV_AT_TP	1
+# define TLS_TCB_AT_TP	0
 
 /* Get the thread descriptor definition.  */
 # include <nptl/descr.h>
--- a/sysdeps/ia64/nptl/tls.h
+++ b/sysdeps/ia64/nptl/tls.h
@@ -87,6 +87,7 @@ register struct pthread *__thread_self __asm__("r13");
 
 /* The DTV is allocated at the TP; the TCB is placed elsewhere.  */
 # define TLS_DTV_AT_TP	1
+# define TLS_TCB_AT_TP	0
 
 /* Get the thread descriptor definition.  */
 # include <nptl/descr.h>
--- a/sysdeps/m68k/nptl/tls.h
+++ b/sysdeps/m68k/nptl/tls.h
@@ -49,6 +49,7 @@ typedef union dtv
 
 /* The TP points to the start of the thread blocks.  */
 # define TLS_DTV_AT_TP	1
+# define TLS_TCB_AT_TP	0
 
 /* Get the thread descriptor definition.  */
 # include <nptl/descr.h>
--- a/sysdeps/mach/hurd/i386/tls.h
+++ b/sysdeps/mach/hurd/i386/tls.h
@@ -26,6 +26,7 @@
 /* The TCB can have any size and the memory following the address the
    thread pointer points to is unspecified.  Allocate the TCB there.  */
 #define TLS_TCB_AT_TP	1
+#define TLS_TCB_AT_TP	0
 
 #ifndef __ASSEMBLER__
 
--- a/sysdeps/microblaze/nptl/tls.h
+++ b/sysdeps/microblaze/nptl/tls.h
@@ -48,6 +48,7 @@ typedef union dtv
 
 /* The TP points to the start of the thread blocks.  */
 # define TLS_DTV_AT_TP  1
+# define TLS_TCB_AT_TP  0
 
 /* Get the thread descriptor definition.  */
 # include <nptl/descr.h>
--- a/sysdeps/mips/nptl/tls.h
+++ b/sysdeps/mips/nptl/tls.h
@@ -67,6 +67,7 @@ typedef union dtv
 
 /* The TP points to the start of the thread blocks.  */
 # define TLS_DTV_AT_TP	1
+# define TLS_TCB_AT_TP	0
 
 /* Get the thread descriptor definition.  */
 # include <nptl/descr.h>
--- a/sysdeps/tile/nptl/tls.h
+++ b/sysdeps/tile/nptl/tls.h
@@ -49,6 +49,7 @@ typedef union dtv
 
 /* The TP points to the start of the thread blocks.  */
 # define TLS_DTV_AT_TP	1
+# define TLS_TCB_AT_TP	0
 
 /* We use the multiple_threads field in the pthread struct */
 #define TLS_MULTIPLE_THREADS_IN_TCB	1


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]