Skip to content

Commit

Permalink
Add more entries to UBSan blacklists, and only compile with a subset …
Browse files Browse the repository at this point in the history
…of UBSan sanitizers.

BUG=489901,174801
R=inferno@chromium.org
TBR=hclam@chromium.org

Review URL: https://codereview.chromium.org/1154593002

Cr-Commit-Position: refs/heads/master@{#331116}
  • Loading branch information
oliverchang authored and Commit bot committed May 22, 2015
1 parent f0f398a commit aa88885
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 96 deletions.
18 changes: 16 additions & 2 deletions build/common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@
# See http://clang.llvm.org/docs/UsersManual.html
'ubsan%': 0,
'ubsan_blacklist%': '<(PRODUCT_DIR)/../../tools/ubsan/blacklist.txt',
'ubsan_vptr_blacklist%': '<(PRODUCT_DIR)/../../tools/ubsan/vptr_blacklist.txt',

# Enable building with UBsan's vptr (Clang's -fsanitize=vptr option).
# -fsanitize=vptr only works with clang, but ubsan_vptr=1 implies clang=1
Expand Down Expand Up @@ -1154,6 +1155,7 @@
'tsan_blacklist%': '<(tsan_blacklist)',
'ubsan%': '<(ubsan)',
'ubsan_blacklist%': '<(ubsan_blacklist)',
'ubsan_vptr_blacklist%': '<(ubsan_vptr_blacklist)',
'ubsan_vptr%': '<(ubsan_vptr)',
'use_instrumented_libraries%': '<(use_instrumented_libraries)',
'use_prebuilt_instrumented_libraries%': '<(use_prebuilt_instrumented_libraries)',
Expand Down Expand Up @@ -4347,7 +4349,19 @@
'target_conditions': [
['_toolset=="target"', {
'cflags': [
'-fsanitize=undefined',
# FIXME: work on enabling more flags and getting rid of false
# positives. http://crbug.com/174801.
'-fsanitize=bounds',
'-fsanitize=float-divide-by-zero',
'-fsanitize=integer-divide-by-zero',
'-fsanitize=null',
'-fsanitize=object-size',
'-fsanitize=return',
'-fsanitize=returns-nonnull-attribute',
'-fsanitize=shift-exponent',
'-fsanitize=signed-integer-overflow',
'-fsanitize=unreachable',
'-fsanitize=vla-bound',
'-fsanitize-blacklist=<(ubsan_blacklist)',
# Employ the experimental PBQP register allocator to avoid
# slow compilation on files with too many basic blocks.
Expand Down Expand Up @@ -4377,7 +4391,7 @@
['_toolset=="target"', {
'cflags': [
'-fsanitize=vptr',
'-fsanitize-blacklist=<(ubsan_blacklist)',
'-fsanitize-blacklist=<(ubsan_vptr_blacklist)',
],
'cflags_cc!': [
'-fno-rtti',
Expand Down
2 changes: 2 additions & 0 deletions third_party/yasm/yasm.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@
'target_defaults': {
# Silence warnings in libc++ builds (C code doesn't need this flag).
'ldflags!': [ '-stdlib=libc++', ],
# https://crbug.com/489901
'cflags!': [ '-fsanitize=bounds' ],
},
'targets': [
{
Expand Down
146 changes: 52 additions & 94 deletions tools/ubsan/blacklist.txt
Original file line number Diff line number Diff line change
@@ -1,113 +1,71 @@
#############################################################################
# UBSan vptr blacklist.
# Function and type based blacklisting use a mangled name, and it is especially
# tricky to represent C++ types. For now, any possible changes by name manglings
# are simply represented as wildcard expressions of regexp, and thus it might be
# over-blacklisted.
# UBSan blacklist.

#############################################################################
# Identical layouts.
# If base and derived classes have identifical memory layouts (i.e., the same
# object size) and both have no virtual functions, we blacklist them as there
# would be not much security implications.

fun:*LifecycleNotifier*addObserver*
fun:*LifecycleNotifier*removeObserver*
fun:*toWebInputElement*
type:*base*MessageLoopForIO*
type:*BlockRefType*
type:*SkAutoTUnref*
type:*WDResult*
type:*ExecutionContext*
type:*WebInputElement*
type:*WebFormControlElement*

# Avoid identical layout cases for 86 different classes in InspectorTypeBuilder,
# all of which are guarded using COMPILER_ASSERT on the object size. Two more
# types are also blacklisted due to the template class (JSONArray <-> Array<T>).

src:*InspectorTypeBuilder.h*
type:*TypeBuilder*
type:*JSONArray*
# YASM does some funny things that UBsan doesn't like.
# https://crbug.com/489901
src:*/third_party/yasm/*

#############################################################################
# Base class's constructor accesses a derived class's member.

fun:*DoublyLinkedListNode*
type:*content*WebUIExtensionData*

# RenderFrameObserverTracker<T>::RenderFrameObserverTracker()
fun:*content*RenderFrameObserverTracker*RenderFrame*

# RenderViewObserverTracker<T>::RenderViewObserverTracker()
fun:*content*RenderViewObserverTracker*RenderView*
# V8 gives too many false positives. Ignore them for now.
src:*/v8/*

#############################################################################
# Base class's destructor accesses a derived class.

fun:*DatabaseContext*contextDestroyed*

# FIXME: Cannot handle template function LifecycleObserver<>::setContext,
# so exclude source file for now.
src:*LifecycleObserver.h*
# Ignore system libraries.
src:*/usr/*

#############################################################################
# static_cast into itself in the constructor.

fun:*RefCountedGarbageCollected*makeKeepAlive*
fun:*ThreadSafeRefCountedGarbageCollected*makeKeepAlive*
# V8 UBsan supressions, commented out for now since we are ignorning v8
# completely.
# fun:*v8*internal*FastD2I*
# fun:*v8*internal*ComputeIntegerHash*
# fun:*v8*internal*ComputeLongHash*
# fun:*v8*internal*ComputePointerHash*
# src:*/v8/src/base/bits.cc
# src:*/v8/src/base/functional.cc
# Undefined behaviour (integer overflow) is expected but ignored in this
# function.
# fun:*JsonParser*ParseJsonNumber*

# Runtime numeric functions.
# src:*/v8/src/runtime/runtime-numbers.cc

# Shifts of negative numbers
# fun:*v8*internal*HPositionInfo*TagPosition*
# fun:*v8*internal*Range*Shl*
# fun:*v8*internal*RelocInfoWriter*WriteTaggedData*

#############################################################################
# Accessing data in destructors where the class has virtual inheritances.

type:*content*RenderWidgetHost*

# Match mangled name for X::~X().
fun:*content*RenderThreadImplD*
fun:*content*RenderViewHostImplD*
fun:*content*UtilityThreadImplD*
# Undefined arithmetic that can be safely ignored.
src:*/third_party/WebKit/Source/wtf/SaturatedArithmetic.h
src:*/ppapi/shared_impl/id_assignment.h

#############################################################################
# Using raw pointer values.
#
# A raw pointer value (16) is used to infer the field offset by
# GOOGLE_PROTOBUF_GENERATED_MESSAGE_FIELD_OFFSET.

src:*/third_party/protobuf/src/google/protobuf/compiler/plugin.pb.cc
src:*/third_party/protobuf/src/google/protobuf/compiler/cpp/cpp_message.cc
src:*/third_party/protobuf/src/google/protobuf/descriptor.pb.cc
# ICU supressions. Mostly hash functions where integer overflow is OK.
fun:*hashEntry*
fun:*LocaleCacheKey*hashCode*
fun:*google*protobuf*hash*
fun:*(hash|Hash)*

#############################################################################
# Avoid link errors.
# Ubsan vptr needs typeinfo on the target class, but it looks like typeinfo is
# not avaiable if the class is not exported. For now, simply blacklisted to
# avoid link errors; e.g., undefined reference to 'typeinfo for [CLASS_NAME]'.

# obj/ppapi/libppapi_proxy.a(obj/ppapi/proxy/ppapi_proxy.proxy_channel.o):../../ppapi/proxy/proxy_channel.cc:__unnamed_53: error: undefined reference to 'typeinfo for IPC::TestSink'
src:*/ppapi/proxy/proxy_channel.cc

# obj/chrome/libbrowser.a(obj/chrome/browser/net/browser.predictor.o):../../chrome/browser/net/predictor.cc:__unnamed_577: error: undefined reference to 'typeinfo for ProxyAdvisor'
src:*/chrome/browser/net/predictor.cc

# obj/third_party/pdfium/libfpdfapi.a(obj/third_party/pdfium/core/src/fpdfapi/fpdf_render/fpdfapi.fpdf_render_text.o):../../third_party/pdfium/core/src/fpdfapi/fpdf_render/:__unnamed_360: error: undefined reference to 'typeinfo for CPDF_InlineImages'
src:*/third_party/pdfium/core/src/fpdfapi/fpdf_render/fpdf_render_text.cpp

# obj/third_party/libwebm/libwebm.a(obj/third_party/libwebm/source/libwebm.mkvmuxer.o)(.data.rel..L__unnamed_2+0x18): error: undefined reference to 'typeinfo for mkvparser::IMkvReader'
src:*/third_party/libwebm/source/mkvmuxer.cpp
# Bounds blacklist.
# Array at the end of struct pattern:
# Maybe UBSan itself can be improved here?
# e.g.
# struct blah {
# int a;
# char foo[2]; // not actually 2
# }
src:*/net/disk_cache/blockfile/backend_impl.cc
src:*/net/disk_cache/blockfile/entry_impl.cc
src:*/third_party/icu/source/common/rbbi.cpp
src:*/third_party/icu/source/common/rbbitblb.cpp
src:*/third_party/icu/source/common/ucmndata.c

#############################################################################
# UBSan seems to be emit false positives when virtual base classes are
# involved, see e.g. crbug.com/448102.

type:*v8*internal*OFStream*
# Delete in destructor on a this where this == nullptr
fun:*re2*RegexpD*

#############################################################################
# UBsan is unable to handle static_cast<A*>(nullptr) and crashes on SIGSEGV.
#

# static_cast<StartPageService*> in StartPageServiceFactory::GetForProfile.
type:*StartPageService*

# Remove once function attribute level blacklisting is implemented.
# See crbug.com/476063.
fun:*forbidGCDuringConstruction*
# Harmless float division by zero.
fun:*RendererFrameManager*CullUnlockedFrames*

0 comments on commit aa88885

Please sign in to comment.