[SYCL][CMAKE] Switch back to _FORTIFY_SOURCE=2 #19268
Conversation
The problem here is that the macro is actually handled by glibc and value `3` isn't supported with older compiler/glibc combinations, causing warnings about the macro redefinition. We still have to support older compilers/glibc and therefore as a temporary solution, reverting `_FORTIFY_SOURCE` back to `2` to avoid build issues with `-Werror`.
AlexeySachkov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
| @@ -167,18 +167,18 @@ macro(append_common_extra_security_flags) | |||
| if(LLVM_ON_UNIX) | |||
| # Fortify Source (strongly recommended): | |||
| if(CMAKE_BUILD_TYPE STREQUAL "Debug") | |||
| message(WARNING "-D_FORTIFY_SOURCE=3 can only be used with optimization.") | |||
| message(WARNING "-D_FORTIFY_SOURCE=3 is not supported.") | |||
| message(WARNING "-D_FORTIFY_SOURCE=2 can only be used with optimization.") | |||
There was a problem hiding this comment.
Is there a way we can detect what compiler we can use 3 on and use that and use 2 otherwise?
There was a problem hiding this comment.
Is there a way we can detect what compiler we can use
3on and use that and use2otherwise?
Theoretically, but it is not clear which exact versions are "good" and which ones are "bad". It seems like gcc 12 is the first "good" (link), but I'm not sure about clang
There was a problem hiding this comment.
The doc says There has been compiler support for this builtin in [Clang](https://clang.llvm.org/) for some time so maybe we can just ignore it/check for some really old version?
If 3 has some benefit IMO we should try to add a check to see if we can use it if it's not too complicated
There was a problem hiding this comment.
075da8c introduces the check. I also had to uplift _FORTIFY_SOURCE in the Unified Runtime to avoid macro redefinition. We set it globally, but UR also sets it locally, because it is intended to buildable standalone.
I tried to uplift _FORTIFY_SOURCE in UR in #19236 and this is how I detected the issue with "bad" gcc, so we will see now if the suggested check works.
AlexeySachkov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
AlexeySachkov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
AlexeySachkov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
AlexeySachkov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
AlexeySachkov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
AlexeySachkov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
AlexeySachkov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
AlexeySachkov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
The problem here is that the macro is actually handled by glibc and value
3isn't supported with older compiler/glibc combinations, causing warnings about the macro redefinition.We still have to support older compilers/glibc and therefore as a temporary solution, reverting
_FORTIFY_SOURCEback to2to avoid build issues with-Werror.