SDL signoff requirements - please enable additional C/C++ compiler warnings

[GrabYourPitchforks]

Please prioritize this work. Depending on the size of the code base, there could be hundreds or thousands of warnings which require addressing.

This work must be completed before RC1 snap.

Recent SDL changes require that we enable additional compiler warnings across our C/C++ projects. The requirements are:

• Compile with warning level 3 or higher. These warnings do not have to be fixed, but they must at least be triaged.
• Enable the following additional warnings, which must be fixed if they occur:
  • C4018 - 'expression' : signed/unsigned mismatch
  • C4055 - 'conversion' : from data pointer 'type1' to function pointer 'type2'
  • C4146 - unary minus operator applied to unsigned type, result still unsigned
  • C4242 - 'identifier' : conversion from 'type1' to 'type2', possible loss of data
  • C4244 - 'conversion' conversion from 'type1' to 'type2', possible loss of data
  • C4267 - 'var' : conversion from 'size_t' to 'type', possible loss of data
  • C4302 - 'conversion' : truncation from 'type 1' to 'type 2'
  • C4308 - negative integral constant converted to unsigned type
  • C4509 - nonstandard extension used: 'function' uses SEH and 'object' has destructor
  • C4510 - 'class' : default constructor could not be generated
  • C4532 - 'continue' : jump out of __finally/finally block has undefined behavior during termination handling
  • C4533 - initialization of 'variable' is skipped by 'instruction'
  • C4610 - object 'class' can never be instantiated - user-defined constructor required
  • C4611 - interaction between 'function' and C++ object destruction is non-portable
  • C4700 - uninitialized local variable 'name' used
  • C4701 - Potentially uninitialized local variable 'name' used
  • C4703 - Potentially uninitialized local pointer variable 'name' used
  • C4789 - destination of memory copy is too small
  • C4995 - 'function': name was marked as #pragma deprecated
  • C4996 - 'function': was declared deprecated also 'std::': Function call with parameters that are potentially unsafe - this call relies on the caller to check that the passed values are correct. To disable this warning, use -D_SCL_SECURE_NO_WARNINGS. See documentation on how to use Visual C++ 'Checked Iterators'

See the SDL guidelines (MSFT internal only) for further information.

To make auditing code easier prior to signoff and release, please include the string Microsoft.Security.SystemsADM.10086 somewhere in the vcxproj / cmake / whatever file which is responsible for C/C++ compilation where all of these flags are defined. That gives the code auditors a target string to search against and validate that the proper warnings are enabled. See below an example of how to do this in a cmake file.

# [[! Microsoft.Security.SystemsADM.10086 !]] - SQL required warnings
add_compile_options($<$<COMPILE_LANGUAGE:C,CXX>:/W3>) # warning level 3
add_compile_options($<$<COMPILE_LANGUAGE:C,CXX>:/WX>) # treat warnings as errors
add_compile_options($<$<COMPILE_LANGUAGE:C,CXX>:/we4018>) # 'expression' : signed/unsigned mismatch
add_compile_options($<$<COMPILE_LANGUAGE:C,CXX>:/we4055>) # 'conversion' : from data pointer 'type1' to function pointer 'type2'
# ... and so on ...

Once the work is finished, or if there is no work to perform, please feel free to close this issue.

Thanks for your assistance!

Quick FAQ

What code is bound to this requirement?

This affects only production code. Production code is generally defined as code which ships as part of the product and which runs on customer machines or which manages infrastructure, such as our build labs. Unit and functional test projects are not considered production code.

Does this need to be backported?

No backporting plans at this time. If actual bugs are found during this process, individual product teams have discretion to selectively backport into the next downlevel servicing vehicle.

What about forked third-party code?

This requirement applies to all code that MSFT builds from source, regardless of its provenance. Ideally any changes that we make to local forked copies can be submitted upstream as a PR so that the wider ecosystem can enjoy their benefits.

If this is impractical, exceptions to this requirement can be sought on an as-needed basis. However, exceptions are: (a) not guaranteed to be granted; and (b) time-constrained. The exception process is not intended to provide a permanent deferral of this work. Please contact the fxsecurity alias if an exception is needed.

What about C# and other languages?

This requirement only affects C/C++ code. Requirements for other languages will be filed as separate issues.

[GrabYourPitchforks]

I see that there was some previous work done as part of https://github.com/dotnet/diagnostics/pull/1047. What would be helpful here is to confirm that the work done there is still in force. Proper warning level set, that all required warn-as-error rules (listed above) are set, and that the audit string is present in the build system so that this can be independently verified via code review later.

Thanks!