Implement use_cfi_cast to optionally enable cast checks.

This is to allow launching cfi-vcal first, and follow up with additional strictness later. BUG=626794,464797 Review-Url: https://codereview.chromium.org/2131423002 Cr-Commit-Position: refs/heads/master@{#404956}
Denger-Network · Jul 13, 2016 · 59d3718 · 59d3718
1 parent 78696be
commit 59d3718
Show file tree

Hide file tree

Showing 6 changed files with 93 additions and 13 deletions.
diff --git a/base/BUILD.gn b/base/BUILD.gn
@@ -1979,6 +1979,8 @@ test("base_unittests") {
     "win/wrapped_window_proc_unittest.cc",
   ]
 
+  defines = []
+
   deps = [
     ":base",
     ":i18n",
@@ -2009,7 +2011,7 @@ test("base_unittests") {
   # Allow more direct string conversions on platforms with native utf8
   # strings
   if (is_mac || is_ios || is_chromeos || is_chromecast) {
-    defines = [ "SYSTEM_NATIVE_UTF8" ]
+    defines += [ "SYSTEM_NATIVE_UTF8" ]
   }
 
   if (is_android) {
@@ -2114,6 +2116,11 @@ test("base_unittests") {
       # data += [ "$root_out_dir/base_unittests.dSYM/" ]
     }
   }
+
+  if (use_cfi_cast) {
+    # TODO(krasin): remove CFI_CAST_CHECK, see https://crbug.com/626794.
+    defines += [ "CFI_CAST_CHECK" ]
+  }
 }
 
 action("build_date") {

diff --git a/base/base.gyp b/base/base.gyp
@@ -639,6 +639,12 @@
         'module_dir': 'base'
       },
       'conditions': [
+        ['cfi_vptr==1 and cfi_cast==1', {
+          'defines': [
+             # TODO(krasin): remove CFI_CAST_CHECK, see https://crbug.com/626794.
+            'CFI_CAST_CHECK',
+          ],
+        }],
         ['OS == "ios" or OS == "mac"', {
           'dependencies': [
             'base_unittests_arc',

diff --git a/base/tools_sanity_unittest.cc b/base/tools_sanity_unittest.cc
@@ -343,6 +343,8 @@ TEST(ToolsSanityTest, AtomicsAreIgnored) {
 }
 
 #if defined(CFI_ENFORCEMENT)
+// TODO(krasin): remove CFI_CAST_CHECK, see https://crbug.com/626794.
+#if defined(CFI_CAST_CHECK)
 TEST(ToolsSanityTest, BadCast) {
   class A {
     virtual void f() {}
@@ -355,6 +357,32 @@ TEST(ToolsSanityTest, BadCast) {
   A a;
   EXPECT_DEATH((void)(B*)&a, "ILL_ILLOPN");
 }
-#endif
+#endif // CFI_CAST_CHECK
+
+class A {
+ public:
+  A(): n_(0) {}
+  virtual void f() { n_++; }
+ protected:
+  int n_;
+};
+
+class B: public A {
+ public:
+  void f() override { n_--; }
+};
+
+NOINLINE void KillVptrAndCall(A *obj) {
+  *reinterpret_cast<void **>(obj) = 0;
+  obj->f();
+}
+
+TEST(ToolsSanityTest, BadVirtualCall) {
+  A a;
+  B b;
+  EXPECT_DEATH({ KillVptrAndCall(&a); KillVptrAndCall(&b); }, "ILL_ILLOPN");
+}
+
+#endif // CFI_ENFORCEMENT
 
 }  // namespace base
diff --git a/build/common.gypi b/build/common.gypi
@@ -698,6 +698,8 @@
       # Control Flow Integrity for virtual calls and casts.
       # See http://clang.llvm.org/docs/ControlFlowIntegrity.html
       'cfi_vptr%': 0,
+      # TODO(krasin): remove it. See https://crbug.com/626794.
+      'cfi_cast%': 0,
       'cfi_diag%': 0,
 
       'cfi_blacklist%': '<(PRODUCT_DIR)/../../tools/cfi/blacklist.txt',
@@ -1259,6 +1261,7 @@
     'video_hole%': '<(video_hole)',
     'v8_use_external_startup_data%': '<(v8_use_external_startup_data)',
     'cfi_vptr%': '<(cfi_vptr)',
+    'cfi_cast%': '<(cfi_cast)',
     'cfi_diag%': '<(cfi_diag)',
     'cfi_blacklist%': '<(cfi_blacklist)',
     'mac_views_browser%': '<(mac_views_browser)',
@@ -6212,8 +6215,6 @@
           ['_toolset=="target"', {
             'cflags': [
               '-fsanitize=cfi-vcall',
-              '-fsanitize=cfi-derived-cast',
-              '-fsanitize=cfi-unrelated-cast',
               '-fsanitize-blacklist=<(cfi_blacklist)',
             ],
             'ldflags': [
@@ -6224,8 +6225,6 @@
             'xcode_settings': {
               'OTHER_CFLAGS': [
                 '-fsanitize=cfi-vcall',
-                '-fsanitize=cfi-derived-cast',
-                '-fsanitize=cfi-unrelated-cast',
                 '-fsanitize-blacklist=<(cfi_blacklist)',
               ],
             },
@@ -6234,6 +6233,34 @@
             'xcode_settings':  {
               'OTHER_LDFLAGS': [
                 '-fsanitize=cfi-vcall',
+              ],
+            },
+          }],
+        ],
+      },
+    }],
+    ['cfi_vptr==1 and cfi_cast==1', {
+      'target_defaults': {
+        'target_conditions': [
+          ['_toolset=="target"', {
+            'cflags': [
+              '-fsanitize=cfi-derived-cast',
+              '-fsanitize=cfi-unrelated-cast',
+            ],
+            'ldflags': [
+              '-fsanitize=cfi-derived-cast',
+              '-fsanitize=cfi-unrelated-cast',
+            ],
+            'xcode_settings': {
+              'OTHER_CFLAGS': [
+                '-fsanitize=cfi-derived-cast',
+                '-fsanitize=cfi-unrelated-cast',
+              ],
+            },
+          }],
+          ['_toolset=="target" and _type!="static_library"', {
+            'xcode_settings':  {
+              'OTHER_LDFLAGS': [
                 '-fsanitize=cfi-derived-cast',
                 '-fsanitize=cfi-unrelated-cast',
               ],

diff --git a/build/config/sanitizers/BUILD.gn b/build/config/sanitizers/BUILD.gn
@@ -155,11 +155,13 @@ config("default_sanitizer_ldflags") {
     }
 
     if (is_cfi && !is_nacl) {
-      ldflags += [
-        "-fsanitize=cfi-vcall",
-        "-fsanitize=cfi-derived-cast",
-        "-fsanitize=cfi-unrelated-cast",
-      ]
+      ldflags += [ "-fsanitize=cfi-vcall" ]
+      if (use_cfi_cast) {
+        ldflags += [
+          "-fsanitize=cfi-derived-cast",
+          "-fsanitize=cfi-unrelated-cast",
+        ]
+      }
       if (use_cfi_diag) {
         ldflags += [
           "-fno-sanitize-trap=cfi",
@@ -246,11 +248,16 @@ config("cfi_flags") {
         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_cast) {
+      cflags += [
+        "-fsanitize=cfi-derived-cast",
+        "-fsanitize=cfi-unrelated-cast",
+      ]
+    }
+
     if (use_cfi_diag) {
       cflags += [
         "-fno-sanitize-trap=cfi",

diff --git a/build/config/sanitizers/sanitizers.gni b/build/config/sanitizers/sanitizers.gni
@@ -49,6 +49,11 @@ declare_args() {
   # TODO(pcc): Remove this flag if/when CFI is enabled in official builds.
   is_cfi = false
 
+  # Enable checks for bad casts: derived cast and unrelated cast.
+  # TODO(krasin): remove this, when we're ready to add these checks by default.
+  # https://crbug.com/626794
+  use_cfi_cast = false
+
   # By default, Control Flow Integrity will crash the program if it detects a
   # violation. Set this to true to print detailed diagnostics instead.
   use_cfi_diag = false