Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove fcf-protection=full #267

Closed
wants to merge 1 commit into from

Conversation

watologo1
Copy link

@watologo1 watologo1 commented Mar 20, 2024

This option causes grief with at least some gcc versions:

built failure ->"cc: error: unrecognized command line option '-fcf- protection=full'; did you mean '-fstack-protector-all'?"

and some archs (i586):

cc1: error: ‘-fcf-protection’ is not compatible with this target

According to our gcc maintainer (translated):
Whatever the reason is that they require gcc version 9..., maybe -fcf-protection, but simply throw it out...

If there is no urgent reason, better get rid of this one.

Description

Affected parts

  • library
  • pqos utility
  • rdtset utility
  • App QoS
  • other: (please specify)

Motivation and Context

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

This option causes grief with at least some gcc versions:

built failure ->"cc: error: unrecognized command line option '-fcf-
protection=full'; did you mean '-fstack-protector-all'?"

and some archs (i586):

 cc1: error: ‘-fcf-protection’ is not compatible with this target

According to our gcc maintainer (translated):
Whatever the reason is that they require gcc version 9..., maybe  -fcf-protection,
but simply throw it out...

If there is no urgent reason, better get rid of this one.

Signed-off-by: Thomas Renninger <trenn@suse.de>
@watologo1
Copy link
Author

I additionally removed the gcc/clang check which at least has been put in pre-build.mk (I removed the including in Makefiles) and finally intel-cmt-cat compiles again on nearly all distros:
https://build.opensuse.org/package/show/home:trenn:branches:server:monitoring/intel-cmt-cat

This is what I'd like to submit next. I followed history of the gcc/clang check, but unfortunately there is no detailed comment why this has been added.

@rstorozh
Copy link
Contributor

The additional checks were introduced in order to increase product security using some flags supported only by relatively new compiler versions. You could read my answer related to the same situation:
#258

Please upgrade your compiler versions.

@rstorozh rstorozh closed this Mar 21, 2024
@watologo1
Copy link
Author

The option is rather unsatisfying.
Are there known issues what would happen without passing: -fcf- protection=full ?
The flag was added recently.
Depending on the build environment it should be up to the developer to add such options from outside via CFLAGS=
E.g. OpenSUSE builds 32 bit via march=i586 which does not support -fcf- protection=full.
Also most build environments seem to not support this flag yet, but the tool would still be fully functioning.
If it's ok, I'll try to come up with defining this more central. Having such a patch splitted over all Makefiles is ugly to maintain.
Ideally, it's still ripped out and the developer (build environment) decides whether to add this flag depending whether his compiler supports it or not.
Then also the "not very nice" check for gcc version could vanish as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants