[PATCH] D83595: [Draft][MSAN] Optimize away poisoning allocas that are always written before load

Evgenii Stepanov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 31 11:58:59 PDT 2020


eugenis added a comment.

In general, this implementation looks pretty complex and easy to get wrong. I'd prefer something along the lines of AArch64StackTagging::collectInitializers - directly calculate the offset for each store/load instruction. It might do some extra work with unrelated memory instructions, but probably not too much.

How do you handle this case?

  a = alloca
  b = bitcast a
  lifetime_start b
  store b

When scanning from lifetime_start, this code will never encounter any direct use of a, and would miss the transitive use.

Missing diff context.



================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:3895
+          return false;
+        }
+      }
----------------
prefer early exits / continue(s)


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:3915
+  void populateRelevantAllocaUsers(Instruction *I, RedefOffsets &Defs,
+                                   UserSet &Users, uint64_t StoreOffs = 0) {
+    Defs.insert({I, StoreOffs});
----------------
This function is never called with StoreOffs != 0, which seems necessary to handle a chain of GEPs.


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:3920
+      if (isa<BitCastInst>(User)) {
+        Defs.insert({User, StoreOffs});
+        continue;
----------------
Only if same BB.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83595



More information about the llvm-commits mailing list