Xapian devel - Mar 2007 - -D_FORTIFY_SOURCE (was Re: [Xapian-commits] 7903: trunk/xapian-core/)

On Wed, Mar 07, 2007 at 11:45:13AM +0000, richard wrote:
> * configure.ac: Add -D_FORTIFY_SOURCE to AM_CXXFLAGS for GCC
> builds.  According to glibc CVS support for this was added in
> October 2004, but it doesn't seem to be documented very well,
> other than in features.h.
I'd not come across this before, so I had a quick poke around.

It's hardly documented *well* in features.h either, at least in the
version I have (glibc "2.4-1ubuntu12.3")!  Sad really, it sounds like
a
promising idea, but if it's not advertised people just won't know to use
it.
> This adds some extra checking for
> array bounds, partially at compile time, but doesn't currently
> find any problems (or cause any noticeable slowdown).
There's perhaps a reason for that - the vast majority of the tests
for __USE_FORTIFY_SOURCE (which is what _FORTIFY_SOURCE controls)
in the glibc headers are followed by "&& !defined
__cplusplus"!

It also doesn't activate at all unless you're using GCC >= 4.1
so if your GCC is older, it'll have no effect at all on speed or
fortification.

The only thing which seems to actually be activated for C++ is some
stuff to do with "__builtin_object_size", which seems to allow
checking of the size of buffers known at compile time.  We don't make
much use of static sized buffers, and we use snprintf where available,
but it doesn't hurt to have extra checks if the overhead is small.

I prodded glibc CVS and found the vanilla glibc version this was first
in, though it looks like RH backported this to older GCC (and possibly
older glibc) for their distros.  I'll add that to the comment.

Cheers,
    Olly

Hmm, perhaps we should put this in config.h instead of using a configure
probe:

#if defined __GNUC__ && !defined _FORTIFY_SOURCE
# define _FORTIFY_SOURCE 2
#endif

Then the user (or distro packager) can easily override our choice when
building xapian by using:

./configure CPPFLAGS=-D_FORTIFY_SOURCE=0

or:

./configure CPPFLAGS=-D_FORTIFY_SOURCE=1

Any objections?

Cheers,
    Olly

Olly Betts wrote:
> It's hardly documented *well* in features.h either, at least in the
> version I have (glibc "2.4-1ubuntu12.3")!
Nor in mine - that was badly phrased: should have been something like 
"not documented well; a small amount of documentation is in
features.h".

 > Sad really, it sounds like a
> promising idea, but if it's not advertised people just won't know
to use
> it.
For the record, I came across it when reading this:
https://ols2006.108.redhat.com/reprints/jones-reprint.pdf
> There's perhaps a reason for that - the vast majority of the tests
> for __USE_FORTIFY_SOURCE (which is what _FORTIFY_SOURCE controls)
> in the glibc headers are followed by "&& !defined
__cplusplus"!
Bah!  Well, I did manage to get an error out of it by breaking some of 
the code in flint, so it's not completely useless; hopefully C++ support 
will get better with time.
> It also doesn't activate at all unless you're using GCC >= 4.1
> so if your GCC is older, it'll have no effect at all on speed or
> fortification.
Ah, I couldn't find documentation on which GCC you needed for it, but I 
assumed it was fairly recent.
> The only thing which seems to actually be activated for C++ is some
> stuff to do with "__builtin_object_size", which seems to allow
> checking of the size of buffers known at compile time.  We don't make
> much use of static sized buffers, and we use snprintf where available,
> but it doesn't hurt to have extra checks if the overhead is small.
> 
> I prodded glibc CVS and found the vanilla glibc version this was first
> in, though it looks like RH backported this to older GCC (and possibly
> older glibc) for their distros.  I'll add that to the comment.
Cool.

-- 
Richard