New issue
Advanced search Search tips

Issue 751313 link

Starred by 1 user
Status: Fixed
Owner:
vtsyrklevich@chromium.org
Closed: Aug 2017
Cc:
kcc@chromium.org
p...@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug-Regression

Blocked on:
issue 750434


Sign in to add a comment

CFI: unit tests for vcalls and bad casts are insufficient and off by default

Project Member Reported by vtsyrklevich@chromium.org, Aug 2 2017 
Chrome Version: tip
OS: Linux x86-64

What steps will reproduce the problem?
(1) Build chrome with use_cfi_diag=true (this option is enabled on the Linux CFI trybot.)
(2) Chrome will not run unit tests that test CFI (in base/tools_sanity_unittest.cc) since CFI_ENFORCEMENT is not set for use_cfi_diag=true. Even if they did run it would not have caught that CFI was broken in that configuration because the tests are too limited: they would trigger the all-vtables check but not test a failure due to a valid-but-incorrect vtable.

Expected behavior: tests that thoroughly verify that cfi-vcall and both cfi-unrelated-cast and cfi-derived-cast are properly detected should run by default. This is currently blocked on rolling clang to pick-up the CFI fixes for use_cfi_diag=true.
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 15 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/37f1e35b81ccd9771c7e10f2e454a3a88249c5f5

commit 37f1e35b81ccd9771c7e10f2e454a3a88249c5f5
Author: Vlad Tsyrklevich <vtsyrklevich@google.com>
Date: Tue Aug 15 23:45:10 2017

Extend CFI unit tests

1) Enable CFI unit tests when use_cfi_diag=true (e.g. matches the Linux
CFI buildbot configuration)
2) Add a unit test to sanity check that a bad derived cast faults
3) Add a unit test to sanity check that a bad vcall (where the vtable is
a real vtable, not NULL) faults

Bug:  751313 
Change-Id: I29e7740be10ff572dc6d6973439b6b5cf167c8ca
Reviewed-on: https://chromium-review.googlesource.com/612505
Reviewed-by: Peter Collingbourne <pcc@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Peter Collingbourne <pcc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494607}
[modify] https://crrev.com/37f1e35b81ccd9771c7e10f2e454a3a88249c5f5/base/BUILD.gn
[modify] https://crrev.com/37f1e35b81ccd9771c7e10f2e454a3a88249c5f5/base/debug/stack_trace_posix.cc
[modify] https://crrev.com/37f1e35b81ccd9771c7e10f2e454a3a88249c5f5/base/tools_sanity_unittest.cc
[modify] https://crrev.com/37f1e35b81ccd9771c7e10f2e454a3a88249c5f5/build/config/sanitizers/BUILD.gn

Comment 2 by vtsyrklevich@chromium.org, Aug 18 2017

Status: Fixed (was: Started)
►

Sign in to add a comment