[PATCH] D92440: [dfsan] Support passing non-i16 shadow values in TLS mode

Wed Dec 2 12:22:22 PST 2020

stephan.yichao.zhao marked an inline comment as done.
stephan.yichao.zhao added inline comments.

================
Comment at: llvm/test/Instrumentation/DataFlowSanitizer/arith.ll:8
+  ; CHECK-DAG: %[[ALABEL:.*]] = load [[ST:.*]], [[ST]]* bitcast ([[VT:\[.*\]]]* @__dfsan_arg_tls to [[ST]]*), align [[ALIGN:.*]]
+  ; CHECK-DAG: %[[BLABEL:.*]] = load [[ST]], [[ST]]* inttoptr (i64 add (i64 ptrtoint ([[VT]]* @__dfsan_arg_tls to i64), i64 8) to [[ST]]*), align [[ALIGN]]
+  ; CHECK: %[[UNION:.*]] = call zeroext [[ST]] @__dfsan_union([[ST]] zeroext %[[ALABEL]], [[ST]] zeroext %[[BLABEL]])
----------------
morehouse wrote:
> IIUC, this adds 8 to `__dfsan_arg_tls` to get `b`'s shadow.  This is because we have 8-byte alignment, right?
> 
> Why do we need 8 byte alignment?
This flows MSan's setting. https://github.com/llvm/llvm-project/commit/d2bd319adc7cf19d6e47d3caf3b6f508f299ea57 introduced 8-byte alignment for MSan. It sounds like sometimes the origin arg's alignment can be too large. This makes arg_tls run out of space.

================
Comment at: llvm/test/Instrumentation/DataFlowSanitizer/arith.ll:12
   ; CHECK: add i8
-  ; CHECK: store i16 %[[ADDLABEL]], i16* @__dfsan_retval_tls
+  ; CHECK: store [[ST]] %[[ADDLABEL]], [[ST]]* bitcast ([[VT]]* @__dfsan_retval_tls to [[ST]]*), align [[ALIGN]]
   ; CHECK: ret i8
----------------
morehouse wrote:
> The `ST` and `VT` names are a little confusing to me.  What do they mean?
renamed ST to SHADOWTYPE, and VT to ARGTLSTYPE

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92440