- 
                Notifications
    You must be signed in to change notification settings 
- Fork 5.2k
[release/8.0-staging] Add flags when the clang's major version is > 20.0 #121150
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
base: release/8.0-staging
Are you sure you want to change the base?
[release/8.0-staging] Add flags when the clang's major version is > 20.0 #121150
Conversation
| Tagging subscribers to this area: @hoyosjs | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates compiler flag configuration for Unix-based builds to use -fno-strict-overflow instead of -fwrapv when using Clang 20.0 or later, and adds -fno-strict-aliasing for Clang 20+.
Key Changes
- Adds version-based conditional logic to use different overflow handling flags based on Clang version
- Introduces -fno-strict-overflowflag for Clang 20+ (which implies-fwrapvand-fwrapv-pointer)
- Adds -fno-strict-aliasingflag for Clang 20+ to match MSVC behavior
| if((CMAKE_C_COMPILER_ID STREQUAL "Clang" AND CMAKE_C_COMPILER_VERSION VERSION_GREATER_EQUAL 20.0) OR | ||
| (CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 20.0)) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any concern here about Clang on macOS? If so, this should use MATCHES instead of STREQUAL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we care about macOS for these changes. Is there doc on why to use MATCHES vs STREQUAL in this case? Is it "clang" on macOS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On macOS, it's AppleClang instead of Clang (as Apple has their forked build with slight differences).
Customer Impact
These issues were reported in #119706 as problems with clang-21 on Fedora 43. The investigation uncovered that clang introduced a potentially breaking change in clang-20 that we do not currently consume. These build changes impact VMR related builds when linux distrobutions performing source build adopt clang-21.
clang-20 breaking change log - https://releases.llvm.org/20.1.0/tools/clang/docs/ReleaseNotes.html#potentially-breaking-changes.
This PR contains the minimal changes needed to fix issues from the following PR #120775.
.NET 10: #121124
.NET 9: #121151
Regression
Build with the new clang-21 compiler will cause the runtime to crash.
Testing
This has been validated using various legs and examples to demonstrate the usage of undefined behavior these flags convert into "defined" behavior in C/C++.
Risk
Low. This has zero impact on our production build since we specifically target clang-16. This is only valid for those partners that are using clang-20+.