[PATCH] D101584: [dfsan] Fix origin tracking for fast8

Fri Apr 30 14:52:16 PDT 2021

stephan.yichao.zhao accepted this revision.
stephan.yichao.zhao added inline comments.
This revision is now accepted and ready to land.

================
Comment at: llvm/test/Instrumentation/DataFlowSanitizer/origin_load.ll:258
   ; CHECK-NEXT:            %[[#WIDE_SHADOW:]] = load i64, i64* %[[#WIDE_SHADOW_PTR]], align [[#SBYTES]]
+  ; CHECK8-NEXT:           %[[#WIDE_SHADOW_LO:]] = shl i64 %[[#WIDE_SHADOW]], 32
+  ; CHECK8-NEXT:           %[[#ORIGIN_PTR2:]] = getelementptr i32, i32* %[[#ORIGIN_PTR]], i64 1
----------------
gbalats wrote:
> stephan.yichao.zhao wrote:
> > If WIDE_SHADOW_LO is named to be WIDE_SHADOW_1; and WIDE_SHADOW is named WIDE_SHADOW_12; and ORIGIN is named ORIGIN1; 
> >  and we do not reuse ORIGIN, but named new assignment to ORIGIN as ORIGIN23 if it is a select from ORIGIN2 and ORIGIN3, it helps read the code. 
> I think if we use WIDE_SHADOW_1 instead of _LO it will be difficult to differentiate between the different wide shadows and their left-shifted counterparts. The naming pattern I used is adding `_LO` suffix to indicate that `WIDE_SHADOW(N)_LO` is the left-shifted version of `WIDE_SHADOW(N)`. Overwriting `ORIGIN` is essential to keep the test smaller by reusing the same expression later on (see lines 312, 316). We need a way to state that, whatever path has been taken (fast8 or fast16, combine load ptr or not), the `ORIGIN` will refer to the name of the final origin that we have computed. Otherwise, we'd have to replicate those lines as well even though they are essentially the same.
> 
> However, I can partially address this (remove some of the overwriting) when removing fast16.
Got it. I missed the pattern needs to be shared. Yes. Please see if this can be simplified after fast16 is removed.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101584