[PATCH] Add -D_FORTIFY_SOURCE=2 to CFLAGS if possible.

[PATCH] Add -D_FORTIFY_SOURCE=2 to CFLAGS if possible.

[Mark Wielaard]

Some distros now add -D_FORTIFY_SOURCE=2 by default and we have missed
some issues in the past caught by it. Add it to CFLAGS if possible.
The configure check will make sure that it doesn't conflict with any
other CFLAGS already defined.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 ChangeLog    |  4 ++++
 configure.ac | 21 +++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index a587b43..b50f79e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2017-02-09  Mark Wielaard  <mark@klomp.org>
+
+	* configure.ac: Add check for adding -D_FORTIFY_SOURCE=2 to CFLAGS.
+
 2017-01-12  Mark Wielaard  <mark@klomp.org>
 
 	* configure.ac: Define PACKAGE_URL for older autoconf.
diff --git a/configure.ac b/configure.ac
index 07ad592..46055f2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -159,6 +159,27 @@ AC_CACHE_CHECK([whether fts.h is bad when included (with LFS)], ac_cv_bad_fts,
 		     ac_cv_bad_fts=no, ac_cv_bad_fts=yes)])
 AS_IF([test "x$ac_cv_bad_fts" = "xyes"], [CFLAGS="$CFLAGS -DBAD_FTS=1"])
 
+# See if we can add -D_FORTIFY_SOURCE=2. Don't do it if it is already
+# (differently) defined or if it generates warnings/errors because we
+# don't use the right optimisation level (string.h will warn about that).
+AC_MSG_CHECKING([whether to add -D_FORTIFY_SOURCE=2 to CFLAGS])
+case "$CFLAGS" in
+  *-D_FORTIFY_SOURCE=2*)
+    AC_MSG_RESULT([no, already there])
+    ;;
+  *)
+    save_CFLAGS="$CFLAGS"
+    CFLAGS="-D_FORTIFY_SOURCE=2 -Werror $CFLAGS"
+    AC_COMPILE_IFELSE([AC_LANG_SOURCE([[
+      #include <string.h>
+      int main() { return 0; }
+    ]])], [ AC_MSG_RESULT([yes])
+            CFLAGS="-D_FORTIFY_SOURCE=2 $save_CFLAGS" ],
+          [ AC_MSG_RESULT([no])
+            CFLAGS="$save_CFLAGS"])
+  ;;
+esac
+
 dnl enable debugging of branch prediction.
 AC_ARG_ENABLE([debugpred],
 AS_HELP_STRING([--enable-debugpred],[build binaries with support to debug branch prediction]),
-- 
1.8.3.1

Re: [PATCH] Add -D_FORTIFY_SOURCE=2 to CFLAGS if possible.

[Mark Wielaard]

On Thu, 2017-02-09 at 21:13 +0100, Mark Wielaard wrote:
> Some distros now add -D_FORTIFY_SOURCE=2 by default and we have missed
> some issues in the past caught by it. Add it to CFLAGS if possible.
> The configure check will make sure that it doesn't conflict with any
> other CFLAGS already defined.

I pushed this to master now.

Re: [PATCH] Add -D_FORTIFY_SOURCE=2 to CFLAGS if possible.

[Mike Frysinger]

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

On 09 Feb 2017 21:13, Mark Wielaard wrote:
> +# See if we can add -D_FORTIFY_SOURCE=2. Don't do it if it is already
> +# (differently) defined or if it generates warnings/errors because we
> +# don't use the right optimisation level (string.h will warn about that).
> +AC_MSG_CHECKING([whether to add -D_FORTIFY_SOURCE=2 to CFLAGS])

-D flags should be in CPPFLAGS ...

> +case "$CFLAGS" in
> +  *-D_FORTIFY_SOURCE=2*)
> +    AC_MSG_RESULT([no, already there])
> +    ;;
> +  *)
> +    save_CFLAGS="$CFLAGS"
> +    CFLAGS="-D_FORTIFY_SOURCE=2 -Werror $CFLAGS"
> +    AC_COMPILE_IFELSE([AC_LANG_SOURCE([[
> +      #include <string.h>
> +      int main() { return 0; }
> +    ]])], [ AC_MSG_RESULT([yes])
> +            CFLAGS="-D_FORTIFY_SOURCE=2 $save_CFLAGS" ],
> +          [ AC_MSG_RESULT([no])
> +            CFLAGS="$save_CFLAGS"])
> +  ;;

why not AC_PREPROC_IFELSE and check if _FORTIFY_SOURCE is defined ?
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] Add -D_FORTIFY_SOURCE=2 to CFLAGS if possible.

[Mark Wielaard]

On Tue, 2017-02-14 at 21:58 -0500, Mike Frysinger wrote:
> On 09 Feb 2017 21:13, Mark Wielaard wrote:
> > +# See if we can add -D_FORTIFY_SOURCE=2. Don't do it if it is already
> > +# (differently) defined or if it generates warnings/errors because we
> > +# don't use the right optimisation level (string.h will warn about that).
> > +AC_MSG_CHECKING([whether to add -D_FORTIFY_SOURCE=2 to CFLAGS])
> 
> -D flags should be in CPPFLAGS ...

Normally yes, but _FORTIFY_SOURCE is "special". It depends on CFLAGS
optimization settings. If the user has given CFLAGS="-O0" for example,
or set _FORTIFY_SOURCE to 1 in CFLAGS already adding -D_FORTIFY_SOURCE=2
to CPPFLAGS won't work as intended.

> > +case "$CFLAGS" in
> > +  *-D_FORTIFY_SOURCE=2*)
> > +    AC_MSG_RESULT([no, already there])
> > +    ;;
> > +  *)
> > +    save_CFLAGS="$CFLAGS"
> > +    CFLAGS="-D_FORTIFY_SOURCE=2 -Werror $CFLAGS"
> > +    AC_COMPILE_IFELSE([AC_LANG_SOURCE([[
> > +      #include <string.h>
> > +      int main() { return 0; }
> > +    ]])], [ AC_MSG_RESULT([yes])
> > +            CFLAGS="-D_FORTIFY_SOURCE=2 $save_CFLAGS" ],
> > +          [ AC_MSG_RESULT([no])
> > +            CFLAGS="$save_CFLAGS"])
> > +  ;;
> 
> why not AC_PREPROC_IFELSE and check if _FORTIFY_SOURCE is defined ?

Same reason. We need to test whether the combination of
-D_FORTIFY_SOURCE=2 and any other CFLAG/CPPFLAGS flags work or not. Just
scanning preprocessor flags won't show issues exposed by actually
compiling with a C header (string.h) to see if that errors out.

Cheers,

Mark