[PATCH] D60617: MSan: handle llvm.lifetime.start intrinsic

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:1037
+  SmallSet<AllocaInst *, 16> AllocaSet;
+  SmallVector<std::pair<IntrinsicInst *, AllocaInst *>, 16> LifetimeStartList;
   SmallVector<StoreInst *, 16> StoreList;
----------------
This does not have to be so complicated.
Get rid of FallbackAllocaSet; instead remove instructions from AllocaSet as they are being poisoned at lifetime calls, then poison remaining alloca instructions.

This way if untraceable lifetime was seen, you can simply skip the lifetime poisoning step.

Please rename InstrumentOnlyAllocas to something that does not suggest the we are instrumenting _only_ allocas (and nothing else).



================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:1310
+          poisonAllocaUserspace(*Alloca, IRB, Len);
+      }
+
----------------
Move this logic to a separate function.


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:2574
+    if (Len->isMinusOne())
+      return;
+
----------------
What's wrong with variable length allocas?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60617