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

[Laurent Pinchart]

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 at 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 at 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')

-- 
Regards,

Laurent Pinchart