[libcamera-devel,v2,2/2] meson: Define _FORTIFY_SOURCE for optimised builds

1854 diff mbox series

_FORTIFY_SOURCE adds useful checks during compilation. The option is
enabled by default by gcc on all non-optimised builds (as it requires
-O1 or higher). Enable it explicitly for clang.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v1:

- Condition _FORTIFY_SOURCE on the optimisation level, not the build
  type
- Enable _FORTIFY_SOURCE on clang only as gcc does it by default

This patch has been tested with gcc5, gcc6, gcc7, gcc8, gcc9, clang7 and
clang8, with all the meson built types (plain, debug, debugoptimized,
release and minsize).

---
 meson.build | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

Hi Laurent,

On 19/08/2019 18:02, Laurent Pinchart wrote:
> _FORTIFY_SOURCE adds useful checks during compilation. The option is
> enabled by default by gcc on all non-optimised builds (as it requires
> -O1 or higher). Enable it explicitly for clang.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Changes since v1:
> 
> - Condition _FORTIFY_SOURCE on the optimisation level, not the build
>   type
> - Enable _FORTIFY_SOURCE on clang only as gcc does it by default
> 
> This patch has been tested with gcc5, gcc6, gcc7, gcc8, gcc9, clang7 and
> clang8, with all the meson built types (plain, debug, debugoptimized,
> release and minsize).
> 
> ---
>  meson.build | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 13d0605f903c..c30287e262a5 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -38,12 +38,22 @@ common_arguments = [
>  c_arguments = common_arguments
>  cpp_arguments = common_arguments
>  
> -# Use libc++ by default if available instead of libstdc++ when compiling with
> -# clang.
> -if cc.get_id() == 'clang' and cc.find_library('libc++', required: false).found()
> -    cpp_arguments += [
> -        '-stdlib=libc++',
> -    ]
> +if cc.get_id() == 'clang'
> +    # Turn _FORTIFY_SOURCE by default on optimised builds (as it requires -O1
> +    # or higher). This is needed on clang only as gcc enables it by default.
> +    if get_option('optimization') != '0'
> +        common_arguments += [
> +            '-D_FORTIFY_SOURCE=1',

GCC enables _FORTIFY_SOURCE level 2 by default.
Should we match that here with clang ?

$ g++ -dM -E -x c++ -O2 - < /dev/null | grep -i FORT
#define _FORTIFY_SOURCE 2


Also note, that gcc only does this for C++ builds (which shouldn't
matter in our case, until we start adding C-bindings) ...

$ gcc -dM -E - < /dev/null | grep -i FORT
$

Otherwise though,

Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Now we just need more build options covered by the automated builds at
linuxtv.org ...

> +        ]
> +    endif
> +
> +    # Use libc++ by default if available instead of libstdc++ when compiling
> +    # with clang.
> +    if cc.find_library('libc++', required: false).found()
> +        cpp_arguments += [
> +            '-stdlib=libc++',
> +        ]
> +    endif
>  endif
>  
>  add_project_arguments(c_arguments, language : 'c')
>

Hi Kieran,

On Wed, Aug 21, 2019 at 12:12:14PM +0100, Kieran Bingham wrote:
> On 19/08/2019 18:02, Laurent Pinchart wrote:
> > _FORTIFY_SOURCE adds useful checks during compilation. The option is
> > enabled by default by gcc on all non-optimised builds (as it requires
> > -O1 or higher). Enable it explicitly for clang.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > Changes since v1:
> > 
> > - Condition _FORTIFY_SOURCE on the optimisation level, not the build
> >   type
> > - Enable _FORTIFY_SOURCE on clang only as gcc does it by default
> > 
> > This patch has been tested with gcc5, gcc6, gcc7, gcc8, gcc9, clang7 and
> > clang8, with all the meson built types (plain, debug, debugoptimized,
> > release and minsize).
> > 
> > ---
> >  meson.build | 22 ++++++++++++++++------
> >  1 file changed, 16 insertions(+), 6 deletions(-)
> > 
> > diff --git a/meson.build b/meson.build
> > index 13d0605f903c..c30287e262a5 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -38,12 +38,22 @@ common_arguments = [
> >  c_arguments = common_arguments
> >  cpp_arguments = common_arguments
> >  
> > -# Use libc++ by default if available instead of libstdc++ when compiling with
> > -# clang.
> > -if cc.get_id() == 'clang' and cc.find_library('libc++', required: false).found()
> > -    cpp_arguments += [
> > -        '-stdlib=libc++',
> > -    ]
> > +if cc.get_id() == 'clang'
> > +    # Turn _FORTIFY_SOURCE by default on optimised builds (as it requires -O1
> > +    # or higher). This is needed on clang only as gcc enables it by default.
> > +    if get_option('optimization') != '0'
> > +        common_arguments += [
> > +            '-D_FORTIFY_SOURCE=1',
> 
> GCC enables _FORTIFY_SOURCE level 2 by default.
> Should we match that here with clang ?

I think so, that's a good point. I'll change it to 2 (after veriyfing
that it builds for all configurations :-)).

> $ g++ -dM -E -x c++ -O2 - < /dev/null | grep -i FORT
> #define _FORTIFY_SOURCE 2
> 
> Also note, that gcc only does this for C++ builds (which shouldn't
> matter in our case, until we start adding C-bindings) ...
> 
> $ gcc -dM -E - < /dev/null | grep -i FORT

You're missing -O2 :-)

$ gcc -dM -E -O0 - < /dev/null | grep -i FORT
$ gcc -dM -E -O2 - < /dev/null | grep -i FORT
#define _FORTIFY_SOURCE 2

> $
> 
> Otherwise though,
> 
> Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> Now we just need more build options covered by the automated builds at
> linuxtv.org ...
> 
> > +        ]
> > +    endif
> > +
> > +    # Use libc++ by default if available instead of libstdc++ when compiling
> > +    # with clang.
> > +    if cc.find_library('libc++', required: false).found()
> > +        cpp_arguments += [
> > +            '-stdlib=libc++',
> > +        ]
> > +    endif
> >  endif
> >  
> >  add_project_arguments(c_arguments, language : 'c')