Page MenuHomePhabricator

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

Authored by hans on Apr 15 2019, 4:40 AM.

Details

Summary

If there are any intrinsics that cannot be traced back to an alloca, we might have missed the start of a variable's scope, leading to false error reports if the variable is poisoned at function entry. Instead, if there are some intrinsics that can't be traced, fail safe and don't poison the variables in that function.

Diff Detail

Repository
rL LLVM

Event Timeline

hans created this revision.Apr 15 2019, 4:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 15 2019, 4:40 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
nick added a subscriber: nick.Apr 15 2019, 8:03 AM
eugenis accepted this revision.Apr 15 2019, 1:39 PM

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.

llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll
223 ↗(On Diff #195133)

did you mean "should poison"?

This revision is now accepted and ready to land.Apr 15 2019, 1:39 PM
vitalybuka added inline comments.Apr 15 2019, 2:56 PM
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
922 ↗(On Diff #195133)

How often does this happen?
If often then maybe we should try to limit only that to allocas involved into such conflicts?

hans marked 2 inline comments as done.Apr 16 2019, 12:32 AM

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.

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
922 ↗(On Diff #195133)

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.

llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll
223 ↗(On Diff #195133)

No, I mean shouldn't poison. Since we don't know the lifetimes we can't poison it. Or am I missing something?

This revision was automatically updated to reflect the committed changes.
eugenis added inline comments.Apr 16 2019, 5:27 PM
llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll
223 ↗(On Diff #195133)

Well, we shouldn't poison them at lifetime intrinsic.
We should poison them at function entry (and only there).

eugenis added inline comments.Apr 16 2019, 5:28 PM
llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll
223 ↗(On Diff #195133)

Ah, I get what you mean. We should not poison alloca contents, only their redzones.