minaco2 / distcc

Automatically exported from code.google.com/p/distcc
GNU General Public License v2.0
0 stars 0 forks source link

Only undefine _FORTIFY_SOURCE when building with -Werror #87

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
In distcc 3.1 (as packaged by Gentoo as sys-devel/distcc-3.1-r5), but still 
present in current head r746, configure.ac around line 205 sets:

PYTHON_CFLAGS="-Wno-missing-prototypes -Wno-missing-declarations \
-Wno-write-strings -Wp,-U_FORTIFY_SOURCE"

The comment above it states that this is because distcc code triggers "errors" 
from _FORTIFY_SOURCE by ignoring libc return values.  This is slightly 
inaccurate.  The libc functions in question are marked as warn_unused_result, 
so distcc causes a warning by ignoring their results.  Although the code should 
be fixed to check these return values and therefore not cause the warnings, 
this ticket is about a much simpler change.  Those warnings are only promoted 
to errors if -Werror is used.  I suggest changing the configure script so that 
-U_FORTIFY_SOURCE is only used if -Werror is also used.  That way, people who 
use --disable-Werror can get the other benefits of source fortification, such 
as improved strcpy checks, enhanced printf checking, open missing mode 
checking, and so on.

Although likely not relevant for a request of source change, answers to 
standard questions follow:

1. sys-devel/distcc-3.1-r5
2. x86_64 / sys-devel/gcc 4.5.3-r1
3. Compile distcc with compiler-assisted hardening features.
4. The configure script suppressed the hardening features, even though the 
build runs to completion with the hardening features enabled.
5. Compiler invocation did not fail.
6. No logs were generated.  The configure script suppressed hardening and did 
not even advertise it.
7. No error messages.

Gentoo runs configure with:
./configure --prefix=/usr --build=x86_64-pc-linux-gnu 
--host=x86_64-pc-linux-gnu --mandir=/usr/share/man --infodir=/usr/share/info 
--datadir=/usr/share --sysconfdir=/etc --localstatedir=/var/lib 
--libdir=/usr/lib64 --without-avahi --with-gtk --without-gnome --disable-Werror 
--with-docdir=/usr/share/doc/distcc-3.1-r5 --enable-rfc2553

The below patch achieves the proposed change:
diff -ru a/configure.ac b/configure.ac
--- a/configure.ac  2008-12-02 21:50:31.000000000 +0000
+++ b/configure.ac  2011-11-12 18:50:49.094680602 +0000
@@ -189,6 +189,12 @@
     if test x"$enable_Werror" != xno
     then
       WERROR_CFLAGS="-Werror"
+     # -Wp,-U_FORTIFY_SOURCE is to turn off _FORTIFY_SOURCE on systems where
+     # it's in the Python Makefile (and hence inherited by us).
+     # -Werror -D_FORTIFY_SOURCE gives compiler errors for some distcc routines 
that
+     # ignore the return value from libc functions (like getcwd).
+     # That would cause this code to not compile, which is no good.
+     PYTHON_CFLAGS="-Wp,-U_FORTIFY_SOURCE"
     fi

     # Additional flags for compiling Python extension modules.
@@ -197,13 +203,8 @@
     # pointer), and -Wwrite-strings, which just had too many false
     # positives (for Python 2.2, anyway; looks like these may be fixed
     # in Python 2.5).
-    # -Wp,-U_FORTIFY_SOURCE is to turn off _FORTIFY_SOURCE on systems where
-    # it's in the Python Makefile (and hence inherited by us).
-    # _FORTIFY_SOURCE gives compiler errors for some distcc routines that
-    # ignore the return value from libc functions (like getcwd).
-    # That would cause this code to not compile, which is no good.
-    PYTHON_CFLAGS="-Wno-missing-prototypes -Wno-missing-declarations \
--Wno-write-strings -Wp,-U_FORTIFY_SOURCE"
+    PYTHON_CFLAGS="$PYTHON_CFLAGS -Wno-missing-prototypes 
-Wno-missing-declarations \
+-Wno-write-strings"

     # For popt/*.c, we disable unused variable warnings.
     POPT_CFLAGS="-Wno-unused"

Original issue reported on code.google.com by google.8...@spamgourmet.com on 12 Nov 2011 at 6:58

GoogleCodeExporter commented 9 years ago
The patch looks good to me.

Original comment by fergus.h...@gmail.com on 14 Nov 2011 at 7:27

GoogleCodeExporter commented 9 years ago
I committed the patch to the svn repository: fixed in svn revision 755.

Original comment by fergus.h...@gmail.com on 6 Feb 2012 at 7:59