The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/de51a84b69b5b1dd458c65f6aaf2335f81b57785

commit de51a84b69b5b1dd458c65f6aaf2335f81b57785
Author: thakis <thakis@chromium.org>
Date: Wed Mar 30 12:30:11 2016

clang/win: Stop passing -Qunused-arguments.

All flags except /GS and /GL are accepted by clang-cl by now. To be able
to turn on -Qunused-arguments, don't pass /GS and /GL in clang builds
for now.

BUG= 504658 ,598772, 598767 
NOTRY=true

Review URL: https://codereview.chromium.org/1828543003

Cr-Commit-Position: refs/heads/master@{#383954}

[modify] https://crrev.com/de51a84b69b5b1dd458c65f6aaf2335f81b57785/build/common.gypi
[modify] https://crrev.com/de51a84b69b5b1dd458c65f6aaf2335f81b57785/build/config/compiler/BUILD.gn
[modify] https://crrev.com/de51a84b69b5b1dd458c65f6aaf2335f81b57785/build/config/win/BUILD.gn

The official documentation:

https://msdn.microsoft.com/en-us/library/8dbf701c.aspx

Note that this is enabled by default cl.

Here's a demo:

C:\src\llvm-build>type gs.cc
void sprintf(char*, ...);
char f(char* p) {
  char buf[10];
  sprintf(buf, "%s", p);
  return buf[0];
}

C:\src\llvm-build>cl /FA gs.cc /c /O2 /GS- && type gs.asm

Now look at gs.asm and compare to what happens if you run the same with /GS (no -) or if you omit /GS altogether (it's on by default).

This is added in the prologue:

	mov	rax, QWORD PTR __security_cookie
	xor	rax, rsp
	mov	QWORD PTR __$ArrayPad$[rsp], rax

And this in the epilogue:

	mov	rcx, QWORD PTR __$ArrayPad$[rsp]
	xor	rcx, rsp
	call	__security_check_cookie

clang's existing -fstack-protector (on by default, at least on mac) mostly does this:

$ clang -O2 -c gs.cc -S -o - -fno-stack-protector > gsoff.asm
$ clang -O2 -c gs.cc -S -o - -fstack-protector > gson.asm

Diffing these shows that the prologue grows

	movq	___stack_chk_guard@GOTPCREL(%rip), %rbx
	movq	(%rbx), %rbx
	movq	%rbx, -16(%rbp)

and the epilogue grows

	movsbl	-26(%rbp), %eax
	cmpq	-16(%rbp), %rbx
	jne	LBB0_2
...
LBB0_2:
	callq	___stack_chk_fail


So it's a bit different, but some of the machinery for that can probably be used for /GS.

More docs (suggesting that this is only used in SEH-using functions even with /GS-):
https://msdn.microsoft.com/en-us/library/ms235362.aspx
https://msdn.microsoft.com/en-us/library/ms235603.aspx

This is a good overview article: https://msdn.microsoft.com/en-us/library/aa290051(VS.71).aspx

Any luck?

This is in progress at http://reviews.llvm.org/D20346 (and 
http://reviews.llvm.org/D20347)

Thx Nico. I forgot to update this thread.


Here are some metrics for a chromium build on Windows with/without stack guard activated.

The first set of column is when the /GS is off by default.
The second set of column is when the /GS is on by default.

Relocs are the number of relocation to security_cookie.

                          Without /GS               With  /GS                Delta          
                  ----------------------     ----------------------    ----------------------
                           Size   Relocs           Size       Relocs          Size
                          (bytes)                (bytes)                    (bytes)
chrome.exe             1,081,344    206         1,107,456     1,128           26,112    2.41%
chrome.dll            47,528,960    228        49,414,144    65,208        1,885,184    3.97%
chrome_child.dll      57,482,752    242        59,433,984    72,722        1,951,232    3.39%

Important points:
  * The heuristic determining if a function must be checked against stack smashing is much more aggressive in LLVM than MSVC, which cause a cost in executable size.
    * This *may* be addressed by implementing an other heuristic in LLVM.
  * Current cookie computation is only a comparison. This needs to be extended to XORing important values (ebp, EH) as it's done on recent MSVC compiler.
  * There is two different versions of the exception handler used (version 3 and version 4) [for 32-bits executable]. Version 4 has additional fields that needs to be updated by the compiler when /GS is on. The exception handler is performing additional checks on EH and GS cookie in version 4.

Missing features:
  * https://msdn.microsoft.com/en-us/library/bb507721.aspx
  * https://msdn.microsoft.com/en-us/library/dd778695.aspx

Thanks! Do you have an idea how much this would change if SSPOn was used instead of SSPStrong? (I don't think we want to do this, but it'd be interesting data)

I made progress on this feature.

On 32-bits, when stack-protector and exceptions are used, the handler is upgraded to version 4 (__except_handler4).
Unfortunately, many features a not yet working in clang.

Work in progress:
[exceptions] Upgrade exception handlers when stack protector is used
http://reviews.llvm.org/D21101

[CodeGen] Fix PrologEpilogInserter to avoid duplicate allocation of SEH structs
http://reviews.llvm.org/D21188

Add support to clang-cl driver for /GS switch
http://reviews.llvm.org/D20347

More patches are on-going...

I also need to check 64-bits.

A step toward making SEH4 works in 32-bits.

[StackProtector] Fix computation of GSCookieOffset and EHCookieOffset with SEH4
http://reviews.llvm.org/D21231

http://llvm.org/viewvc/llvm-project?view=revision&revision=272832 turned this on, but the cookie xoring isn't implemented yet -- so we'll get two binary bumps from this. One now, one once xor'ing is done (Hans, FYI)

The non-xoring version is now rolled in and turned on in our pinned clang-cl.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cc683aecb8e88d03139477b8eba81aeb1e3c2e9c

commit cc683aecb8e88d03139477b8eba81aeb1e3c2e9c
Author: thakis <thakis@chromium.org>
Date: Fri Jul 08 16:47:11 2016

clang/win: It's ok to pass /GS to clang-cl now.

We used to not pass /GS since clang-cl used to not implement buffer security
checks and would warn about this flag being ignored.  Now clang-cl implements
some form of buffer security checking and no longer warns on /GS, so that's
no longer necessary. This reverts the gyp bits of
https://codereview.chromium.org/1828543003/

/GS is also on by default, and as of https://codereview.chromium.org/1957523005
it's no longer explicitly passed in gn builds -- so no .gn files need to be
changed.

Since /GS is on by default, this CL doesn't change behavior.

BUG= 598767 

Review-Url: https://codereview.chromium.org/2129213002
Cr-Commit-Position: refs/heads/master@{#404413}

[modify] https://crrev.com/cc683aecb8e88d03139477b8eba81aeb1e3c2e9c/build/common.gypi

etienneb, are you looking at cookie xoring? That's the last missing piece here, I think.

I think we can declare this done.