GN(android) Fix build all with is_asan

build/config/sanitizers/BUILD.gn

 # Copyright 2014 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import("//build/config/chrome_build.gni")
 import("//build/config/sanitizers/sanitizers.gni")
 
 # Contains the dependencies needed for sanitizers to link into executables and
 # shared_libraries. Unconditionally depend upon this target as it is empty if
 # |is_asan|, |is_lsan|, |is_tsan|, |is_msan| and |use_custom_libcxx| are false.
 group("deps") {
   if (using_sanitizer) {
-    public_configs = [ ":sanitizer_options_link_helper" ]
+    public_configs = [
+      ":sanitizer_options_link_helper",
+
+      # Even when a target removes default_sanitizer_flags, it may be depending
+      # on a library that did not remove default_sanitizer_flags. Thus, we need
+      # to add the ldflags here as well as in default_sanitizer_flags.
+      ":default_sanitizer_ldflags",

[brettw, 2016/01/19 20:39:42]

Doesn't this mean that if a target removes the default sanitizer flags, this
will add it back? That seems... undesirable. It seems like a benefit if you
remove the sanitizer config explicitly but it can't be removed, that you get an
error rather than having it silently be added back for you. Or maybe I
misunderstand some aspect of this.

[agrieve, 2016/02/02 19:22:31]

That's correct, but I think it's desirable and how it works already. 

If you list this as a dep, then you get the ldflags regardless of whether your
specific target removes the default_sanitizer_flags config (which mainly sets
ccflags)

If you don't want any sanitizer business in your app, then you should remove the
config *and* not depend on this target.

[brettw, 2016/02/02 22:31:17]

Okay, you convinced me.

+    ]
     deps = [
       ":options_sources",
     ]
 
     if (use_prebuilt_instrumented_libraries) {
       deps += [ "//third_party/instrumented_libraries:deps" ]
     }
     if (use_custom_libcxx) {
       deps += [ "//buildtools/third_party/libc++:libcxx_proxy" ]
     }
   }
 }
 
 config("sanitizer_options_link_helper") {
   ldflags = [ "-Wl,-u_sanitizer_options_link_helper" ]
-  if (is_asan) {
-    ldflags += [ "-fsanitize=address" ]
-  }
-  if (is_lsan) {
-    ldflags += [ "-fsanitize=leak" ]
-  }
-  if (is_tsan) {
-    ldflags += [ "-fsanitize=thread" ]
-  }
-  if (is_msan) {
-    ldflags += [ "-fsanitize=memory" ]
-  }
-  if (is_ubsan) {
-    ldflags += [ "-fsanitize=undefined" ]
-  }
-  if (is_ubsan_vptr) {
-    ldflags += [ "-fsanitize=vptr" ]
-  }
 }
 
 source_set("options_sources") {
   visibility = [
     ":deps",
     "//:gn_visibility",
   ]
   sources = [
     "//build/sanitizers/sanitizer_options.cc",
   ]
@@ skipping 10 matching lines @@
 
   if (is_lsan) {
     sources += [ "//build/sanitizers/lsan_suppressions.cc" ]
   }
 
   if (is_tsan) {
     sources += [ "//build/sanitizers/tsan_suppressions.cc" ]
   }
 }
 
+config("default_sanitizer_ldflags") {
+  if (is_posix) {
+    if (is_asan) {
+      ldflags = [ "-fsanitize=address" ]
+    }
+    if (is_lsan) {
+      ldflags = [ "-fsanitize=leak" ]
+    }
+    if (is_tsan) {
+      ldflags = [ "-fsanitize=thread" ]
+    }
+    if (is_msan) {
+      ldflags = [ "-fsanitize=memory" ]
+    }
+    if (is_ubsan) {
+      ldflags = [ "-fsanitize=undefined" ]
+    }
+    if (is_ubsan_vptr) {
+      ldflags = [ "-fsanitize=vptr" ]
+    }
+  }
+}
+
 # This config is applied by default to all targets. It sets the compiler flags
 # for sanitizer usage, or, if no sanitizer is set, does nothing.
 #
 # This needs to be in a separate config so that targets can opt out of
 # sanitizers if they desire.
 config("default_sanitizer_flags") {
   cflags = []
   cflags_cc = []
   ldflags = []
   defines = []
+  configs = [ ":default_sanitizer_ldflags" ]

[brettw, 2016/01/19 20:39:43]

This will cause these ldflags to get duplicated in the targets (since
sub-configs and regular configs aren't uniquified. I think we should avoid this.

[agrieve, 2016/02/02 19:22:31]

Didn't know that, and you obviously know about this way better than I do, but I
just did:
    ninja -n -v chrome_apk > build.log

And it looks like -fsanitize=address appears only once for each
gcc_solink_helper.py invokation.

[brettw, 2016/02/02 22:31:17]

Oh actually this is fine. I misunderstood this and thought this was duplicating
a config in BUILDCONFIG.

 
   # Only works on Posix-like platforms.
   # FIXME: this is not true, remove the conditional.
   if (is_posix) {
     # Common options for AddressSanitizer, LeakSanitizer, ThreadSanitizer,
     # MemorySanitizer and non-official CFI builds.
     if (using_sanitizer || (is_cfi && !is_official_build)) {
       cflags += [
         "-fno-omit-frame-pointer",
         "-gline-tables-only",
@@ skipping 145 matching lines @@
 
 config("default_sanitizer_coverage_flags") {
   cflags = []
 
   if (use_sanitizer_coverage) {
     # FIXME: make this configurable.
     cflags +=
         [ "-fsanitize-coverage=edge,indirect-calls,8bit-counters,trace-cmp" ]
   }
 }