[Windows MSVC CFG] Add support for CFG, and enable on chrome.exe & chrome_elf.dll.

[Windows MSVC CFG] Add support for CFG, and enable on chrome.exe & chrome_elf.dll.

CFG supported on x86 and x64 >= Windows 8.1 Update (KB3000850).

Initial support for Control Flow Guard.  "Enable" on chrome.exe to take
advantage of MS system32 dll CFG.  Also compile and link chrome_elf with CFG.

NOTE: As ASLR is disabled in debug builds, so too is CFG.

Using the dumpbin tool from the Visual Studio command prompt, run "dumpbin /headers /loadconfig" against chrome.exe or chrome_elf.dll.  1) "DLL characteristics" now includes "Control Flow Guard", and 2) if code was compiled with cfg there will be "load config" for "Guard CF" function table and flags.

BUG=584575
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Committed: https://crrev.com/d46842301ebc1f41a92448026bf95e3194538213
Cr-Commit-Position: refs/heads/master@{#425226}

[penny, 2016-10-13 20:13:31]

Description was changed from

==========
[Windows MSVC CFG] Add support for CFG, and enable on chrome.exe &
chrome_elf.dll.

Initial support for Control Flow Guard.  "Enable" on chrome.exe to take
advantage of MS system32 dll CFG.  Also compile and link chrome_elf with CFG.

BUG=584575
==========

to

==========
[Windows MSVC CFG] Add support for CFG, and enable on chrome.exe &
chrome_elf.dll.

Initial support for Control Flow Guard.  "Enable" on chrome.exe to take
advantage of MS system32 dll CFG.  Also compile and link chrome_elf with CFG.

BUG=584575
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[penny, 2016-10-13 20:14:53]

The CQ bit was checked by pennymac@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-13 20:15:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[penny, 2016-10-13 20:59:52]

Description was changed from

==========
[Windows MSVC CFG] Add support for CFG, and enable on chrome.exe &
chrome_elf.dll.

Initial support for Control Flow Guard.  "Enable" on chrome.exe to take
advantage of MS system32 dll CFG.  Also compile and link chrome_elf with CFG.

BUG=584575
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
[Windows MSVC CFG] Add support for CFG, and enable on chrome.exe &
chrome_elf.dll.

CFG supported on x86 and x64 >= Windows 8.1 Update (KB3000850).

Initial support for Control Flow Guard.  "Enable" on chrome.exe to take
advantage of MS system32 dll CFG.  Also compile and link chrome_elf with CFG.

BUG=584575
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[penny, 2016-10-13 21:04:11]

pennymac@chromium.org changed reviewers:
+ scottmg@chromium.org

[penny, 2016-10-13 21:05:50]

pennymac@chromium.org changed reviewers:
+ thestig@chromium.org

[penny, 2016-10-13 21:16:50]

Description was changed from

==========
[Windows MSVC CFG] Add support for CFG, and enable on chrome.exe &
chrome_elf.dll.

CFG supported on x86 and x64 >= Windows 8.1 Update (KB3000850).

Initial support for Control Flow Guard.  "Enable" on chrome.exe to take
advantage of MS system32 dll CFG.  Also compile and link chrome_elf with CFG.

BUG=584575
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
[Windows MSVC CFG] Add support for CFG, and enable on chrome.exe &
chrome_elf.dll.

CFG supported on x86 and x64 >= Windows 8.1 Update (KB3000850).

Initial support for Control Flow Guard.  "Enable" on chrome.exe to take
advantage of MS system32 dll CFG.  Also compile and link chrome_elf with CFG.

NOTE: As ASLR is disabled in debug builds, so too is CFG.

Using the dumpbin tool from the Visual Studio command prompt, run "dumpbin
/headers /loadconfig" against chrome.exe or chrome_elf.dll.  1) "DLL
characteristics" now includes "Control Flow Guard", and 2) if code was compiled
with cfg there will be "load config" for "Guard CF" function table and flags.

BUG=584575
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[penny, 2016-10-13 21:26:24]

Hello Scott, Lei,

This is a first pass at adding CFG support to the Windows MSVC official builds.

Scott, please have a look at the config I've added to contain CFG.

Lei, please review how I've included this new config in only chrome.exe (and
chrome_elf.dll).

If we end up turning CFG on for everything, I would just add this to the default
Windows compiler and linker GN configs.  But for now, I'm only opting in
chrome_elf and the chrome.exe.  I would also add a sanity check to
src\tools\checkbins\checkbins.py.  But this would be in a later CL.

Many thanks for your time.  Let me know what you think!

