Re: [hatari-devel] Enabling _FORTIFY_SOURCE for Debug builds?

[Thomas Huth <th.huth@xxxxxxxxx>]

Am Sun, 14 Apr 2024 13:09:43 +0300
schrieb Eero Tamminen <oak@xxxxxxxxxxxxxx>:

> Hi,
> 
> On 14.4.2024 12.26, Thomas Huth wrote:
> > Am Sun, 14 Apr 2024 11:47:25 +0300  
> >> On 13.4.2024 22.53, Thomas Huth wrote:  
> >>> Weird, I'm using 3.27.7 (on Fedora), and for me, it does not add the -O0
> >>> after the -g ... but OK, if there are versions that add -O0 automatically,
> >>> it's likely better to remove the -D_FORTIFY_SOURCE again. Done now.  
> >>
> >> Why you moved FORTIFY_SOURCE behind Debug check few months ago:
> >> https://github.com/hatari/hatari/commit/adc8ae2c9d0a28886e8f70f24ebfceb5097e54a0
> >> ?
> >>
> >> The reason for it being in the options was catching things before
> >> release i.e. before distros package new Hatari release and complain
> >> about things it warns about (some distros enable  FORTIFY_SOURCE for
> >> packages, and Mac tooling also enables some similar checks).  
> 
> Ubuntu enables it by default, Debian does not:
> * https://wiki.ubuntu.com/ToolChain/CompilerFlags#A-D_FORTIFY_SOURCE.3D3
> * 
> https://salsa.debian.org/toolchain-team/gcc/-/blob/master/debian/patches/gcc-distro-specs.diff?ref_type=heads#L84
> * 
> https://salsa.debian.org/toolchain-team/gcc/-/blob/master/debian/rules.patch?ref_type=heads#L326
> 
> 
> >> => Wouldn't it have made more sense to invert your Debug check?  
> > 
> > Sorry, I don't quite get your suggestion... you noticed that the line was
> > completely commented out before adc8ae2c9d0a288, did you?  
> 
> Oh, I completely missed that (red-on-black "git show" uses for removed 
> lines is not as visible as green-on-black it uses for additions)!
> 
> But for reasons I mentioned, maybe it would still be useful to enable it 
> when _not_ doing Debug build?

When I tried to enable it in the Ubuntu pipelines manually, I got the error
message that _FORTIFY_SOURCE got redefined. I think it depends on the value
that you assign to the macro (either 1, 2 or 3) and the value that is
pre-defined by the distro. So it sounds best to me to let the distro choose
it's preferred setting. We'll get test coverage via the CI anyway now, so
you don't have to worry too much that we don't have enough coverage for
this before doing a release.

 Thomas