sshock / AFFLIBv3

AFF is an open and extensible file format to store disk images and associated metadata.
Other
77 stars 20 forks source link

configure.ac: Unconditionally defining _FORTIFY_SOURCE=2 overrides _FORTIFY_SOURCE=3 #58

Open chrfranke opened 5 months ago

chrfranke commented 5 months ago

Many recent distros provide GCC >= 12.0 which supports __builtin_dynamic_object_size(): https://gcc.gnu.org/onlinedocs/gcc/Object-Size-Checking.html

This is used instead of __builtin_object_size() by newer CRT includes if _FORTIFY_SOURCE=3 is set.

Compiler presets or packaging support tools now often default to _FORTIFY_SOURCE=3 which would be downgraded by these lines of configure.ac:

CFLAGS="$CFLAGS -D_FORTIFY_SOURCE=2 -Wall -g"
CXXFLAGS="$CXXFLAGS -D_FORTIFY_SOURCE=2 -Wall -g"

(A similar problem may be the -g which may override -ggdb)

sshock commented 5 months ago

Hi Christian. Are you suggesting simply set _FORTIFY_SOURCE to 3 and change -g to -ggdb here in configure.ac?

That sounds fine and good to me, other than I'm not 100% sure the compiler must always be gcc, and if it were something else -ggdb wouldn't be valid.

chrfranke commented 5 months ago

If downstream predefines certain *FLAGS, these should IMO always have preference.

Here a working example for configure.ac which sets _FORTIFY_SOURCE=3 if not pre(un)defined. A full check, which also detects compiler presets (e.g. Ubuntu gcc), would require AC_COMPILE_IF_ELSE.

case " $CPPFLAGS $CFLAGS $CXXFLAGS " in
  *\ -[[DU]]_FORTIFY_SOURCE\ *|*\ -D_FORTIFY_SOURCE=*)
    ;; # _FORTIFY_SOURCE pre(un)defined (does not detect compiler presets)
  *)
    CFLAGS="$CFLAGS -D_FORTIFY_SOURCE=3"
    CXXFLAGS="$CXXFLAGS -D_FORTIFY_SOURCE=3" ;;
esac

Another recommended security flag is -fstack-protector-strong (gcc >= 4.9). It is typically also preset on recent distributions.

At least in the ancient past (IIRC at least with the solaris compiler), even -Wall was not portable. Here an example which sets -Wall only for gcc/clang (GCC=yes in configure) and also sets -Werror=return-type for C++ only.

case "$GCC: $CFLAGS $CXXFLAGS " in
  :*|*\ -W*)
    ;; # No gcc/clang used or -W option(s) are predefined
  yes:*)
    CFLAGS="$CFLAGS -Wall"
    # Never ignore -Wreturn-type as g++ >= 8.0 assumes that control never
    # reaches the end of a non-void function.
    CXXFLAGS="$CXXFLAGS -Wall -Werror=return-type" ;;
esac

BTW, there is no need to set -g because configure already does this by default if the compiler supports it. It also adds -O2 for gcc/clang.

sshock commented 5 months ago

Could you provide a pull request?