Revert of Allocator shims working on VS2015.

Revert of Allocator shims working on VS2015. (patchset #12 id:220001 of https://codereview.chromium.org/1414453017/ )

Reason for revert:
We're seeing probably the same or similar issue when linking on win again.

https://build.chromium.org/p/chromium/builders/Win/builds/45048

FAILED: blacklist_test_main_dll.dll blacklist_test_main_dll.dll.lib blacklist_test_main_dll.dll.pdb 
C:\b\depot_tools\python276_bin\python.exe gyp-win-tool link-with-manifests environment.x86 True blacklist_test_main_dll.dll "C:\b\depot_tools\python276_bin\python.exe gyp-win-tool link-wrapper environment.x86 False link.exe /nologo /IMPLIB:blacklist_test_main_dll.dll.lib /DLL /OUT:blacklist_test_main_dll.dll @blacklist_test_main_dll.dll.rsp" 2 mt.exe rc.exe "obj\chrome_elf\blacklist_test_main_dll.blacklist_test_main_dll.dll.intermediate.manifest" obj\chrome_elf\blacklist_test_main_dll.blacklist_test_main_dll.dll.generated.manifest
allocator.lib(allocator.allocator_shim_win.obj) : error LNK2005: __query_new_mode already defined in libucrt.lib(new_mode.obj)
allocator.lib(allocator.allocator_shim_win.obj) : error LNK2005: __set_new_mode already defined in libucrt.lib(new_mode.obj)
allocator.lib(allocator.allocator_shim_win.obj) : error LNK2005: _calloc already defined in libucrt.lib(calloc.obj)
allocator.lib(allocator.allocator_shim_win.obj) : error LNK2005: _free already defined in libucrt.lib(free.obj)
allocator.lib(allocator.allocator_shim_win.obj) : error LNK2005: _malloc already defined in libucrt.lib(malloc.obj)
blacklist_test_main_dll.dll : fatal error LNK1169: one or more multiply defined symbols found

Let me revert this CL to see if the revert fixes the build breakage.

Original issue's description:
> Allocator shims working on VS2015.
> 
> VS2015 is happy for us to simply override the CRT symbols since they
> seem to be defined weakly by the universal CRT.
> 
> This shim is far simpler than the previous one, as the libcmt stripping
> technique can be removed.
> 
> Allocator shim is also now only be enabled on Release Static builds.
> 
> BUG=481611
> TEST=base_unittests --gtest_filter=*Memory* in all configurations
>  (Debug/Release/Static/Component/GN/gyp/x64/x86)
> CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
> 
> Committed: https://crrev.com/e20c2e0ab4e5b796e7a1890fb9048b5eb307d015
> Cr-Commit-Position: refs/heads/master@{#383810}
> 
> Committed: https://crrev.com/08a9665ab0bce3813425f8d7ec9bec9178d5faa4
> Cr-Commit-Position: refs/heads/master@{#384711}

TBR=thakis@chromium.org,brucedawson@chromium.org,rnk@chromium.org,piman@chromium.org,primiano@chromium.org,wfh@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=481611

[Yuki, 2016-07-08 04:51:39]

The CQ bit was checked by yukishiino@chromium.org

[Yuki, 2016-07-08 04:51:40]

Created Revert of Allocator shims working on VS2015.

[commit-bot: I haz the power, 2016-07-08 04:52:02]

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

[Will Harris, 2016-07-08 05:04:44]

On 2016/07/08 04:52:02, commit-bot: I haz the power wrote:
> CQ is trying da patch. Follow status at
>  
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

This cl landed three months ago, it seems strange to revert it. I don't think
this should be reverted.

[Will Harris, 2016-07-08 05:04:53]

The CQ bit was unchecked by wfh@chromium.org

[piman, 2016-07-08 05:15:19]

On 2016/07/08 05:04:44, Will Harris wrote:
> On 2016/07/08 04:52:02, commit-bot: I haz the power wrote:
> > CQ is trying da patch. Follow status at
> >  
> >
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
> 
> This cl landed three months ago, it seems strange to revert it. I don't think
> this should be reverted.

Right, please do not revert a 3-month-old CL without review.

[Will Harris, 2016-07-08 05:25:23]

On 2016/07/08 05:15:19, piman wrote:
> On 2016/07/08 05:04:44, Will Harris wrote:
> > On 2016/07/08 04:52:02, commit-bot: I haz the power wrote:
> > > CQ is trying da patch. Follow status at
> > >  
> > >
> >
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
> > 
> > This cl landed three months ago, it seems strange to revert it. I don't
think
> > this should be reverted.
> 
> Right, please do not revert a 3-month-old CL without review.

The bug for this is here

https://bugs.chromium.org/p/chromium/issues/detail?id=626539

I'm typing on phone with no access to desktop now but if the cl that caused this
hasn't been identified by tomorrow then me or Bruce can find it. You need to
drill down into the link order and find out what has changed. Perhaps something
in base (just a guess).

[Will Harris, 2016-07-08 05:29:01]

On 2016/07/08 05:25:23, Will Harris wrote:
> On 2016/07/08 05:15:19, piman wrote:
> > On 2016/07/08 05:04:44, Will Harris wrote:
> > > On 2016/07/08 04:52:02, commit-bot: I haz the power wrote:
> > > > CQ is trying da patch. Follow status at
> > > >  
> > > >
> > >
> >
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
> > > 
> > > This cl landed three months ago, it seems strange to revert it. I don't
> think
> > > this should be reverted.
> > 
> > Right, please do not revert a 3-month-old CL without review.
> 
> The bug for this is here
> 
> https://bugs.chromium.org/p/chromium/issues/detail?id=626539
> 
> I'm typing on phone with no access to desktop now but if the cl that caused
this
> hasn't been identified by tomorrow then me or Bruce can find it. You need to
> drill down into the link order and find out what has changed. Perhaps
something
> in base (just a guess).

I should add that reverting the shim will *suppress* this error but it wont be
fixing the root cause. If there is a critical need to reopen the tree then go
ahead and revert but we cannot ship builds to channels higher than Canary
without allocator shim as it is a security regression.

[Yuki, 2016-07-08 05:41:52]

On 2016/07/08 05:29:01, Will Harris wrote:
> On 2016/07/08 05:25:23, Will Harris wrote:
> > On 2016/07/08 05:15:19, piman wrote:
> > > On 2016/07/08 05:04:44, Will Harris wrote:
> > > > On 2016/07/08 04:52:02, commit-bot: I haz the power wrote:
> > > > > CQ is trying da patch. Follow status at
> > > > >  
> > > > >
> > > >
> > >
> >
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
> > > > 
> > > > This cl landed three months ago, it seems strange to revert it. I don't
> > think
> > > > this should be reverted.
> > > 
> > > Right, please do not revert a 3-month-old CL without review.
> > 
> > The bug for this is here
> > 
> > https://bugs.chromium.org/p/chromium/issues/detail?id=626539
> > 
> > I'm typing on phone with no access to desktop now but if the cl that caused
> this
> > hasn't been identified by tomorrow then me or Bruce can find it. You need to
> > drill down into the link order and find out what has changed. Perhaps
> something
> > in base (just a guess).
> 
> I should add that reverting the shim will *suppress* this error but it wont be
> fixing the root cause. If there is a critical need to reopen the tree then go
> ahead and revert but we cannot ship builds to channels higher than Canary
> without allocator shim as it is a security regression.

Thanks for the reply.  I'll try to find another way.  If I couldn't solve this
issue by EOD in my time zone, then I'll file a bug assigning you.  In that case,
please take care of it.  Thanks in advance.

[Yuki, 2016-07-08 07:19:49]

Description was changed from

==========
Revert of Allocator shims working on VS2015. (patchset #12 id:220001 of
https://codereview.chromium.org/1414453017/ )

Reason for revert:
We're seeing probably the same or similar issue when linking on win again.

https://build.chromium.org/p/chromium/builders/Win/builds/45048

FAILED: blacklist_test_main_dll.dll blacklist_test_main_dll.dll.lib
blacklist_test_main_dll.dll.pdb 
C:\b\depot_tools\python276_bin\python.exe gyp-win-tool link-with-manifests
environment.x86 True blacklist_test_main_dll.dll
"C:\b\depot_tools\python276_bin\python.exe gyp-win-tool link-wrapper
environment.x86 False link.exe /nologo /IMPLIB:blacklist_test_main_dll.dll.lib
/DLL /OUT:blacklist_test_main_dll.dll @blacklist_test_main_dll.dll.rsp" 2 mt.exe
rc.exe
"obj\chrome_elf\blacklist_test_main_dll.blacklist_test_main_dll.dll.intermediate.manifest"
obj\chrome_elf\blacklist_test_main_dll.blacklist_test_main_dll.dll.generated.manifest
allocator.lib(allocator.allocator_shim_win.obj) : error LNK2005:
__query_new_mode already defined in libucrt.lib(new_mode.obj)
allocator.lib(allocator.allocator_shim_win.obj) : error LNK2005: __set_new_mode
already defined in libucrt.lib(new_mode.obj)
allocator.lib(allocator.allocator_shim_win.obj) : error LNK2005: _calloc already
defined in libucrt.lib(calloc.obj)
allocator.lib(allocator.allocator_shim_win.obj) : error LNK2005: _free already
defined in libucrt.lib(free.obj)
allocator.lib(allocator.allocator_shim_win.obj) : error LNK2005: _malloc already
defined in libucrt.lib(malloc.obj)
blacklist_test_main_dll.dll : fatal error LNK1169: one or more multiply defined
symbols found

Let me revert this CL to see if the revert fixes the build breakage.

Original issue's description:
> Allocator shims working on VS2015.
> 
> VS2015 is happy for us to simply override the CRT symbols since they
> seem to be defined weakly by the universal CRT.
> 
> This shim is far simpler than the previous one, as the libcmt stripping
> technique can be removed.
> 
> Allocator shim is also now only be enabled on Release Static builds.
> 
> BUG=481611
> TEST=base_unittests --gtest_filter=*Memory* in all configurations
>  (Debug/Release/Static/Component/GN/gyp/x64/x86)
>
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
> 
> Committed: https://crrev.com/e20c2e0ab4e5b796e7a1890fb9048b5eb307d015
> Cr-Commit-Position: refs/heads/master@{#383810}
> 
> Committed: https://crrev.com/08a9665ab0bce3813425f8d7ec9bec9178d5faa4
> Cr-Commit-Position: refs/heads/master@{#384711}

TBR=thakis@chromium.org,brucedawson@chromium.org,rnk@chromium.org,piman@chrom...
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=481611
==========

to

==========
Revert of Allocator shims working on VS2015. (patchset #12 id:220001 of
https://codereview.chromium.org/1414453017/ )

Reason for revert:
We're seeing probably the same or similar issue when linking on win again.

https://build.chromium.org/p/chromium/builders/Win/builds/45048

FAILED: blacklist_test_main_dll.dll blacklist_test_main_dll.dll.lib
blacklist_test_main_dll.dll.pdb 
C:\b\depot_tools\python276_bin\python.exe gyp-win-tool link-with-manifests
environment.x86 True blacklist_test_main_dll.dll
"C:\b\depot_tools\python276_bin\python.exe gyp-win-tool link-wrapper
environment.x86 False link.exe /nologo /IMPLIB:blacklist_test_main_dll.dll.lib
/DLL /OUT:blacklist_test_main_dll.dll @blacklist_test_main_dll.dll.rsp" 2 mt.exe
rc.exe
"obj\chrome_elf\blacklist_test_main_dll.blacklist_test_main_dll.dll.intermediate.manifest"
obj\chrome_elf\blacklist_test_main_dll.blacklist_test_main_dll.dll.generated.manifest
allocator.lib(allocator.allocator_shim_win.obj) : error LNK2005:
__query_new_mode already defined in libucrt.lib(new_mode.obj)
allocator.lib(allocator.allocator_shim_win.obj) : error LNK2005: __set_new_mode
already defined in libucrt.lib(new_mode.obj)
allocator.lib(allocator.allocator_shim_win.obj) : error LNK2005: _calloc already
defined in libucrt.lib(calloc.obj)
allocator.lib(allocator.allocator_shim_win.obj) : error LNK2005: _free already
defined in libucrt.lib(free.obj)
allocator.lib(allocator.allocator_shim_win.obj) : error LNK2005: _malloc already
defined in libucrt.lib(malloc.obj)
blacklist_test_main_dll.dll : fatal error LNK1169: one or more multiply defined
symbols found

Let me revert this CL to see if the revert fixes the build breakage.

Original issue's description:
> Allocator shims working on VS2015.
> 
> VS2015 is happy for us to simply override the CRT symbols since they
> seem to be defined weakly by the universal CRT.
> 
> This shim is far simpler than the previous one, as the libcmt stripping
> technique can be removed.
> 
> Allocator shim is also now only be enabled on Release Static builds.
> 
> BUG=481611
> TEST=base_unittests --gtest_filter=*Memory* in all configurations
>  (Debug/Release/Static/Component/GN/gyp/x64/x86)
>
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
> 
> Committed: https://crrev.com/e20c2e0ab4e5b796e7a1890fb9048b5eb307d015
> Cr-Commit-Position: refs/heads/master@{#383810}
> 
> Committed: https://crrev.com/08a9665ab0bce3813425f8d7ec9bec9178d5faa4
> Cr-Commit-Position: refs/heads/master@{#384711}

TBR=thakis@chromium.org,brucedawson@chromium.org,rnk@chromium.org,piman@chrom...
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=481611
==========

[Primiano Tucci (use gerrit), 2016-07-08 13:25:58]

On 2016/07/08 05:41:52, Yuki wrote:
> Thanks for the reply.  I'll try to find another way.  If I couldn't solve this
> issue by EOD in my time zone, then I'll file a bug assigning you.  In that
case,
> please take care of it.  Thanks in advance.

So, I ended up being pinged by an EMEA sheriff.
After some investigation turns out that the culrpit was the change in
sandbox/win/src/win_utils.cc introduced by
https://codereview.chromium.org/2121513002
Which I am now reverting in https://codereview.chromium.org/2135653002/

It is not clear to me why that's happening. I have a very limited knowledge
about what chrome_elf is and how it is supposed to work. I suppose that change
ended up pulling some extra dep via base and changing the linker invocation.
Unfortunately don't have time left to investigate further. Having to bring back
my windows machine from life and building on a terminal surrogate was enough of
a painful experience for today :P

[Yuki, 2016-07-08 13:44:16]

On 2016/07/08 13:25:58, Primiano Tucci wrote:
> On 2016/07/08 05:41:52, Yuki wrote:
> > Thanks for the reply.  I'll try to find another way.  If I couldn't solve
this
> > issue by EOD in my time zone, then I'll file a bug assigning you.  In that
> case,
> > please take care of it.  Thanks in advance.
> 
> So, I ended up being pinged by an EMEA sheriff.
> After some investigation turns out that the culrpit was the change in
> sandbox/win/src/win_utils.cc introduced by
> https://codereview.chromium.org/2121513002
> Which I am now reverting in https://codereview.chromium.org/2135653002/
> 
> It is not clear to me why that's happening. I have a very limited knowledge
> about what chrome_elf is and how it is supposed to work. I suppose that change
> ended up pulling some extra dep via base and changing the linker invocation.
> Unfortunately don't have time left to investigate further. Having to bring
back
> my windows machine from life and building on a terminal surrogate was enough
of
> a painful experience for today :P

I'm sorry for that and thanks for your work.
It was surprise for me that the CL changed the linker invocation somehow without
changing any *.gyp(i) or *.gn(i).

[brucedawson, 2016-07-11 19:54:24]

> I'm sorry for that and thanks for your work.
> It was surprise for me that the CL changed the linker invocation somehow
without
> changing any *.gyp(i) or *.gn(i).

If there are multiple object files that define the same symbol then linking can
be very fragile and can easily be perturbed by seemingly irrelevant changes.

The linker repeatedly scans through its library inputs looking for the symbols
that it needs to resolve. As it does this it also finds new symbols that have to
be resolved. If a symbol is defined in multiple object files then which object
file it pulls in depends on exactly when in this cycle the symbol is first
referenced. So, a change in library order or an arbitrarily long chain of symbol
references can perturb the behavior. Or, a reference to a new symbol can do
this.

In this case something caused libucrt.lib(new_mode.obj) to be referenced when
Will's allocator.lib(allocator.allocator_shim_win.obj) was supposed to be
supplying the symbols. This could mean that there is a symbol that exists only
in new_mode.obj that somebody referenced. Or it could mean that new_mode.obj
happened to win the race.

If it is the latter (I assume that it is) then the ideal fix is to explicitly
link in allocator_shim_win.obj, instead of pulling it in from a library. That
guarantees that the desired .obj file always wins the race. Unfortunately Chrome
lacks a convenient mechanism for doing this in every binary. We thought that we
had found a stable way of pulling in the correct .obj file, but I guess it's not
as stable as we had hoped.