third_party/mesa: turn off ubsan_vptr, since it's compiled without rtti.

build/config/sanitizers/BUILD.gn

Index: build/config/sanitizers/BUILD.gn
diff --git a/build/config/sanitizers/BUILD.gn b/build/config/sanitizers/BUILD.gn
index e7e36b5db84422529b68fcf56e244ea317fb9490..b08bc33f233f77e97072e1317165eea64eabb84f 100644
--- a/build/config/sanitizers/BUILD.gn
+++ b/build/config/sanitizers/BUILD.gn
@@ -113,18 +113,9 @@ config("default_sanitizer_ldflags") {
   }
 }
 
-# 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 (by removing the config) if they desire. Even if a target
-# removes this config, executables & shared libraries should still depend on
-# :deps if any of their dependencies have not opted out of sanitizers.
-config("default_sanitizer_flags") {
+config("common_sanitizer_flags") {
   cflags = []
   cflags_cc = []
-  defines = []
-  configs = [ ":default_sanitizer_ldflags" ]
 
   # Sanitizers need line table info for stack traces. They don't need type info
   # or variable info, so we can leave that out to speed up the build.
@@ -142,6 +133,20 @@ config("default_sanitizer_flags") {
       cflags += [ "/Oy-" ]
     }
   }
+
+  if (use_custom_libcxx) {
+    prefix = "//buildtools/third_party"
+    include = "trunk/include"
+    cflags_cc += [
+      "-nostdinc++",
+      "-isystem" + rebase_path("$prefix/libc++/$include", root_build_dir),
+      "-isystem" + rebase_path("$prefix/libc++abi/$include", root_build_dir),
+    ]
+  }
+}
+
+config("asan_flags") {
+  cflags = []
   if (is_asan) {
     cflags += [ "-fsanitize=address" ]
     if (is_win) {
@@ -186,18 +191,43 @@ config("default_sanitizer_flags") {
       }
     }
   }
-  if (is_lsan) {
-    cflags += [ "-fsanitize=leak" ]
-  }
-  if (is_tsan) {
-    assert(is_linux, "tsan only supported on linux x86_64")
-    tsan_blacklist_path =
-        rebase_path("//tools/memory/tsan_v2/ignores.txt", root_build_dir)
+}
+
+config("cfi_flags") {
+  defines = []

[brettw, 2016/04/27 22:03:23]

This needs a cflags = []

[krasin, 2016/04/27 22:54:37]

Done. Thank you for the catch. I've removed += from defines, as they are set in
a single place.

+  if (is_cfi && !is_nacl) {
+    cfi_blacklist_path =
+        rebase_path("//tools/cfi/blacklist.txt", root_build_dir)
     cflags += [
-      "-fsanitize=thread",
-      "-fsanitize-blacklist=$tsan_blacklist_path",
+      "-fsanitize=cfi-vcall",
+      "-fsanitize=cfi-derived-cast",
+      "-fsanitize=cfi-unrelated-cast",
+      "-fsanitize-blacklist=$cfi_blacklist_path",
     ]
+
+    if (use_cfi_diag) {
+      cflags += [
+        "-fno-sanitize-trap=cfi",
+        "-fsanitize-recover=cfi",
+        "-fno-inline-functions",
+        "-fno-inline",
+        "-fno-omit-frame-pointer",
+        "-O1",
+      ]
+    } else {
+      defines += [ "CFI_ENFORCEMENT" ]
+    }
   }
+}
+
+config("lsan_flags") {
+  if (is_lsan) {
+    cflags = [ "-fsanitize=leak" ]
+  }
+}
+
+config("msan_flags") {
+  cflags = []

[brettw, 2016/04/27 22:03:23]

In this case and a few below when it's only set once and nothing else happens, I
think it's better to just do "cflags = []" once and not do the empty one at the
top and append later pattern. GN will tell you if you clobber a nonempty list
with another nonempty list, which effectively eliminates mistakes.

[krasin, 2016/04/27 22:54:37]

Done. Thanks, that's a useful warning.

   if (is_msan) {
     assert(is_linux, "msan only supported on linux x86_64")
     msan_blacklist_path =
@@ -208,6 +238,23 @@ config("default_sanitizer_flags") {
       "-fsanitize-blacklist=$msan_blacklist_path",
     ]
   }
+}
+
+config("tsan_flags") {
+  cflags = []
+  if (is_tsan) {
+    assert(is_linux, "tsan only supported on linux x86_64")
+    tsan_blacklist_path =
+        rebase_path("//tools/memory/tsan_v2/ignores.txt", root_build_dir)
+    cflags += [

[brettw, 2016/04/27 22:03:23]

Ditto, just = and delete the empty list at the top.

[krasin, 2016/04/27 22:54:37]

Done.

+      "-fsanitize=thread",
+      "-fsanitize-blacklist=$tsan_blacklist_path",
+    ]
+  }
+}
+
+config("ubsan_flags") {
+  cflags = []
   if (is_ubsan) {
     ubsan_blacklist_path =
         rebase_path("//tools/ubsan/blacklist.txt", root_build_dir)
@@ -245,14 +292,9 @@ config("default_sanitizer_flags") {
       ]
     }
   }
-  if (is_ubsan_vptr) {
-    ubsan_vptr_blacklist_path =
-        rebase_path("//tools/ubsan/vptr_blacklist.txt", root_build_dir)
-    cflags += [
-      "-fsanitize=vptr",
-      "-fsanitize-blacklist=$ubsan_vptr_blacklist_path",
-    ]
-  }
+}
+
+config("ubsan_security_flags") {
   if (is_ubsan_security) {
     ubsan_blacklist_path =
         rebase_path("//tools/ubsan/blacklist.txt", root_build_dir)
@@ -261,43 +303,63 @@ config("default_sanitizer_flags") {
       "-fsanitize-blacklist=$ubsan_blacklist_path",
     ]
   }
-  if (is_cfi && !is_nacl) {
-    cfi_blacklist_path =
-        rebase_path("//tools/cfi/blacklist.txt", root_build_dir)
-    cflags += [
-      "-fsanitize=cfi-vcall",
-      "-fsanitize=cfi-derived-cast",
-      "-fsanitize=cfi-unrelated-cast",
-      "-fsanitize-blacklist=$cfi_blacklist_path",
-    ]
-
-    if (use_cfi_diag) {
-      cflags += [
-        "-fno-sanitize-trap=cfi",
-        "-fsanitize-recover=cfi",
-        "-fno-inline-functions",
-        "-fno-inline",
-        "-fno-omit-frame-pointer",
-        "-O1",
-      ]
-    } else {
-      defines += [ "CFI_ENFORCEMENT" ]
-    }
-  }
+}
 
