[PATCH] D97789: [dfsan] Propagate origin tracking at store

[stephan.yichao.zhao via Phabricator via llvm-commits]

stephan.yichao.zhao added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:2498
+  }
+  DFSF.storePrimitiveShadowOrigin(SI.getPointerOperand(), Size, SI.getAlign(),
+                                  PrimitiveShadow, Origin, &SI);
----------------
gbalats wrote:
> For this function, Origin is really an optional argument (as it's just a nullptr when origin tracking is not enabled).
> I think that the previous name (storePrimitiveShadow) is clearer in that respect as it better captures its main purpose.
This name is consistent with other function names that operate on both shadow and origins.
If we only use Shadow but not Origin, following other function names, one may assume it does not do anything with Origin: storeZeroPrimitiveShadow is a case.


================
Comment at: llvm/test/Instrumentation/DataFlowSanitizer/origin_ldst.ll:385-396
+  ; CHECK: @"dfs$store64_align2"
+  ; CHECK: [[AO:%.*]] = load i32, i32* getelementptr inbounds ([200 x i32], [200 x i32]* @__dfsan_arg_origin_tls, i64 0, i64 1), align 4
+  ; CHECK: [[AS:%.*]] = load i[[#SBITS]], i[[#SBITS]]* inttoptr (i64 add (i64 ptrtoint ([100 x i64]* @__dfsan_arg_tls to i64), i64 2) to i[[#SBITS]]*), align [[#SBYTES]]
+  ; CHECK: %_dfscmp = icmp ne i[[#SBITS]] [[AS]], 0
+  ; CHECK: br i1 %_dfscmp, label %[[L1:.*]], label %[[L2:.*]],
+  ; CHECK: [[L1]]:
+  ; CHECK: [[NO:%.*]] = call i32 @__dfsan_chain_origin(i32 [[AO]])
----------------
gbalats wrote:
> Are these expectations exhaustive? Or are there missing lines? In the first case, adding the -NEXT part should be straightforward.
> Also, my personal preference is to use numeric substitution blocks for registers (e.g., `[[#R:]]`) if the names do not increase readability (that's just a nit though).
Added NEXT for most cases.

These names help read the IR to understand which are origins and shadows, and operations on them at IR level. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97789