Make Win Clang ASan builds work

Make Win Clang ASan builds work.

CLOSED - OTHER FIXES LANDED INSTEAD.

R=etienneb@chromium.org, thakis@chromium.org
BUG=598671

[Dirk Pranke, 2016-08-02 01:09:28]

Description was changed from

==========
patch from https://codereview.chromium.org/download/issue2189153002_1.diff

R=etienneb@chromium.org, thakis@chromium.org
BUG=598671
==========

to

==========
Make Win Clang ASan builds work.

R=etienneb@chromium.org, thakis@chromium.org
BUG=598671
==========

[Dirk Pranke, 2016-08-02 01:10:43]

I think this is most if not all of what's needed to make the asan build work.

However, the build is really really slow so I've only so far been able to
confirm that content_shell links cleanly on my latest build.

When I try to run this in a static build, things seem okay until we run
mksnapshot, which keels over with an actual sanitizer error that's probably
worth looking into.

[Nico, 2016-08-02 14:20:51]

thakis@chromium.org changed reviewers:
+ rnk@chromium.org

[Nico, 2016-08-02 14:20:52]

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

https://codereview.chromium.org/2198333002/diff/20001/build/config/BUILD.gn#n...
build/config/BUILD.gn:438: configs += [
"//build/config/sanitizers:executable_config" ]
It seems very surprising to me that shared_library_config here depends on
executable_config in the other place. I think it's better to have a
shared_library_config in the other place as well.

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

https://codereview.chromium.org/2198333002/diff/20001/build/config/sanitizers...
build/config/sanitizers/BUILD.gn:259: libs = [
"clang_rt.asan_dynamic_runtime_thunk-x86_64.lib" ]
Why is this needed here at all now that the other config exists? Shouldn't you
have

 137 config("executable_config") {
 138   if (is_win && is_asan) {
 139     if (current_cpu == "x64") {
           if (is_component_build) {
             libs = [ "clang_rt.asan_dynamic-x86_64.lib",
                       "clang_rt.asan_dynamic_runtime_thunk-x86_64.lib",
             ]
           else {
 140         libs = [ "clang_rt.asan-x86_64.lib" ]
           }
 141     } else {
 142       # same for 32 bit
 143     }
 144   }
 145 }

and

 137 config("shared_library_config") {
 138   if (is_win && is_asan) {
 139     if (current_cpu == "x64") {
           if (is_component_build) {
             libs = [ "clang_rt.asan_dynamic-x86_64.lib",
                       "clang_rt.asan_dynamic_runtime_thunk-x86_64.lib",
             ]
           else {
 140         libs = [ "clang_rt.asan_dynamic-x86_64.lib" ]
           }
 141     } else {
 142       # same for 32 bit
 143     }
 144   }
 145 }

[Dirk Pranke, 2016-08-02 17:19:03]

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

https://codereview.chromium.org/2198333002/diff/20001/build/config/BUILD.gn#n...
build/config/BUILD.gn:438: configs += [
"//build/config/sanitizers:executable_config" ]
On 2016/08/02 14:20:51, Nico wrote:
> It seems very surprising to me that shared_library_config here depends on
> executable_config in the other place. I think it's better to have a
> shared_library_config in the other place as well.

Agreed; This patch was based on etienneb's patch, and I hadn't noticed this
line, but it should change.

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

https://codereview.chromium.org/2198333002/diff/20001/build/config/sanitizers...
build/config/sanitizers/BUILD.gn:259: libs = [
"clang_rt.asan_dynamic_runtime_thunk-x86_64.lib" ]
Yes, I think you're right.

[etienneb, 2016-08-02 22:50:27]

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

https://codereview.chromium.org/2198333002/diff/20001/build/config/BUILD.gn#n...
build/config/BUILD.gn:438: configs += [
"//build/config/sanitizers:executable_config" ]
On 2016/08/02 17:19:03, Dirk Pranke wrote:
> On 2016/08/02 14:20:51, Nico wrote:
> > It seems very surprising to me that shared_library_config here depends on
> > executable_config in the other place. I think it's better to have a
> > shared_library_config in the other place as well.
> 
> Agreed; This patch was based on etienneb's patch, and I hadn't noticed this
> line, but it should change.

I agree too.
This was a quick try to make the build work.

[Dirk Pranke, 2016-08-11 01:48:31]

Description was changed from

==========
Make Win Clang ASan builds work.

R=etienneb@chromium.org, thakis@chromium.org
BUG=598671
==========

to

==========
Make Win Clang ASan builds work.

CLOSED - OTHER FIXES LANDED INSTEAD.

R=etienneb@chromium.org, thakis@chromium.org
BUG=598671
==========