morehouse updated this revision to Diff 280286.
morehouse marked 6 inline comments as done.
morehouse added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

- Add documentation.
- Remove fast16labels runtime flag.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84371/new/

https://reviews.llvm.org/D84371

Files:
  clang/docs/DataFlowSanitizer.rst
  compiler-rt/lib/dfsan/dfsan.cpp
  compiler-rt/lib/dfsan/dfsan_flags.inc
  compiler-rt/lib/dfsan/done_abilist.txt
  compiler-rt/test/dfsan/fast16labels.c
  llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
  llvm/test/Instrumentation/DataFlowSanitizer/fast16labels.ll

Index: llvm/test/Instrumentation/DataFlowSanitizer/fast16labels.ll
===================================================================
--- /dev/null
+++ llvm/test/Instrumentation/DataFlowSanitizer/fast16labels.ll
@@ -0,0 +1,100 @@
+; Test that fast-16-labels mode uses inline ORs rather than calling
+; __dfsan_union or __dfsan_union_load.
+; RUN: opt < %s -dfsan -fast-16-labels -S | FileCheck %s --implicit-check-not="call{{.*}}__dfsan_union"
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i8 @add(i8 %a, i8 %b) {
+  ; CHECK-LABEL: define i8 @"dfs$add"
+  ; CHECK-DAG: %[[ALABEL:.*]] = load{{.*}}__dfsan_arg_tls, i64 0, i64 0
+  ; CHECK-DAG: %[[BLABEL:.*]] = load{{.*}}__dfsan_arg_tls, i64 0, i64 1
+  ; CHECK: %[[ADDLABEL:.*]] = or i16 %[[ALABEL]], %[[BLABEL]]
+  ; CHECK: add i8
+  ; CHECK: store i16 %[[ADDLABEL]], i16* @__dfsan_retval_tls
+  ; CHECK: ret i8
+  %c = add i8 %a, %b
+  ret i8 %c
+}
+
+define i8 @load8(i8* %p) {
+  ; CHECK-LABEL: define i8 @"dfs$load8"
+  ; CHECK: load i16, i16*
+  ; CHECK: ptrtoint i8* {{.*}} to i64
+  ; CHECK: and i64
+  ; CHECK: mul i64
+  ; CHECK: inttoptr i64
+  ; CHECK: load i16, i16*
+  ; CHECK: or i16
+  ; CHECK: load i8, i8*
+  ; CHECK: store i16 {{.*}} @__dfsan_retval_tls
+  ; CHECK: ret i8
+
+  %a = load i8, i8* %p
+  ret i8 %a
+}
+
+define i16 @load16(i16* %p) {
+  ; CHECK-LABEL: define i16 @"dfs$load16"
+  ; CHECK: ptrtoint i16*
+  ; CHECK: and i64
+  ; CHECK: mul i64
+  ; CHECK: inttoptr i64 {{.*}} i16*
+  ; CHECK: getelementptr i16
+  ; CHECK: load i16, i16*
+  ; CHECK: load i16, i16*
+  ; CHECK: or i16
+  ; CHECK: or i16
+  ; CHECK: load i16, i16*
+  ; CHECK: store {{.*}} @__dfsan_retval_tls
+  ; CHECK: ret i16
+
+  %a = load i16, i16* %p
+  ret i16 %a
+}
+
+define i32 @load32(i32* %p) {
+  ; CHECK-LABEL: define i32 @"dfs$load32"
+  ; CHECK: ptrtoint i32*
+  ; CHECK: and i64
+  ; CHECK: mul i64
+  ; CHECK: inttoptr i64 {{.*}} i16*
+  ; CHECK: bitcast i16* {{.*}} i64*
+  ; CHECK: load i64, i64*
+  ; CHECK: lshr i64 {{.*}}, 32
+  ; CHECK: or i64
+  ; CHECK: lshr i64 {{.*}}, 16
+  ; CHECK: or i64
+  ; CHECK: trunc i64 {{.*}} i16
+  ; CHECK: or i16
+  ; CHECK: load i32, i32*
+  ; CHECK: store i16 {{.*}} @__dfsan_retval_tls
+  ; CHECK: ret i32
+
+  %a = load i32, i32* %p
+  ret i32 %a
+}
+
+define i64 @load64(i64* %p) {
+  ; CHECK-LABEL: define i64 @"dfs$load64"
+  ; CHECK: ptrtoint i64*
+  ; CHECK: and i64
+  ; CHECK: mul i64
+  ; CHECK: inttoptr i64 {{.*}} i16*
+  ; CHECK: bitcast i16* {{.*}} i64*
+  ; CHECK: load i64, i64*
+  ; CHECK: getelementptr i64, i64* {{.*}}, i64 1
+  ; CHECK: load i64, i64*
+  ; CHECK: or i64
+  ; CHECK: lshr i64 {{.*}}, 32
+  ; CHECK: or i64
+  ; CHECK: lshr i64 {{.*}}, 16
+  ; CHECK: or i64
+  ; CHECK: trunc i64 {{.*}} i16
+  ; CHECK: or i16
+  ; CHECK: load i64, i64*
+  ; CHECK: store i16 {{.*}} @__dfsan_retval_tls
+  ; CHECK: ret i64
+
+  %a = load i64, i64* %p
+  ret i64 %a
+}
Index: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
@@ -91,6 +91,7 @@
 #include "llvm/Transforms/Instrumentation.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/Local.h"
+#include "llvm/Transforms/Utils/ModuleUtils.h"
 #include <algorithm>
 #include <cassert>
 #include <cstddef>
@@ -176,6 +177,14 @@
     cl::desc("Insert calls to __dfsan_*_callback functions on data events."),
     cl::Hidden, cl::init(false));
 
+// Use a distinct bit for each base label, enabling faster unions with less
+// instrumentation.  Limits the max number of base labels to 16.
+static cl::opt<bool> ClFast16Labels(
+    "fast-16-labels",
+    cl::desc("Use more efficient instrumentation, limiting the number of "
+             "labels to 16."),
+    cl::Hidden, cl::init(false));
+
 static StringRef GetGlobalTypeString(const GlobalValue &G) {
   // Types of GlobalVariables are always pointer types.
   Type *GType = G.getValueType();
@@ -359,6 +368,7 @@
   FunctionType *DFSanVarargWrapperFnTy;
   FunctionType *DFSanLoadStoreCmpCallbackFnTy;
   FunctionType *DFSanMemTransferCallbackFnTy;
+  FunctionType *DFSanUseFast16LabelsFnTy;
   FunctionCallee DFSanUnionFn;
   FunctionCallee DFSanCheckedUnionFn;
   FunctionCallee DFSanUnionLoadFn;
@@ -370,6 +380,7 @@
   FunctionCallee DFSanStoreCallbackFn;
   FunctionCallee DFSanMemTransferCallbackFn;
   FunctionCallee DFSanCmpCallbackFn;
+  FunctionCallee DFSanUseFast16LabelsFn;
   MDNode *ColdCallWeights;
   DFSanABIList ABIList;
   DenseMap<Value *, Function *> UnwrappedFnMap;
@@ -612,6 +623,8 @@
   DFSanMemTransferCallbackFnTy =
       FunctionType::get(Type::getVoidTy(*Ctx), DFSanMemTransferCallbackArgs,
                         /*isVarArg=*/false);
+  DFSanUseFast16LabelsFnTy =
+      FunctionType::get(Type::getVoidTy(*Ctx), {}, /*isVarArg=*/false);
 
   if (GetArgTLSPtr) {
     Type *ArgTLSTy = ArrayType::get(ShadowTy, 64);
@@ -793,6 +806,9 @@
       Mod->getOrInsertFunction("__dfsan_nonzero_label", DFSanNonzeroLabelFnTy);
   DFSanVarargWrapperFn = Mod->getOrInsertFunction("__dfsan_vararg_wrapper",
                                                   DFSanVarargWrapperFnTy);
+
+  DFSanUseFast16LabelsFn = Mod->getOrInsertFunction("dfsan_use_fast16labels",
+                                                    DFSanUseFast16LabelsFnTy);
 }
 
 // Initializes event callback functions and declare them in the module
@@ -1053,6 +1069,24 @@
     }
   }
 
+  if (ClFast16Labels) {
+    // Call dfsan_use_fast16labels() during preinit to enable runtime support
+    // for fast16labels mode.
+    auto *UseFast16LabelsInit =
+        new GlobalVariable(M, PointerType::getUnqual(DFSanUseFast16LabelsFnTy),
+                           /*constant=*/true, GlobalVariable::PrivateLinkage,
+                           cast<Constant>(DFSanUseFast16LabelsFn.getCallee()),
+                           "__dfsan_fast16labels_preinit");
+    UseFast16LabelsInit->setSection(".preinit_array");
+
+    // Dedup with comdat
+    if (Triple(M.getTargetTriple()).supportsCOMDAT())
+      UseFast16LabelsInit->setComdat(
+          M.getOrInsertComdat("dfsan_fast16labels.module_preinit"));
+
+    appendToUsed(M, UseFast16LabelsInit);
+  }
+
   return Changed || !FnsToInstrument.empty() ||
          M.global_size() != InitialGlobalSize || M.size() != InitialModuleSize;
 }
@@ -1179,7 +1213,10 @@
     return CCS.Shadow;
 
   IRBuilder<> IRB(Pos);
-  if (AvoidNewBlocks) {
+  if (ClFast16Labels) {
+    CCS.Block = Pos->getParent();
+    CCS.Shadow = IRB.CreateOr(V1, V2);
+  } else if (AvoidNewBlocks) {
     CallInst *Call = IRB.CreateCall(DFS.DFSanCheckedUnionFn, {V1, V2});
     Call->addAttribute(AttributeList::ReturnIndex, Attribute::ZExt);
     Call->addParamAttr(0, Attribute::ZExt);
@@ -1289,6 +1326,30 @@
         IRB.CreateAlignedLoad(DFS.ShadowTy, ShadowAddr1, ShadowAlign), Pos);
   }
   }
+
+  if (ClFast16Labels && Size % (64 / DFS.ShadowWidthBits) == 0) {
+    // First OR all the WideShadows, then OR individual shadows within the
+    // combined WideShadow.  This is fewer instructions than ORing shadows
+    // individually.
+    IRBuilder<> IRB(Pos);
+    Value *WideAddr =
+        IRB.CreateBitCast(ShadowAddr, Type::getInt64PtrTy(*DFS.Ctx));
+    Value *CombinedWideShadow =
+        IRB.CreateAlignedLoad(IRB.getInt64Ty(), WideAddr, ShadowAlign);
+    for (uint64_t Ofs = 64 / DFS.ShadowWidthBits; Ofs != Size;
+         Ofs += 64 / DFS.ShadowWidthBits) {
+      WideAddr = IRB.CreateGEP(Type::getInt64Ty(*DFS.Ctx), WideAddr,
+                               ConstantInt::get(DFS.IntptrTy, 1));
+      Value *NextWideShadow =
+          IRB.CreateAlignedLoad(IRB.getInt64Ty(), WideAddr, ShadowAlign);
+      CombinedWideShadow = IRB.CreateOr(CombinedWideShadow, NextWideShadow);
+    }
+    for (unsigned Width = 32; Width >= DFS.ShadowWidthBits; Width >>= 1) {
+      Value *ShrShadow = IRB.CreateLShr(CombinedWideShadow, Width);
+      CombinedWideShadow = IRB.CreateOr(CombinedWideShadow, ShrShadow);
+    }
+    return IRB.CreateTrunc(CombinedWideShadow, DFS.ShadowTy);
+  }
   if (!AvoidNewBlocks && Size % (64 / DFS.ShadowWidthBits) == 0) {
     // Fast path for the common case where each byte has identical shadow: load
     // shadow 64 bits at a time, fall out to a __dfsan_union_load call if any
Index: compiler-rt/test/dfsan/fast16labels.c
===================================================================
--- compiler-rt/test/dfsan/fast16labels.c
+++ compiler-rt/test/dfsan/fast16labels.c
@@ -1,13 +1,13 @@
-// RUN: %clang_dfsan %s -o %t
-// RUN: DFSAN_OPTIONS=fast16labels=1 %run %t
-// RUN: DFSAN_OPTIONS=fast16labels=1 not %run %t dfsan_create_label 2>&1 \
+// RUN: %clang_dfsan %s -mllvm -fast-16-labels -o %t
+// RUN: %run %t
+// RUN: not %run %t dfsan_create_label 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CREATE-LABEL
-// RUN: DFSAN_OPTIONS=fast16labels=1 not %run %t dfsan_get_label_info 2>&1 \
+// RUN: not %run %t dfsan_get_label_info 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=GET-LABEL-INFO
-// RUN: DFSAN_OPTIONS=fast16labels=1 not %run %t dfsan_has_label_with_desc \
-// RUN:   2>&1 | FileCheck %s --check-prefix=HAS-LABEL-WITH-DESC
+// RUN: not %run %t dfsan_has_label_with_desc 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=HAS-LABEL-WITH-DESC
 //
-// Tests DFSAN_OPTIONS=fast16labels=1
+// Tests fast16labels mode.
 //
 #include <sanitizer/dfsan_interface.h>
 
Index: compiler-rt/lib/dfsan/done_abilist.txt
===================================================================
--- compiler-rt/lib/dfsan/done_abilist.txt
+++ compiler-rt/lib/dfsan/done_abilist.txt
@@ -28,6 +28,8 @@
 fun:dfsan_set_write_callback=custom
 fun:dfsan_flush=uninstrumented
 fun:dfsan_flush=discard
+fun:dfsan_use_fast16labels=uninstrumented
+fun:dfsan_use_fast16labels=discard
 
 ###############################################################################
 # glibc
Index: compiler-rt/lib/dfsan/dfsan_flags.inc
===================================================================
--- compiler-rt/lib/dfsan/dfsan_flags.inc
+++ compiler-rt/lib/dfsan/dfsan_flags.inc
@@ -29,7 +29,3 @@
 DFSAN_FLAG(const char *, dump_labels_at_exit, "", "The path of the file where "
                                                   "to dump the labels when the "
                                                   "program terminates.")
-DFSAN_FLAG(bool, fast16labels, false,
-    "Enables experimental mode where DFSan supports only 16 power-of-2 labels "
-    "(1, 2, 4, 8, ... 32768) and the label union is computed as a bit-wise OR."
-)
Index: compiler-rt/lib/dfsan/dfsan.cpp
===================================================================
--- compiler-rt/lib/dfsan/dfsan.cpp
+++ compiler-rt/lib/dfsan/dfsan.cpp
@@ -18,15 +18,16 @@
 // prefixed __dfsan_.
 //===----------------------------------------------------------------------===//
 
+#include "dfsan/dfsan.h"
+
 #include "sanitizer_common/sanitizer_atomic.h"
 #include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_file.h"
-#include "sanitizer_common/sanitizer_flags.h"
 #include "sanitizer_common/sanitizer_flag_parser.h"
+#include "sanitizer_common/sanitizer_flags.h"
+#include "sanitizer_common/sanitizer_internal_defs.h"
 #include "sanitizer_common/sanitizer_libc.h"
 
-#include "dfsan/dfsan.h"
-
 using namespace __dfsan;
 
 typedef atomic_uint16_t atomic_dfsan_label;
@@ -37,6 +38,9 @@
 static atomic_dfsan_label __dfsan_last_label;
 static dfsan_label_info __dfsan_label_info[kNumLabels];
 
+// True if dfsan_use_fast16labels() was called.
+static bool fast16labels = false;
+
 Flags __dfsan::flags_data;
 
 SANITIZER_INTERFACE_ATTRIBUTE THREADLOCAL dfsan_label __dfsan_retval_tls;
@@ -164,11 +168,16 @@
   Die();
 }
 
+// Configures the DFSan runtime to use fast16labels mode.
+extern "C" SANITIZER_INTERFACE_ATTRIBUTE void dfsan_use_fast16labels() {
+  fast16labels = true;
+}
+
 // Resolves the union of two unequal labels.  Nonequality is a precondition for
 // this function (the instrumentation pass inlines the equality test).
 extern "C" SANITIZER_INTERFACE_ATTRIBUTE
 dfsan_label __dfsan_union(dfsan_label l1, dfsan_label l2) {
-  if (flags().fast16labels)
+  if (fast16labels)
     return l1 | l2;
   DCHECK_NE(l1, l2);
 
@@ -259,7 +268,7 @@
 
 extern "C" SANITIZER_INTERFACE_ATTRIBUTE
 dfsan_label dfsan_create_label(const char *desc, void *userdata) {
-  if (flags().fast16labels)
+  if (fast16labels)
     ReportUnsupportedFast16("dfsan_create_label");
   dfsan_label label =
       atomic_fetch_add(&__dfsan_last_label, 1, memory_order_relaxed) + 1;
@@ -319,14 +328,14 @@
 
 extern "C" SANITIZER_INTERFACE_ATTRIBUTE
 const struct dfsan_label_info *dfsan_get_label_info(dfsan_label label) {
-  if (flags().fast16labels)
+  if (fast16labels)
     ReportUnsupportedFast16("dfsan_get_label_info");
   return &__dfsan_label_info[label];
 }
 
 extern "C" SANITIZER_INTERFACE_ATTRIBUTE int
 dfsan_has_label(dfsan_label label, dfsan_label elem) {
-  if (flags().fast16labels)
+  if (fast16labels)
     return label & elem;
   if (label == elem)
     return true;
@@ -340,7 +349,7 @@
 
 extern "C" SANITIZER_INTERFACE_ATTRIBUTE dfsan_label
 dfsan_has_label_with_desc(dfsan_label label, const char *desc) {
-  if (flags().fast16labels)
+  if (fast16labels)
     ReportUnsupportedFast16("dfsan_has_label_with_desc");
   const dfsan_label_info *info = dfsan_get_label_info(label);
   if (info->l1 != 0) {
@@ -361,7 +370,7 @@
 
 extern "C" SANITIZER_INTERFACE_ATTRIBUTE void
 dfsan_dump_labels(int fd) {
-  if (flags().fast16labels)
+  if (fast16labels)
     return;
 
   dfsan_label last_label =
Index: clang/docs/DataFlowSanitizer.rst
===================================================================
--- clang/docs/DataFlowSanitizer.rst
+++ clang/docs/DataFlowSanitizer.rst
@@ -174,6 +174,57 @@
     return 0;
   }
 
+fast16labels mode
+=================
+
+If you need 16 or fewer labels, you can use fast16labels instrumentation for
+less CPU and code size overhead.  To use fast16labels instrumentation, you'll
+need to specify `-fsanitize=dataflow -mllvm -fast-16-labels` in your compile and
+link commands and use a modified API for creating and managing labels.
+
+In fast16labels mode, base labels are simply 16-bit unsigned integers that are
+powers of 2 (i.e. 1, 2, 4, 8, ..., 32768), and union labels are created by ORing
+base labels.  In this mode DFSan does not manage any label metadata, so the
+functions `dfsan_create_label`, `dfsan_get_label_info`,
+`dfsan_has_label_with_desc`, `dfsan_get_label_count`, and `dfsan_dump_labels`
+are unsupported.  Instead of using them, the user should maintain any necessary
+metadata about base labels themselves.
+
+For example:
+
+.. code-block:: c++
+
+  #include <sanitizer/dfsan_interface.h>
+  #include <assert.h>
+
+  int main(void) {
+    int i = 1;
+    int j = 2;
+    int k = 3;
+    dfsan_label i_label = 1;
+    dfsan_label j_label = 2;
+    dfsan_label k_label = 4;
+    dfsan_set_label(i_label, &i, sizeof(i));
+    dfsan_set_label(j_label, &j, sizeof(j));
+    dfsan_set_label(k_label, &k, sizeof(k));
+
+    dfsan_label ij_label = dfsan_get_label(i + j);
+
+    assert(ij_label & i_label);  // ij_label has i_label
+    assert(ij_label & j_label);  // ij_label has j_label
+    assert(!(ij_label & k_label));  // ij_label doesn't have k_label
+    assert(ij_label == 3)  // Verifies all of the above
+
+    dfsan_label ijk_label = dfsan_get_label(i + j + k);
+
+    assert(ijk_label & i_label);  // ijk_label has i_label
+    assert(ijk_label & j_label);  // ijk_label has j_label
+    assert(ijk_label & k_label);  // ijk_label has k_label
+    assert(ijk_label == 7);  // Verifies all of the above
+
+    return 0;
+  }
+
 Current status
 ==============
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to