[clang-cl] Make /Gs imply default stack probes, not /Gs0 (PR39074)

[hans]

Summary

Also remove text saying /O1 and /O2 implies /Gs. They don't seem to do that, at least with modern MSVC (see bug).

[majnemer]

FWIW, Microsoft's newest documentation does not say that /O2 and /O1 imply /Gs: https://docs.microsoft.com/en-us/cpp/build/reference/o1-o2-minimize-size-maximize-speed?view=vs-2017

[hans]

In D52499#1245474, @majnemer wrote:

FWIW, Microsoft's newest documentation does not say that /O2 and /O1 imply /Gs: https://docs.microsoft.com/en-us/cpp/build/reference/o1-o2-minimize-size-maximize-speed?view=vs-2017

Great, thanks for finding that!

[thakis]

Thanks! lgtm.

include/clang/Driver/CLCompatOptions.td

94

https://docs.microsoft.com/en-us/cpp/build/reference/gs-control-stack-checking-calls?view=vs-2017 still claims that /Gs is /Gs0 though. Is that just wrong? https://bugs.llvm.org/show_bug.cgi?id=39074#c2 suggests it is. Should we ask bruce to file a doc bug?

And since this flag is supposed to get the default behavior, should it be a CLIgnoredFlag instead of duplicating the 4096 somewhat redundantly?

[hans]

include/clang/Driver/CLCompatOptions.td

94

I've submitted feedback on the document page: https://github.com/MicrosoftDocs/cpp-docs/issues/445

By not ignoring it, we enable using /Gs to override a previous e.g. /Gs0 flag, which I think is the correct thing to do.

[thakis]

Sounds great, thanks!

include/clang/Driver/CLCompatOptions.td

def _SLASH_GA : CLFlag<"GA">, Alias<ftlsmodel_EQ>, AliasArgs<["local-exec"]>,
   HelpText<"Assume thread-local variables are defined in the executable">;
 def _SLASH_GR : CLFlag<"GR">, HelpText<"Enable emission of RTTI data">;
 def _SLASH_GR_ : CLFlag<"GR-">, HelpText<"Disable emission of RTTI data">;
 def _SLASH_GF : CLIgnoredFlag<"GF">, HelpText<"Enable string pooling (default)">;
 def _SLASH_GF_ : CLFlag<"GF-">, HelpText<"Disable string pooling">,
   Alias<fwritable_strings>;
 def _SLASH_GS : CLFlag<"GS">, HelpText<"Enable buffer security check (default)">;
 def _SLASH_GS_ : CLFlag<"GS-">, HelpText<"Disable buffer security check">;
-// FIXME: Not sure /Gs really means /Gs0 (see PR39074).
-def : CLFlag<"Gs">, HelpText<"Same as /Gs0">, Alias<mstack_probe_size>, AliasArgs<["0"]>;
+def : CLFlag<"Gs">, HelpText<"Use stack probes (default)">,
+  Alias<mstack_probe_size>, AliasArgs<["4096"]>;
 def _SLASH_Gs : CLJoined<"Gs">,
   HelpText<"Set stack probe size (default 4096)">, Alias<mstack_probe_size>;
 def _SLASH_Gy : CLFlag<"Gy">, HelpText<"Put each function in its own section">,
   Alias<ffunction_sections>;
 def _SLASH_Gy_ : CLFlag<"Gy-">,
   HelpText<"Don't put each function in its own section (default)">,
   Alias<fno_function_sections>;
 def _SLASH_Gw : CLFlag<"Gw">, HelpText<"Put each data item in its own section">,

 // The _SLASH_O option handles all the /O flags, but we also provide separate
 // aliased options to provide separate help messages.
 def _SLASH_O : CLJoined<"O">,
   HelpText<"Set multiple /O flags at once; e.g. '/O2y-' is the same as '/O2 /y-'">,
   MetaVarName<"<flags>">;
 // FIXME: Not sure why we have -O0 here; MSVC doesn't support that.
 def : CLFlag<"O0">, Alias<O0>, HelpText<"Disable optimization">;
 def : CLFlag<"O1">, Alias<_SLASH_O>, AliasArgs<["1"]>,
-  HelpText<"Optimize for size (equivalent to /Og /Os /Oy /Ob2 /Gs /GF /Gy)">;
+  HelpText<"Optimize for size (equivalent to /Og /Os /Oy /Ob2 /GF /Gy)">;
 def : CLFlag<"O2">, Alias<_SLASH_O>, AliasArgs<["2"]>,
-  HelpText<"Optimize for speed (equivalent to /Og /Oi /Ot /Oy /Ob2 /Gs /GF /Gy)">;
+  HelpText<"Optimize for speed (equivalent to /Og /Oi /Ot /Oy /Ob2 /GF /Gy)">;
 def : CLFlag<"Ob0">, Alias<_SLASH_O>, AliasArgs<["b0"]>,
   HelpText<"Disable function inlining">;
 def : CLFlag<"Ob1">, Alias<_SLASH_O>, AliasArgs<["b1"]>,
   HelpText<"Only inline functions which are (explicitly or implicitly) marked inline">;
 def : CLFlag<"Ob2">, Alias<_SLASH_O>, AliasArgs<["b2"]>,
   HelpText<"Inline functions as deemed beneficial by the compiler">;
 def : CLFlag<"Od">, Alias<_SLASH_O>, AliasArgs<["d"]>,
   HelpText<"Disable optimization">;

test/Driver/cl-options.c

 
 // RUN: %clang_cl /Gy -### -- %s 2>&1 | FileCheck -check-prefix=Gy %s
 // Gy: -ffunction-sections
 
 // RUN: %clang_cl /Gy /Gy- -### -- %s 2>&1 | FileCheck -check-prefix=Gy_ %s
 // Gy_-NOT: -ffunction-sections
 
 // RUN: %clang_cl /Gs -### -- %s 2>&1 | FileCheck -check-prefix=Gs %s
-// Gs: "-mstack-probe-size=0"
+// Gs: "-mstack-probe-size=4096"
 // RUN: %clang_cl /Gs0 -### -- %s 2>&1 | FileCheck -check-prefix=Gs0 %s
 // Gs0: "-mstack-probe-size=0"
 // RUN: %clang_cl /Gs4096 -### -- %s 2>&1 | FileCheck -check-prefix=Gs4096 %s
 // Gs4096: "-mstack-probe-size=4096"
 
 // RUN: %clang_cl /Gw -### -- %s 2>&1 | FileCheck -check-prefix=Gw %s
 // Gw: -fdata-sections