[PATCH] [UBSAN] Suppress an error report with given type information

Alexey Samsonov vonosmas at gmail.com
Thu Jul 31 12:16:49 PDT 2014


================
Comment at: lib/ubsan/ubsan_diag.cc:21
@@ -20,1 +20,3 @@
+#include "sanitizer_common/sanitizer_suppressions.h"
+#include "sanitizer_common/sanitizer_placement_new.h"
 #include <stdio.h>
----------------
Revise new #includes you need.

================
Comment at: lib/ubsan/ubsan_diag.cc:319
@@ +318,3 @@
+void __ubsan::InitializeSuppressions() {
+  if (common_flags()->suppressions[0] == '\0')
+    return;
----------------
I'd prefer to skip check for suppressions common flag and assume that suppression context is always properly initialized (but can be empty).

================
Comment at: lib/ubsan/ubsan_diag.cc:323
@@ +322,3 @@
+  // Check if SuppressionContext is initilized by other sanitizers.
+  if (!SuppressionContext::GetOrNull())
+    SuppressionContext::Init();
----------------
just introduce SuppressionContext::InitIfNecessary()

================
Comment at: lib/ubsan/ubsan_diag.cc:327
@@ +326,3 @@
+
+bool __ubsan::MatchSuppressions(const char *Name, SuppressionType Type) {
+  Suppression *s;
----------------
As you're encapsulating access to SuppressionContext, you may provide a superior interface, so that callers of MatchSuppressions() won't have to know about the enum "SuppressionVptrCheck". E.g. you can introduce a simple function
  bool __ubsan::TypeIsSuppressed(const char *TypeName);

================
Comment at: lib/ubsan/ubsan_diag.cc:330
@@ +329,3 @@
+
+#if !SANITIZER_CAN_USE_PREINIT_ARRAY
+  InitIfNecessary();
----------------
you can plain if() here:
  if (!SANITIZER_CAN_USE_PREINIT_ARRAY)
    InitIfNecessary();
please add a comment describing why you need this.

================
Comment at: lib/ubsan/ubsan_diag.cc:334
@@ +333,3 @@
+
+  if (common_flags()->suppressions[0] != '\0' &&
+      SuppressionContext::Get()->Match(Name, Type, &s))
----------------
Collapse
  if (condition)
    return true;
  return false;

to
  return condition;

================
Comment at: lib/ubsan/ubsan_flags.cc:17
@@ -16,2 +16,3 @@
 #include "sanitizer_common/sanitizer_flags.h"
+#include <stdio.h>
 
----------------
accidental include?

================
Comment at: lib/ubsan/ubsan_handlers_cxx.cc:39
@@ +38,3 @@
+  // Check if an error report is suppressed.
+  if (MatchSuppressions(DTI.getMostDerivedTypeName(), SuppressionVptrCheck))
+    return;
----------------
you can save DTI.getMostDerivedTypeName() in a variable (it's called several times in this function).

================
Comment at: test/ubsan/TestCases/TypeCheck/vptr.cpp:15
@@ +14,3 @@
+// RUN: (echo "vptr_check:S"; echo "vptr_check:T"; echo "vptr_check:U") > %t.supp
+// RUN: (echo " " ; ASAN_OPTIONS=suppressions=%t.supp UBSAN_OPTIONS=suppressions=%t.supp %run %t mS 2>&1) | FileCheck %s --check-prefix=CHECK-SUPPRESS
+// RUN: (echo " " ; ASAN_OPTIONS=suppressions=%t.supp UBSAN_OPTIONS=suppressions=%t.supp %run %t fS 2>&1) | FileCheck %s --check-prefix=CHECK-SUPPRESS
----------------
remove stray (echo " ")

http://reviews.llvm.org/D4702






More information about the llvm-commits mailing list