[scottmg, 2016-10-13 21:38:12]

Do you have a measurement of how this affects binary size and/or performance?

https://codereview.chromium.org/2412983006/diff/1/build/config/win/BUILD.gn
File build/config/win/BUILD.gn (right):

https://codereview.chromium.org/2412983006/diff/1/build/config/win/BUILD.gn#n...
build/config/win/BUILD.gn:268: if (visual_studio_version == "2015" && !is_clang)
{
You can drop the version check, 2013 won't work for a lot of other reasons, and
I assume this will be supported for VS 2017 or whatever.

https://codereview.chromium.org/2412983006/diff/1/build/config/win/BUILD.gn#n...
build/config/win/BUILD.gn:275: ldflags = [ "/guard:cf" ]
Doesn't seem much point in having /guard:cf passed to the compiler if it's not
passed to the linker. Just merge into one if?

[scottmg, 2016-10-13 21:40:06]

https://codereview.chromium.org/2412983006/diff/1/build/config/win/BUILD.gn
File build/config/win/BUILD.gn (right):

https://codereview.chromium.org/2412983006/diff/1/build/config/win/BUILD.gn#n...
build/config/win/BUILD.gn:267: config("win_msvc_cfg") {
Also a brief comment here with an MSDN link for those unfamiliar, as "CFG" is a
bit of a generic acronym.

[penny, 2016-10-13 22:08:30]

Thanks Scott.

For this CL, there's very little impact.  Nothing changes in chrome.exe, except
we start to take advantage of CFG coverage built into MS system binaries.  We
don't get that coverage without the exe having cfg enabled.  Nothing is compiled
into our code here yet.

Chrome_elf IS now compiled with safety checks added, but the impact is minimal
for this small DLL.  Once an official canary is built with this CL, I can give
you exact numbers.  I'll also see perf FYI numbers after this lands.  Again,
this first CL will be very small impact.

https://codereview.chromium.org/2412983006/diff/1/build/config/win/BUILD.gn
File build/config/win/BUILD.gn (right):

https://codereview.chromium.org/2412983006/diff/1/build/config/win/BUILD.gn#n...
build/config/win/BUILD.gn:267: config("win_msvc_cfg") {
On 2016/10/13 21:40:06, scottmg wrote:
> Also a brief comment here with an MSDN link for those unfamiliar, as "CFG" is
a
> bit of a generic acronym.

Done.

https://codereview.chromium.org/2412983006/diff/1/build/config/win/BUILD.gn#n...
build/config/win/BUILD.gn:268: if (visual_studio_version == "2015" && !is_clang)
{
On 2016/10/13 21:38:12, scottmg wrote:
> You can drop the version check, 2013 won't work for a lot of other reasons,
and
> I assume this will be supported for VS 2017 or whatever.

Done.

https://codereview.chromium.org/2412983006/diff/1/build/config/win/BUILD.gn#n...
build/config/win/BUILD.gn:275: ldflags = [ "/guard:cf" ]
On 2016/10/13 21:38:12, scottmg wrote:
> Doesn't seem much point in having /guard:cf passed to the compiler if it's not
> passed to the linker. Just merge into one if?

Done.

[scottmg, 2016-10-13 22:21:58]

OK, lgtm. We should make sure to get performance data before turning it on for
more than chrome_elf though.

https://codereview.chromium.org/2412983006/diff/20001/build/config/win/BUILD.gn
File build/config/win/BUILD.gn (right):

https://codereview.chromium.org/2412983006/diff/20001/build/config/win/BUILD....
build/config/win/BUILD.gn:268: #
https://msdn.microsoft.com/en-us/library/windows/desktop/mt637065(v=vs.85).aspx
even nittier; remove the (v=vs8.5) part to make it
"https://msdn.microsoft.com/en-us/library/windows/desktop/mt637065.aspx" so that
it fits in 80col.

https://codereview.chromium.org/2412983006/diff/20001/build/config/win/BUILD....
build/config/win/BUILD.gn:270: # /DYNAMICBASE (ASLR) is turned off in debug
builds,
nit; rewrap this comment.

[Lei Zhang, 2016-10-13 22:28:16]

rs lgtm

[penny, 2016-10-13 23:10:22]

That's 100% part of the plan.

Thanks both!

https://codereview.chromium.org/2412983006/diff/20001/build/config/win/BUILD.gn
File build/config/win/BUILD.gn (right):

https://codereview.chromium.org/2412983006/diff/20001/build/config/win/BUILD....
build/config/win/BUILD.gn:268: #
https://msdn.microsoft.com/en-us/library/windows/desktop/mt637065(v=vs.85).aspx
On 2016/10/13 22:21:57, scottmg wrote:
> even nittier; remove the (v=vs8.5) part to make it
> "https://msdn.microsoft.com/en-us/library/windows/desktop/mt637065.aspx" so
that
> it fits in 80col.

Good one.

https://codereview.chromium.org/2412983006/diff/20001/build/config/win/BUILD....
build/config/win/BUILD.gn:270: # /DYNAMICBASE (ASLR) is turned off in debug
builds,
On 2016/10/13 22:21:57, scottmg wrote:
> nit; rewrap this comment.

Done.

[penny, 2016-10-13 23:10:47]

The CQ bit was checked by pennymac@chromium.org

[penny, 2016-10-13 23:10:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, scottmg@chromium.org
Link to the patchset: https://codereview.chromium.org/2412983006/#ps40001
(title: "Code review nit fixes.")

[commit-bot: I haz the power, 2016-10-13 23:11:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-14 01:14:59]

Description was changed from

==========
[Windows MSVC CFG] Add support for CFG, and enable on chrome.exe &
chrome_elf.dll.

CFG supported on x86 and x64 >= Windows 8.1 Update (KB3000850).

Initial support for Control Flow Guard.  "Enable" on chrome.exe to take
advantage of MS system32 dll CFG.  Also compile and link chrome_elf with CFG.

NOTE: As ASLR is disabled in debug builds, so too is CFG.

Using the dumpbin tool from the Visual Studio command prompt, run "dumpbin
/headers /loadconfig" against chrome.exe or chrome_elf.dll.  1) "DLL
characteristics" now includes "Control Flow Guard", and 2) if code was compiled
with cfg there will be "load config" for "Guard CF" function table and flags.

BUG=584575
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
[Windows MSVC CFG] Add support for CFG, and enable on chrome.exe &
chrome_elf.dll.

CFG supported on x86 and x64 >= Windows 8.1 Update (KB3000850).

Initial support for Control Flow Guard.  "Enable" on chrome.exe to take
advantage of MS system32 dll CFG.  Also compile and link chrome_elf with CFG.

NOTE: As ASLR is disabled in debug builds, so too is CFG.

Using the dumpbin tool from the Visual Studio command prompt, run "dumpbin
/headers /loadconfig" against chrome.exe or chrome_elf.dll.  1) "DLL
characteristics" now includes "Control Flow Guard", and 2) if code was compiled
with cfg there will be "load config" for "Guard CF" function table and flags.

BUG=584575
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[commit-bot: I haz the power, 2016-10-14 01:15:00]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-14 01:17:06]

Description was changed from

==========
[Windows MSVC CFG] Add support for CFG, and enable on chrome.exe &
chrome_elf.dll.

CFG supported on x86 and x64 >= Windows 8.1 Update (KB3000850).

Initial support for Control Flow Guard.  "Enable" on chrome.exe to take
advantage of MS system32 dll CFG.  Also compile and link chrome_elf with CFG.

NOTE: As ASLR is disabled in debug builds, so too is CFG.

Using the dumpbin tool from the Visual Studio command prompt, run "dumpbin
/headers /loadconfig" against chrome.exe or chrome_elf.dll.  1) "DLL
characteristics" now includes "Control Flow Guard", and 2) if code was compiled
with cfg there will be "load config" for "Guard CF" function table and flags.

BUG=584575
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
[Windows MSVC CFG] Add support for CFG, and enable on chrome.exe &
chrome_elf.dll.

CFG supported on x86 and x64 >= Windows 8.1 Update (KB3000850).

Initial support for Control Flow Guard.  "Enable" on chrome.exe to take
advantage of MS system32 dll CFG.  Also compile and link chrome_elf with CFG.

NOTE: As ASLR is disabled in debug builds, so too is CFG.

Using the dumpbin tool from the Visual Studio command prompt, run "dumpbin
/headers /loadconfig" against chrome.exe or chrome_elf.dll.  1) "DLL
characteristics" now includes "Control Flow Guard", and 2) if code was compiled
with cfg there will be "load config" for "Guard CF" function table and flags.

BUG=584575
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Committed: https://crrev.com/d46842301ebc1f41a92448026bf95e3194538213
Cr-Commit-Position: refs/heads/master@{#425226}
==========

[commit-bot: I haz the power, 2016-10-14 01:17:07]

Patchset 3 (id:??) landed as
https://crrev.com/d46842301ebc1f41a92448026bf95e3194538213
Cr-Commit-Position: refs/heads/master@{#425226}