[PATCH] D103176: [dfsan] Add a flag about whether to propagate offset labels at gep

[George Balatsouras via Phabricator via llvm-commits]

gbalats added inline comments.


================
Comment at: compiler-rt/test/dfsan/gep.c:15
+  dfsan_set_label(1, &i, sizeof(i));
+  p = p + i;
+#ifdef PROP_OFFSET_LABELS
----------------



================
Comment at: compiler-rt/test/dfsan/gep.c:15
+  dfsan_set_label(1, &i, sizeof(i));
+  p = p + i;
+#ifdef PROP_OFFSET_LABELS
----------------
gbalats wrote:
> 
Can you add another case where p is not a pointer but is incremented in the same way to demonstrate that propagation always happens for non-pointers?


================
Comment at: compiler-rt/test/dfsan/gep.c:16
+  p = p + i;
+#ifdef PROP_OFFSET_LABELS
+  assert(dfsan_get_label(p) == 1);
----------------
You could remove the need for macros by just printing the result of dfsan_get_label and using different CHECK prefixes for the two run invocations above.


================
Comment at: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:2794-2796
+  DFSF.setShadow(&GEPI, DFSF.getShadow(GEPI.getOperand(0)));
+  if (DFSF.DFS.shouldTrackOrigins())
+    DFSF.setOrigin(&GEPI, DFSF.getOrigin(GEPI.getOperand(0)));
----------------
Use getPointerOperand instead and extract to variable.


================
Comment at: llvm/test/Instrumentation/DataFlowSanitizer/dont_combine_offset_labels_on_gep.ll:2
+; RUN: opt < %s -dfsan -dfsan-track-origins=1 -dfsan-combine-offset-labels-on-gep=false -dfsan-fast-8-labels=true -S | FileCheck %s --check-prefixes=CHECK,CHECK_ORIGIN
+; RUN: opt < %s -dfsan -dfsan-combine-offset-labels-on-gep=false -dfsan-fast-8-labels=true -S | FileCheck %s --check-prefixes=CHECK
+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"
----------------



================
Comment at: llvm/test/Instrumentation/DataFlowSanitizer/dont_combine_offset_labels_on_gep.ll:13
+  ; CHECK: @"dfs$gepop"
+  ; CHECK_ORIGIN: [[PO:%.*]] = load i32, i32* getelementptr inbounds ([200 x i32], [200 x i32]* @__dfsan_arg_origin_tls, i64 0, i64 0), align [[ALIGN_O:4]]
+  ; CHECK: [[PS:%.*]] = load i[[#SBITS]], i[[#SBITS]]* bitcast ([100 x i64]* @__dfsan_arg_tls to i[[#SBITS]]*), align [[ALIGN_S:2]]
----------------
nit: For PO and PS which are numeric, I find it easier to use `%[[#PO:]]` and then reference it by `%[[#PO]]`


================
Comment at: llvm/test/Instrumentation/DataFlowSanitizer/dont_combine_offset_labels_on_gep.ll:16
+  ; CHECK: %e = getelementptr [10 x [20 x i32]], [10 x [20 x i32]]* %p, i32 %a, i32 %b, i32 %c
+  ; CHECK: store i[[#SBITS]] [[PS]], i[[#SBITS]]* bitcast ([100 x i64]* @__dfsan_retval_tls to i[[#SBITS]]*), align [[ALIGN_S]]
+  ; CHECK_ORIGIN: store i32 [[PO]], i32* @__dfsan_retval_origin_tls, align [[ALIGN_O]]
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103176