-  if (use_custom_libcxx) {
-    prefix = "//buildtools/third_party"
-    include = "trunk/include"
-    cflags_cc += [
-      "-nostdinc++",
-      "-isystem" + rebase_path("$prefix/libc++/$include", root_build_dir),
-      "-isystem" + rebase_path("$prefix/libc++abi/$include", root_build_dir),
+config("ubsan_vptr_flags") {
+  if (is_ubsan_vptr) {
+    ubsan_vptr_blacklist_path =
+        rebase_path("//tools/ubsan/vptr_blacklist.txt", root_build_dir)
+    cflags = [
+      "-fsanitize=vptr",
+      "-fsanitize-blacklist=$ubsan_vptr_blacklist_path",
     ]
   }
 }
 
+# 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 (by removing the config) if they desire. Even if a target
+# removes this config, executables & shared libraries should still depend on
+# :deps if any of their dependencies have not opted out of sanitizers.
+# Keep this list in sync with default_sanitizer_flags_but_ubsan_vptr.
+config("default_sanitizer_flags") {
+  configs = [
+    ":common_sanitizer_flags",
+    ":default_sanitizer_ldflags",
+    ":asan_flags",
+    ":cfi_flags",
+    ":lsan_flags",
+    ":msan_flags",
+    ":tsan_flags",
+    ":ubsan_flags",
+    ":ubsan_security_flags",
+    ":ubsan_vptr_flags",
+  ]
+}
+
+# This config is equivalent to default_sanitizer_flags, but excludes ubsan_vptr.
+# This allows to opt out of ubsan_vptr, when needed. In particular,
+# //third_party/mesa has some code compiled without rtti, which requires

[brettw, 2016/04/27 22:03:23]

I'd actually not bother mentioning where this is needed. Such things tend to get
out-of-date and it's better just to check for users to see what they want. It
might be nice to add a bit more background about why one might need to opt-out
of ubsan_ptr.

[krasin, 2016/04/27 22:54:37]

I've rephrased the comment.

+# to disable ubsan_vptr.
+config("default_sanitizer_flags_but_ubsan_vptr") {
+  configs = [
+    ":common_sanitizer_flags",
+    ":default_sanitizer_ldflags",
+    ":asan_flags",
+    ":cfi_flags",
+    ":lsan_flags",
+    ":msan_flags",
+    ":tsan_flags",
+    ":ubsan_flags",
+    ":ubsan_security_flags",
+  ]
+}
+
 config("default_sanitizer_coverage_flags") {
   cflags = []
+  defines = []

[brettw, 2016/04/27 22:03:23]

You can keep this config the way it was.

[krasin, 2016/04/27 22:54:37]

Done.

 
   if (use_sanitizer_coverage) {
     cflags += [
@@ -312,6 +374,6 @@ config("default_sanitizer_coverage_flags") {
         "-sanitizer-coverage-block-threshold=0",
       ]
     }
-    defines = [ "SANITIZER_COVERAGE" ]
+    defines += [ "SANITIZER_COVERAGE" ]
   }
 }