[PATCH] D60686: Asan use-after-scope: don't poison allocas if there were untraced lifetime intrinsics in the function (PR41481)

Hans Wennborg via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 16 00:33:15 PDT 2019


hans marked 2 inline comments as done.
hans added a comment.

In D60686#1467406 <https://reviews.llvm.org/D60686#1467406>, @eugenis wrote:

> LGTM
>  Consider extending the test with another alloca which does not participate in any untraceable lifetimes, but has to be poisoned in entry block anyway.


But if there are untraceable lifetimes, we don't know what allocas might potentially be involved. At least not as the code currently works, where it doesn't trace selects and phis with different values.



================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:922
 
+    if (HasUntracedLifetimeIntrinsic) {
+      // If there are lifetime intrinsics which couldn't be traced back to an
----------------
vitalybuka wrote:
> How often does this happen?
> If often then maybe we should try to limit only that to allocas involved into such conflicts?
I think it's very rare, but it happens. We didn't run into it in any Chromium tests, but one test in V8 did. I think just bailing out for such functions is okay.


================
Comment at: llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll:223
+  ; Since we cannot account for all lifetime intrinsics in this function, we
+  ; might have missed a lifetime.start one and therefore shouldn't poison the
+  ; allocas at function entry.
----------------
eugenis wrote:
> did you mean "should poison"?
No, I mean shouldn't poison. Since we don't know the lifetimes we can't poison it. Or am I missing something?


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

https://reviews.llvm.org/D60686





More information about the llvm-commits mailing list