Skip to content

configure: Raise FORTIFY_SOURCE level to 3#642

Merged
michaelrsweet merged 1 commit intoOpenPrinting:masterfrom
zdohnal:raise_fortify
May 1, 2023
Merged

configure: Raise FORTIFY_SOURCE level to 3#642
michaelrsweet merged 1 commit intoOpenPrinting:masterfrom
zdohnal:raise_fortify

Conversation

@zdohnal
Copy link
Member

@zdohnal zdohnal commented Mar 24, 2023

GCC supports level 3 for some time, try using it.

Copy link
Member

@michaelrsweet michaelrsweet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to test whether level 3 works before using it.

@michaelrsweet
Copy link
Member

FWIW, macOS would allow a value of 3 but doesn't do anything different from 2.

@michaelrsweet
Copy link
Member

michaelrsweet commented Mar 24, 2023

Info about what level 3 does is here

I'm not sure whether the CI infrastructure supports level 3 - have you run:

./configure --enable-debug --disable-shared
make
make test

on a system that supports it?

@zdohnal
Copy link
Member Author

zdohnal commented Mar 24, 2023

Ok, interesting - if I use --enable-debug --disable-shared I can't compile due (regardless of FORTIFY_SOURCE patch):

Linking ippevepcl...
/usr/bin/ld: ippevepcl.o: relocation R_X86_64_32 against `.rodata' can not be used when making a PIE object; recompile with -fPIE

Adding -fPIC into CFLAGS fixes it (on contrary to the message)... but in cups-sharedlibs.m4 we set PICFLAG to 0 if --disable-shared is used.

Either way, the compilation goes fine here - I have Fedora 38, where all packages should be built with -D_FORTIFY_SOURCE=3, so gcc supports it and if I build our current master here with the flag, tests pass.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to look for presence of _FORTIFY_SOURCE in the flags and then skipping addition if it's already present? That way the flag won't end up overriding distribution flags.

@zdohnal zdohnal force-pushed the raise_fortify branch 2 times, most recently from 65fda3c to c7dd5aa Compare April 28, 2023 08:02
If _FORTIFY_SOURCE is not defined in flags, use its level 3.

Backported from libcups.
@zdohnal
Copy link
Member Author

zdohnal commented Apr 28, 2023

Total size with _FORTIFY_SOURCE=3 = 14 634 920
Total size with _FORTIFY_SOURCE=2 = 14 622 368

Time consumption:
_FORTIFY_SOURCE=3:

real	2m10.716s
user	0m11.175s
sys	0m2.953s

_FORTIFY_SOURCE=2:

real	2m10.428s
user	0m10.849s
sys	0m2.999s

So the tests are slightly slower and object files bigger on Fedora 39 with _FORTIFY_SOURCE=3, but IMO it is within acceptable limits.

@michaelrsweet michaelrsweet self-assigned this May 1, 2023
@michaelrsweet michaelrsweet added enhancement New feature or request priority-low platform issue Issue is specific to an OS or desktop labels May 1, 2023
@michaelrsweet michaelrsweet added this to the v2.4.3 milestone May 1, 2023
@michaelrsweet michaelrsweet merged commit bae56da into OpenPrinting:master May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request platform issue Issue is specific to an OS or desktop priority-low

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants