[PATCH] D109847: [DFSan] Add force_zero_label abilist option to DFSan. This can be used as a work-around for overtainting.

[Matt Morehouse via Phabricator via llvm-commits]

morehouse added a comment.

Please also update the documentation:  https://clang.llvm.org/docs/DataFlowSanitizer.html#abi-list



================
Comment at: compiler-rt/test/dfsan/Inputs/force_zero_abilist.txt:1
-fun:f=uninstrumented
+fun:function_to_force_zero=force_zero_labels
 
----------------
browneee wrote:
> morehouse wrote:
> > Why a separate abilist.txt?
> For separate test. Happy to combine them if you prefer.
Yes, let's combine them for simplicity.


================
Comment at: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:150
+// will make all writes and return values set zero labels.
+// The other supported annotations are "functional" and "discard",
 // which are described below under DataFlowSanitizer::WrapperKind.
----------------
Nit:  Could we make the distinction between uninstrumented and instrumented functions a little clearer?

I think we only use functional and discard with uninstrumented, while we only use force_zero_labels with instrumented.


================
Comment at: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:1212
+    DFSanFunction DFSF(*this, F, /*IsNativeABI=*/true,
+                       /*ForceZeroLabels*/ false);
     Function::arg_iterator ValAI = F->arg_begin(), ShadowAI = AI;
----------------
For consistency.


================
Comment at: llvm/test/Instrumentation/DataFlowSanitizer/Inputs/force_zero_abilist.txt:1
-fun:f=uninstrumented
+fun:function_to_force_zero=force_zero_labels
 
----------------
Let's also reuse the existing ABI list for llvm tests.


================
Comment at: llvm/test/Instrumentation/DataFlowSanitizer/force_zero.ll:9
+  ; CHECK: %[[#SHADOW_PTR:]] = inttoptr i64 %[[#SHADOW_XOR]] to i8*
+  ; CHECK: store i32 0
+  ; CHECK: store i32 %{{.*}}
----------------
Let's check that 0 is being stored specifically to `SHADOW_PTR`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109847