[PATCH] D143565: [Asan] Ensure unpoisonning doesn't get inlined unnecessarily due to small holes in the mask

[Roy Sundahl via Phabricator via llvm-commits]

rsundahl added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:2964
+
     if (j - i >= ClMaxInlinePoisoningSize) {
       copyToShadowInline(ShadowMask, ShadowBytes, Done, i, IRB, ShadowBase);
----------------
This function is probably not the place to guess that it's ok to ignore the shadow mask because it's assumed that it's being called for un-poisoning. While this may be the case, the shadow mask and shadow data provide high fidelity control over behavior and assuming the intent at this low level is problematic. I realize that at a line 2888 the same assumption is made during inlining, but moving this up to the caller and cleaning up these two functions would be far superior. 


================
Comment at: llvm/test/Instrumentation/AddressSanitizer/asan-optimize-inline-poisoning.ll:2
+; RUN: opt < %s  -passes=asan -asan-max-inline-poisoning-size=8 -S | FileCheck %s --check-prefix=CHECK
+
+target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
----------------
Your test when run with -asan-max-inline-poisoning-size=0 still inlines but it doesn't look like it is due to your changes (unless datalayout is contrived for the test and doesn't occur otherwise.) It's more likely that my testing for [[ https://reviews.llvm.org/D136197  ]] was inadequate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143565