Add strongly recommended security compiler flags #660
Conversation
CMakeLists.txt
Outdated
| @@ -50,11 +50,11 @@ endif() | |||
|
|
|||
| # CXX flags setup | |||
| if(NOT MSVC) | |||
| add_compile_options(-fPIC -Wall -Wpedantic | |||
| add_compile_options(-fPIC -Wall -Wpedantic -D_FORTIFY_SOURCE=2 | |||
There was a problem hiding this comment.
We could try adding -D_FORTIFY_SOURCE=3 which adds additional runtime checks (supposedly with little to no performance impact).
In general, anything above 1 can break otherwise correct programs, so this might introduce a regression in SYCL...
There was a problem hiding this comment.
Should I add it under the UR_DEVELOPER_MODE flag?
There was a problem hiding this comment.
No, it's fine. This just might be a little bit more work than it appears since we don't have comprehensive regression testing :)
There was a problem hiding this comment.
For now let's change it to -D_FORTIFY_SOURCE=1, and we will revisit this once we have an e2e CI on UR end.
There was a problem hiding this comment.
For safety reasons, we should stay at level 2. I have added the required flags in this PR, but the PR may not build, as I am waiting for the @PatKamin PR with the flags wrapper.
wlemkows
force-pushed
the
add-compiler-flags
branch
from
0ad226e
to
1148f1d
Compare
wlemkows
force-pushed
the
add-compiler-flags
branch
2 times, most recently
from
04aa628
to
64a847b
Compare
| @@ -57,8 +57,11 @@ if(NOT MSVC) | |||
| add_compile_options(-fPIC -Wall -Wpedantic | |||
| $<$<CXX_COMPILER_ID:GNU>:-fdiagnostics-color=always> | |||
| $<$<CXX_COMPILER_ID:Clang,AppleClang>:-fcolor-diagnostics>) | |||
| if (CMAKE_BUILD_TYPE STREQUAL "Release") | |||
| add_compile_options(-D_FORTIFY_SOURCE=2) | |||
There was a problem hiding this comment.
Please, verify if optimization level of 1 and above has to be set for this macro to work as expected.
There was a problem hiding this comment.
FORTIFY_SOURCE requires O>0, we use O3 for the Release version.
No description provided.