View unanswered posts | View active topics It is currently Tue Sep 17, 2019 2:57 am



Reply to topic  [ 8 posts ] 
Patches to compile with CPPFLAGS=-D_FORTIFY_SOURCE=2 
Author Message
Rookie

Joined: Wed Jul 04, 2012 2:24 pm
Posts: 14
Reply with quote
Post Patches to compile with CPPFLAGS=-D_FORTIFY_SOURCE=2
Dear ZSNES devs,

I'd like to get your (dis)approval for a patch set that I am about to apply to the zsnes package in Debian.

The underlying problem is that Debian encourages to build package with "hardening build flags", i.e. compiler flags that add security-related features like stack protection. These flags also add "-D_FORTIFY_SOURCE=2" to CPPFLAGS, which cause zsnes to fail to build. An analysis of the problem is given by ubuntu here:
https://bugs.launchpad.net/ubuntu/intrepid/+source/zsnes/+bug/250425/comments/45

However, Fedora, nee RPM Fusion, provides a patch for it:
http://cvs.rpmfusion.org/viewvc/rpms/zsnes/devel/zsnes-1.51-FORTIFY_SOURCE.patch?revision=1.1&root=free&view=markup

With this patch, zsnes compiles fine with GCC 4.6.x and "-D_FORTIFY_SOURCE=2" enabled in CPPFLAGS, although it spits a lot of warnings. However, when the compiler is updated to GCC 4.7.x, compilation breaks again for code that is produced by parsegen, so an additional patch is needed to omit the direct creation of object code from parsegen but create intermediate .c-files instead:
http://cvs.rpmfusion.org/viewvc/rpms/zsnes/devel/zsnes-1.51-psr.patch?revision=1.1&root=free&view=markup

As you know the affected code best, could you please do me a favour and share your opinion about these two patches?

Best regards,
Fabian


Mon Aug 27, 2012 10:56 am
Profile
ZSNES Developer
ZSNES Developer
User avatar

Joined: Tue Jul 27, 2004 10:54 pm
Posts: 3901
Location: Solar powered park bench
Reply with quote
Post Re: Patches to compile with CPPFLAGS=-D_FORTIFY_SOURCE=2
The patches proposed by Fedora for memcpy() indeed fix the "problem" however they misrepresent the variables with their changes. We're aware that GCC is unable to see all of ZSNES assembly code, so it outputs warnings for situations that in "normal" code would be a problem.

The correct solution would be to port of all of ZSNES's assembly code there to C, but that's a huge job, which is only partially done.

If you want to use the patches to initc.c and zstate.c to quiet some warnings, I *think* they're okay, but we wouldn't include them in ZSNES itself. I say "think", because I haven't checked to see if they're actually used anywhere other than in memcpy() routines.

Regarding the patch to parsegen, we know that such a change will cause certain versions of GCC to produce invalid code with certain system CFLAGS. You'll note that not only does parsegen compile internally, but after the CFLAGS it adds to the end -O1. We found certain versions of GCC were producing incorrect code at -O2 and -O3 for the C files parsegen produces.

_________________
May 9 2007 - NSRT 3.4, now with lots of hashing and even more accurate information! Go download it.
_____________
Insane Coding


Mon Aug 27, 2012 9:32 pm
Profile WWW
Rookie

Joined: Wed Jul 04, 2012 2:24 pm
Posts: 14
Reply with quote
Post Re: Patches to compile with CPPFLAGS=-D_FORTIFY_SOURCE=2
Nach wrote:
Regarding the patch to parsegen, we know that such a change will cause certain versions of GCC to produce invalid code with certain system CFLAGS. You'll note that not only does parsegen compile internally, but after the CFLAGS it adds to the end -O1. We found certain versions of GCC were producing incorrect code at -O2 and -O3 for the C files parsegen produces.


I have just made a different experience and am not sure if it is in contradiction to your statement above.

Currently, with -O1 added to the system CFLAGS, parsegen fails to compile internally. If I remove the additional -O1 and compile only with the system CFLAGS (which have got -O3 added by the ZSNES Makefile, BTW), the code compiles cleanly. It does, however, also succeed to compile internally if I leave the additional -O1 in place and simply add "-fipa-cp".
Do you consider this safe?

- Fabian


Tue Sep 04, 2012 1:55 pm
Profile
ZSNES Developer
ZSNES Developer
User avatar

Joined: Tue Jul 27, 2004 10:54 pm
Posts: 3901
Location: Solar powered park bench
Reply with quote
Post Re: Patches to compile with CPPFLAGS=-D_FORTIFY_SOURCE=2
fabian wrote:
Currently, with -O1 added to the system CFLAGS, parsegen fails to compile internally.

I find that extremely odd. Can you paste for me the line it's trying to use to compile with?

fabian wrote:
If I remove the additional -O1 and compile only with the system CFLAGS (which have got -O3 added by the ZSNES Makefile, BTW), the code compiles cleanly.

Which however does not mean that the compilation is correct.

fabian wrote:
It does, however, also succeed to compile internally if I leave the additional -O1 in place and simply add "-fipa-cp".
Do you consider this safe?

I'm not famialiar with that GCC option. I've been searching for some documentation for a couple of minutes now, but to no avail.

_________________
May 9 2007 - NSRT 3.4, now with lots of hashing and even more accurate information! Go download it.
_____________
Insane Coding


Sat Sep 08, 2012 7:49 pm
Profile WWW
Rookie

Joined: Wed Jul 04, 2012 2:24 pm
Posts: 14
Reply with quote
Post Re: Patches to compile with CPPFLAGS=-D_FORTIFY_SOURCE=2
Nach wrote:
I find that extremely odd. Can you paste for me the line it's trying to use to compile with?


./parsegen -D__UNIXSDL__ -D__OPENGL__ -gcc gcc -compile -flags "-g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -D_FORTIFY_SOURCE=2 -pipe -I. -I/usr/local/include -I/usr/include -D__UNIXSDL__ -I/usr/include/SDL -D_GNU_SOURCE=1 -D_REENTRANT -D__LIBAO__ -D__OPENGL__ -march=i486 -O3 -fomit-frame-pointer -fprefetch-loop-arrays -fforce-addr -D__RELEASE__ -O1" -cheader cfg.h -fname cfg cfg.o cfg.psr
parsegen: gcc -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -D_FORTIFY_SOURCE=2 -pipe -I. -I/usr/local/include -I/usr/include -D__UNIXSDL__ -I/usr/include/SDL -D_GNU_SOURCE=1 -D_REENTRANT -D__LIBAO__ -D__OPENGL__ -march=i486 -O3 -fomit-frame-pointer -fprefetch-loop-arrays -fforce-addr -D__RELEASE__ -O1 -o cfg.o -c cfg.c
cfg.c:1:0: warning: -fprefetch-loop-arrays not supported for this target (try -march switches) [enabled by default]
In file included from /usr/include/stdio.h:930:0,
from cfg.c:5:
cfg.c: In function ‘write_cfg_vars’:
/usr/include/i386-linux-gnu/bits/stdio2.h:96:1: error: inlining failed in call to always_inline ‘fprintf’: indirect function call with a yet undetermined callee
cfg.c:552:7: error: called from here
In file included from /usr/include/stdio.h:930:0,
from cfg.c:5:
/usr/include/i386-linux-gnu/bits/stdio2.h:96:1: error: inlining failed in call to always_inline ‘fprintf’: indirect function call with a yet undetermined callee
[...]

Quote:
I'm not famialiar with that GCC option. I've been searching for some documentation for a couple of minutes now, but to no avail.


-fipa-cp
Perform interprocedural constant propagation. This optimization analyzes the program to determine when values passed to functions are constants and then optimizes accordingly. This optimization can substantially increase performance if the application has constants passed to functions. This flag is enabled by default at -O2, -Os and -O3.
http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/Optimize-Options.html#Optimize-Options


Mon Sep 10, 2012 11:02 am
Profile
ZSNES Developer
ZSNES Developer
User avatar

Joined: Tue Jul 27, 2004 10:54 pm
Posts: 3901
Location: Solar powered park bench
Reply with quote
Post Re: Patches to compile with CPPFLAGS=-D_FORTIFY_SOURCE=2
In general, your flags look odd. -g? -O2?

In any case, I played with your line, the exact issue seems to occur with -O1 combined with -D_FORTIFY_SOURCE=2. Smells like yet another GCC bug. Although probably can put some blame on EGLIBC as well, as it changes how stdio works with fortify source, and is defining various functions as requiring inline.

I looked up -fipa-cp, and it seems it was introduced in GCC 4.3, which is around the same time we first started noticing issues with parsegen generated files being compiled at -O2 and -O3. Since -O2 and higher turns it on, for all I know, it's the cause, I haven't tested it.

I noticed that using -D_FORTIFY_SOURCE=2 with -O0 compiles. I don't recall though if we ever verified -O0 as working properly. Also based on how different the code is generated with all these options, I wonder if yet another GCC bug isn't hiding here. I think we'll need someone to go over the GCC generated code and ensure no "else if" statements are being deleted like we found in the past. Or alternatively, testing if all changes in the config file propagate properly to ZSNES in action, to ensure that the config file is indeed parsed as intended.

_________________
May 9 2007 - NSRT 3.4, now with lots of hashing and even more accurate information! Go download it.
_____________
Insane Coding


Tue Sep 11, 2012 4:22 pm
Profile WWW
Rookie

Joined: Wed Jul 04, 2012 2:24 pm
Posts: 14
Reply with quote
Post Re: Patches to compile with CPPFLAGS=-D_FORTIFY_SOURCE=2
Nach wrote:
In general, your flags look odd. -g? -O2?

These belong to the standard build flags in Debian.

Anyway, thanks for your analysis. For the time being, I just removed the trailing -O1 from the parsegen command line. This has the same effect as the Fedora patch, which avoinds compilation inside parsegen and leads to the creation intermediate .c files which are then compiled into object code with the standard flags.


Mon Sep 17, 2012 1:12 pm
Profile
ZSNES Shake Shake Prinny
User avatar

Joined: Wed Jul 28, 2004 4:15 pm
Posts: 5615
Location: PAL50, dood !
Reply with quote
Post Re: Patches to compile with CPPFLAGS=-D_FORTIFY_SOURCE=2
Can't say having -g as systemwide default is a good move, but YMMV I guess.

_________________
皆黙って俺について来い!!
Code:
<jmr> bsnes has the most accurate wiki page but it takes forever to load (or something)

Pantheon: Gideon Zhi | CaitSith2 | Nach | kode54


Tue Sep 18, 2012 8:54 pm
Profile
Display posts from previous:  Sort by  
Reply to topic   [ 8 posts ] 

Who is online

Users browsing this forum: No registered users and 2 guests


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum

Search for:
Jump to:  
Powered by phpBB © 2000, 2002, 2005, 2007 phpBB Group.
Designed by ST Software.