Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 10, 2025

Add /sdl compiler flag for SDL compliance

This PR adds the /sdl compiler flag to enable SDL (Security Development Lifecycle) checks for all MSVC builds and removes redundant flags that are now subsumed by /sdl.

Summary

  • Revert individual SDL linker switches from CMakeLists.txt files
  • Add /sdl flag to central MSVC configuration in eng/native/configurecompiler.cmake
  • Remove redundant /GS flag (subsumed by /sdl)
  • Remove redundant /we warning options (subsumed by /sdl)

Changes

Added the /sdl compiler flag globally in eng/native/configurecompiler.cmake for all MSVC C/CXX compilations, and removed redundant flags:

  • Removed /GS flag (buffer security checks) - included in /sdl
  • Removed 19 /we warning options (SDL-required warnings as errors) - included in /sdl

Implementation

add_compile_options($<$<COMPILE_LANGUAGE:C,CXX>:/sdl>) # enable additional security checks

The /sdl flag:

  • Enables additional security checks at compile time
  • Promotes certain SDL-required warnings to errors (C4055, C4146, C4242, C4244, C4267, C4302, C4308, C4509, C4510, C4532, C4533, C4610, C4611, C4700, C4701, C4703, C4789, C4995, C4996)
  • Includes /GS (buffer security checks) functionality
  • Enables additional runtime checks
  • Is applied to all MSVC builds from a central location

Net Changes

  • Added: 1 line (/sdl flag)
  • Removed: 24 lines (1 /GS flag + 19 /we options + 3 comment lines + 1 empty line)

This simplifies the configuration while maintaining full SDL compliance through the comprehensive /sdl flag.

Why /sdl instead of individual switches?

Individual switches like /DYNAMICBASE, /NXCOMPAT, and /HIGHENTROPYVA are already enabled by default in modern MSVC toolchains. The /sdl compiler flag provides comprehensive SDL compliance by enabling additional compile-time checks and warnings, eliminating the need for explicit /GS and numerous /we options.

Note: /GUARD:CF (Control Flow Guard) is already applied globally via configurecompiler.cmake.

Fixes #121509

Original prompt

This section details on the original issue you should resolve

<issue_title>Missing SDL switch for some native binaries</issue_title>
<issue_description>- [ ] System.Globalization.Native

  • System.IO.Compression.Native</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…Compression.Native

Co-authored-by: agocke <515774+agocke@users.noreply.github.com>
Copilot AI changed the title [WIP] Add SDL switch for native binaries Add SDL linker switches to System.Globalization.Native and System.IO.Compression.Native Nov 10, 2025
Copilot AI requested a review from agocke November 10, 2025 21:54
Copilot finished work on behalf of agocke November 10, 2025 21:54
Removed individual SDL linker switches from CMakeLists.txt files and instead added the /sdl compiler flag globally in eng/native/configurecompiler.cmake. This approach is cleaner and applies the SDL checks to all MSVC builds from a central location.

Co-authored-by: agocke <515774+agocke@users.noreply.github.com>
Copilot AI changed the title Add SDL linker switches to System.Globalization.Native and System.IO.Compression.Native Add /sdl compiler flag for SDL compliance Nov 10, 2025
Copilot finished work on behalf of agocke November 10, 2025 23:56
The /sdl flag is a superset of /GS, so the explicit /GS flag is redundant and can be removed.

Co-authored-by: agocke <515774+agocke@users.noreply.github.com>
Copilot finished work on behalf of agocke November 11, 2025 00:28
Copilot AI requested a review from agocke November 11, 2025 00:28
@jkotas
Copy link
Member

jkotas commented Nov 11, 2025

@copilot Delete /we options that are redundant with /sdl from eng\native\configurecompiler.cmake

@agocke agocke marked this pull request as ready for review November 11, 2025 00:52
Copilot AI review requested due to automatic review settings November 11, 2025 00:52
The /sdl flag automatically enables SDL-required warnings as errors (C4055, C4146, C4242, C4244, C4267, C4302, C4308, C4509, C4510, C4532, C4533, C4610, C4611, C4700, C4701, C4703, C4789, C4995, C4996), so these explicit /we options are no longer needed.

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 11, 2025

@copilot Delete /we options that are redundant with /sdl from eng\native\configurecompiler.cmake

Removed in commit e3b098d. The /sdl flag automatically enables these 19 SDL-required warnings as errors, so the explicit /we options are no longer needed.

Copilot finished reviewing on behalf of agocke November 11, 2025 00:53
Copy link
Contributor

Copilot AI left a 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 adds the /sdl compiler flag to enable comprehensive Security Development Lifecycle (SDL) checks for all MSVC builds. The /sdl flag provides a superset of security features that includes buffer security checks (/GS), making the explicit /GS flag redundant.

  • Added /sdl flag globally in eng/native/configurecompiler.cmake for all MSVC C/CXX compilations
  • Removed the now-redundant /GS flag since /sdl already includes buffer security checks

Copilot AI requested a review from jkotas November 11, 2025 00:56
Copilot finished work on behalf of jkotas November 11, 2025 00:56
@jkotas
Copy link
Member

jkotas commented Nov 11, 2025

/ba-g android timeouts

@jkotas jkotas merged commit e125fc4 into main Nov 11, 2025
128 of 152 checks passed
@jkotas jkotas deleted the copilot/add-sdl-switch-native-binaries branch November 11, 2025 05:58
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.

Missing SDL switch for some native binaries

4 participants