[PATCH] D82680: MSAN: Allow emitting checks for struct types

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis marked an inline comment as done.
eugenis added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:1155
     unsigned StoreSize = DL.getTypeStoreSize(Shadow->getType());
     if (Shadow->getType()->isAggregateType()) {
       paintOrigin(IRB, updateOrigin(Origin, IRB), OriginPtr, StoreSize,
----------------
guiand wrote:
> eugenis wrote:
> > this can be improved now by moving the origin update code behind the shadow check
> This check covers arrays as well, so I don't think I can get rid of it completely but I can change it to check for `isArrayType()`. I wrote some code to let `convertShadowToScalar` handle Array types as well, but I thought I would leave that to a separate patch.
Ack.


================
Comment at: llvm/test/Instrumentation/MemorySanitizer/check-struct.ll:9
+define { i32, i8 } @main() sanitize_memory {
+; CHECK: %p = inttoptr i64 0 to { i32, i8 }*
+  %p = inttoptr i64 0 to { i32, i8 } *
----------------
Please check that the test passes when LLVM is build without assertions. AFAIR temp names are lost in that case.



================
Comment at: llvm/test/Instrumentation/MemorySanitizer/check-struct.ll:18
+; CHECK: [[F1_OR:%.+]] = or i1 [[F0_POISONED]], [[F1_POISONED]]
+; CHECK: %_mscmp = icmp ne i1 [[F1_OR]], false
+; CHECK: br i1 %_mscmp
----------------
There is still this unnecessary icmp.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82680