symbol bloat for fortify warnings

symbol bloat for fortify warnings

[Roland McGrath]

I really dislike the addition of symbols in the dynamic symbol table, and
in the ABI we will maintain permanently, just as a kludge for emitting
link-time warnings.  We can get the warnings done just with stuff in
libc_nonshared.a, and not suffer this bloat.

I propose the following approach instead.  This works now to produce linker
warnings.  When gcc is hopefully soon extended to support an attribute like
`deprecated' but with supplied message text, it can be:

#define __warndecl(name, msg) \
  static __inline__ __attribute__ ((__always_inline__, __warning__ (msg))) \
  void name (void) { }

to get better warnings at compile-time and produce no run-time overhead at all.

Ok?


Thanks,
Roland


2005-02-24  Roland McGrath  <roland@redhat.com>

	* debug/Versions (libc: GLIBC_2.4): Remove
	__memset_zero_constant_len_parameter.
	* sysdeps/generic/memset_chk.c: Remove alias and warning.
	* misc/sys/cdefs.h (__warndecl): New macro.
	* debug/warning-nop.c: New file.
	* string/bits/string3.h (memset): Call __warn_memset_zero_len with no
	arguments, instead of calling __memset_zero_constant_len_parameter.
	Use __warndecl for __warn_memset_zero_len.
	* debug/Makefile (routines): Add $(static-only-routines).
	(static-only-routines): New variable.

Index: misc/sys/cdefs.h
===================================================================
RCS file: /cvs/glibc/libc/misc/sys/cdefs.h,v
retrieving revision 1.61
diff -B -b -p -u -r1.61 cdefs.h
--- misc/sys/cdefs.h	7 Jan 2005 19:39:28 -0000	1.61
+++ misc/sys/cdefs.h	25 Feb 2005 02:46:07 -0000
@@ -130,6 +130,7 @@
 /* Fortify support.  */
 #define __bos(ptr) __builtin_object_size (ptr, __USE_FORTIFY_LEVEL > 1)
 #define __bos0(ptr) __builtin_object_size (ptr, 0)
+#define __warndecl(name, msg) extern void name (void)
 
 
 /* Support for flexible arrays.  */
Index: string/bits/string3.h
===================================================================
RCS file: /cvs/glibc/libc/string/bits/string3.h,v
retrieving revision 1.2
diff -B -b -p -u -r1.2 string3.h
--- string/bits/string3.h	21 Feb 2005 23:04:07 -0000	1.2
+++ string/bits/string3.h	25 Feb 2005 02:46:07 -0000
@@ -85,11 +85,11 @@ __mempcpy_ichk (void *__restrict __dest,
    especially problematic if the intended fill value is zero.  In this
    case no work is done at all.  We detect these problems by referring
    non-existing functions.  */
-extern char *__memset_zero_constant_len_parameter (void *, int, size_t,
-						   size_t);
+__warndecl (__warn_memset_zero_len,
+	    "memset used with constant zero length parameter; this could be due to transposed parameters");
 #define memset(dest, ch, len) \
   (__builtin_constant_p (len) && (len) == 0				      \
-   ? __memset_zero_constant_len_parameter (dest, ch, len, 0)		      \
+   ? (__warn_memset_zero_len (), (void) (ch), (void) (len), (void *) (dest))  \
    : ((__bos0 (dest) != (size_t) -1)					      \
       ? __builtin___memset_chk (dest, ch, len, __bos0 (dest))		      \
       : __memset_ichk (dest, ch, len)))
Index: debug/warning-nop.c
===================================================================
RCS file: debug/warning-nop.c
diff -N debug/warning-nop.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ debug/warning-nop.c	25 Feb 2005 02:46:07 -0000
@@ -0,0 +1,38 @@
+/* Dummy nop functions to elicit link-time warnings.
+   Copyright (C) 2005 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, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+#include <sys/cdefs.h>
+
+void __nop (void)
+{
+}
+
+/* Don't insert any other #include's before this #undef!  */
+
+#undef __warndecl
+#define __warndecl(name, msg) \
+  strong_alias (__nop, name) link_warning (name, msg)
+
+#undef	__USE_FORTIFY_LEVEL
+#define __USE_FORTIFY_LEVEL 99
+
+/* Following here we need an #include for each public header file
+   that uses __warndecl.  */
+
+#include <string.h>
Index: debug/Makefile
===================================================================
RCS file: /cvs/glibc/libc/debug/Makefile,v
retrieving revision 1.23
diff -B -b -p -u -r1.23 Makefile
--- debug/Makefile	21 Feb 2005 23:08:04 -0000	1.23
+++ debug/Makefile	25 Feb 2005 02:46:07 -0000
@@ -24,14 +24,16 @@ subdir	:= debug
 headers	:= execinfo.h
 distribute = sigcontextinfo.h register-dump.h frame.h
 
-routines := backtrace backtracesyms backtracesymsfd noophooks \
+routines  = backtrace backtracesyms backtracesymsfd noophooks \
 	    memcpy_chk memmove_chk mempcpy_chk memset_chk stpcpy_chk \
 	    strcat_chk strcpy_chk strncat_chk strncpy_chk \
 	    sprintf_chk vsprintf_chk snprintf_chk vsnprintf_chk \
 	    printf_chk fprintf_chk vprintf_chk vfprintf_chk \
 	    gets_chk chk_fail readonly-area fgets_chk fgets_u_chk \
 	    read_chk pread_chk pread64_chk recv_chk recvfrom_chk \
-	    readlink_chk getwd_chk getcwd_chk
+	    readlink_chk getwd_chk getcwd_chk \
+	    $(static-only-routines)
+static-only-routines := warning-nop
 
 CFLAGS-backtrace.c = -fno-omit-frame-pointer
 CFLAGS-sprintf_chk.c = -D_IO_MTSAFE_IO
Index: debug/Versions
===================================================================
RCS file: /cvs/glibc/libc/debug/Versions,v
retrieving revision 1.5
diff -B -b -p -u -r1.5 Versions
--- debug/Versions	21 Feb 2005 23:06:50 -0000	1.5
+++ debug/Versions	25 Feb 2005 02:46:07 -0000
@@ -22,7 +22,6 @@ libc {
     __fgets_chk; __fgets_unlocked_chk;
     __read_chk; __pread_chk; __pread64_chk;
     __readlink_chk; __getcwd_chk; __getwd_chk;
-    __memset_zero_constant_len_parameter;
     __recv_chk; __recvfrom_chk;
   }
 }
Index: sysdeps/generic/memset_chk.c
===================================================================
RCS file: /cvs/glibc/libc/sysdeps/generic/memset_chk.c,v
retrieving revision 1.2
diff -B -b -p -u -r1.2 memset_chk.c
--- sysdeps/generic/memset_chk.c	21 Feb 2005 23:05:43 -0000	1.2
+++ sysdeps/generic/memset_chk.c	25 Feb 2005 02:46:07 -0000
@@ -90,7 +90,3 @@ __memset_chk (dstpp, c, len, dstlen)
 
   return dstpp;
 }
-strong_alias (__memset_chk, __memset_zero_constant_len_parameter)
-
-link_warning (__memset_zero_constant_len_parameter,
-	      "memset used with constant zero length parameter; this could be due to transposed parameters")

Re: symbol bloat for fortify warnings

[Ulrich Drepper]

[-- Attachment #1: Type: text/plain, Size: 405 bytes --]

This nop function will work for this special case, and I agree, it
should be used.

But it is no general solution.  For instance, there is a related test
which I have not yet implemented.  If the second memset parameter is >
255, the called function cannot be a no-op and should not be in
libc_nonshared.

--
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

Re: symbol bloat for fortify warnings

[Roland McGrath]

> This nop function will work for this special case, and I agree, it
> should be used.

Ok.  

> But it is no general solution.  For instance, there is a related test
> which I have not yet implemented.  If the second memset parameter is >
> 255, the called function cannot be a no-op and should not be in
> libc_nonshared.

I really don't see a problem with having this produce a no-op call for the
warning, plus the normal memset call.  In fact, I think it's preferable to
make it:

__builtin_constant_p (ch) && (ch) > 255
? (__warn_memset_ch_overflows (), __builtin_memset ((dest), (ch), (len))) : ...

so that the compiler might still choose to inline the memset (and if not it
will generate the normal "memset" call directly).  In the current
implementation, that will produce one useless call+ret for the warning
stub.  But that is only a temporary situation until we have the compiler
support to emit the warning directly without any call in the assembly emitted.


Thanks,
Roland