Re: [PATCH] D20347: Add support to clang-cl driver for /GS switch

[Reid Kleckner via cfe-commits]

rnk accepted this revision.
rnk added a reviewer: rnk.
rnk added a comment.

lgtm

Surely this is going to break something, but let's throw the switch and find 
out.


http://reviews.llvm.org/D20347



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20347: Add support to clang-cl driver for /GS switch

[Nico Weber via cfe-commits]

thakis added a comment.

probably at least the "the XOR with RSP/EBP/ESP" bit still (and maybe EH 
function upgrades instead of bailing)



Comment at: lib/Driver/Tools.cpp:9990
@@ +9989,3 @@
+   /*default=*/false))
+CmdArgs.push_back("/GS-");
+

To pass on /GS- to the fallback compiler when needed, I suppose.


http://reviews.llvm.org/D20347



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20347: Add support to clang-cl driver for /GS switch

[Hans Wennborg via cfe-commits]

hans added a comment.

Is this waiting for anything more now that http://reviews.llvm.org/D20346 has 
landed?


http://reviews.llvm.org/D20347



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20347: Add support to clang-cl driver for /GS switch

[Etienne Bergeron via cfe-commits]

etienneb added a comment.

This patch needs land after http://reviews.llvm.org/D20346.
thx for the review.


http://reviews.llvm.org/D20347



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20347: Add support to clang-cl driver for /GS switch

[Etienne Bergeron via cfe-commits]

etienneb updated this revision to Diff 58502.
etienneb marked an inline comment as done.
etienneb added a comment.

more tests


http://reviews.llvm.org/D20347

Files:
  include/clang/Driver/CLCompatOptions.td
  lib/Driver/Tools.cpp
  test/Driver/cl-fallback.c
  test/Driver/cl-options.c

Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -59,6 +59,16 @@
 // RUN: %clang_cl /GR- -### -- %s 2>&1 | FileCheck -check-prefix=GR_ %s
 // GR_: -fno-rtti
 
+// Security Buffer Check is on by default.
+// RUN: %clang_cl -### -- %s 2>&1 | FileCheck -check-prefix=GS-default %s
+// GS-default: "-stack-protector" "2"
+
+// RUN: %clang_cl /GS -### -- %s 2>&1 | FileCheck -check-prefix=GS %s
+// GS: "-stack-protector" "2"
+
+// RUN: %clang_cl /GS- -### -- %s 2>&1 | FileCheck -check-prefix=GS_ %s
+// GS_-NOT: -stack-protector
+
 // RUN: %clang_cl /Gy -### -- %s 2>&1 | FileCheck -check-prefix=Gy %s
 // Gy: -ffunction-sections
 
Index: test/Driver/cl-fallback.c
===
--- test/Driver/cl-fallback.c
+++ test/Driver/cl-fallback.c
@@ -1,7 +1,7 @@
 // Note: %s must be preceded by --, otherwise it may be interpreted as a
 // command-line option, e.g. on Mac where %s is commonly under /Users.
 
-// RUN: %clang_cl --target=i686-pc-win32 /fallback /Dfoo=bar /Ubaz /Ifoo /O0 /Ox /GR /GR- /Gy /Gy- \
+// RUN: %clang_cl --target=i686-pc-win32 /fallback /Dfoo=bar /Ubaz /Ifoo /O0 /Ox /GR /GR- /GS /GS- /Gy /Gy- \
 // RUN:   /Gw /Gw- /LD /LDd /EHs /EHs- /Zl /MD /MDd /MTd /MT /FImyheader.h /Zi \
 // RUN:   -garbage -moregarbage \
 // RUN:   -### -- %s 2>&1 \
@@ -22,6 +22,7 @@
 // CHECK: "/Oy"
 // CHECK: "/GF"
 // CHECK: "/GR-"
+// CHECK: "/GS-"
 // CHECK: "/Gy-"
 // CHECK: "/Gw-"
 // CHECK: "/Z7"
