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

[George Balatsouras via Phabricator via llvm-commits]

gbalats added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:2498
+  }
+  DFSF.storePrimitiveShadowOrigin(SI.getPointerOperand(), Size, SI.getAlign(),
+                                  PrimitiveShadow, Origin, &SI);
----------------
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.


================
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]])
----------------
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).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97789