@@ -41,6 +42,10 @@
 // GR: cl.exe
 // GR: "/GR-"
 
+// RUN: %clang_cl /fallback /GS- -### -- %s 2>&1 | FileCheck -check-prefix=GS %s
+// GS: cl.exe
+// GS: "/GS-"
+
 // RUN: %clang_cl /fallback /Od -### -- %s 2>&1 | FileCheck -check-prefix=O0 %s
 // O0: cl.exe
 // O0: "/Od"
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -6146,6 +6146,14 @@
/*default=*/false))
 CmdArgs.push_back("-fno-rtti-data");
 
+  // This controls whether or not we emit stack-protector instrumentation.
+  // In MSVC, Buffer Security Check (/GS) is on by default.
+  if (Args.hasFlag(options::OPT__SLASH_GS, options::OPT__SLASH_GS_,
+   /*default=*/true)) {
+CmdArgs.push_back("-stack-protector");
+CmdArgs.push_back(Args.MakeArgString(Twine(LangOptions::SSPStrong)));
+  }
+
   // Emit CodeView if -Z7 is present.
   *EmitCodeView = Args.hasArg(options::OPT__SLASH_Z7);
   if (*EmitCodeView)
@@ -9976,6 +9984,11 @@
   if (Args.hasFlag(options::OPT__SLASH_GR_, options::OPT__SLASH_GR,
/*default=*/false))
 CmdArgs.push_back("/GR-");
+
+  if (Args.hasFlag(options::OPT__SLASH_GS_, options::OPT__SLASH_GS,
+   /*default=*/false))
+CmdArgs.push_back("/GS-");
+
   if (Arg *A = Args.getLastArg(options::OPT_ffunction_sections,
options::OPT_fno_function_sections))
 CmdArgs.push_back(A->getOption().getID() == options::OPT_ffunction_sections
Index: include/clang/Driver/CLCompatOptions.td
===
--- include/clang/Driver/CLCompatOptions.td
+++ include/clang/Driver/CLCompatOptions.td
@@ -77,6 +77,8 @@
 def _SLASH_GR_ : CLFlag<"GR-">, HelpText<"Disable emission of RTTI data">;
 def _SLASH_GF_ : CLFlag<"GF-">, HelpText<"Disable string pooling">,
   Alias;
+def _SLASH_GS : CLFlag<"GS">, HelpText<"Enable buffer security check">;
+def _SLASH_GS_ : CLFlag<"GS-">, HelpText<"Disable buffer security check">;
 def _SLASH_Gs : CLJoined<"Gs">, HelpText<"Set stack probe size">,
   Alias;
 def _SLASH_Gy : CLFlag<"Gy">, HelpText<"Put each function in its own section">,
@@ -287,7 +289,6 @@
 def _SLASH_FC : CLIgnoredFlag<"FC">;
 def _SLASH_FS : CLIgnoredFlag<"FS">, HelpText<"Force synchronous PDB writes">;
 def _SLASH_GF : CLIgnoredFlag<"GF">;
-def _SLASH_GS_ : CLIgnoredFlag<"GS-">;
 def _SLASH_kernel_ : CLIgnoredFlag<"kernel-">;
 def _SLASH_nologo : CLIgnoredFlag<"nologo">;
 def _SLASH_Ob1 : CLIgnoredFlag<"Ob1">;
@@ -329,7 +330,6 @@
 def _SLASH_GL_ : CLFlag<"GL-">;
 def _SLASH_Gm : CLFlag<"Gm">;
 def _SLASH_Gm_ : CLFlag<"Gm-">;
-def _SLASH_GS : CLFlag<"GS">;
 def _SLASH_GT : CLFlag<"GT">;
 def _SLASH_Guard : CLJoined<"guard:">;
 def _SLASH_GZ : CLFlag<"GZ">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits