This is an archive of the discontinued LLVM Phabricator instance.

[lsan] Support ASan's stack-use-after-return mode in LSan.
ClosedPublic

Authored by earthdok on Oct 11 2013, 11:47 AM.

Details

Reviewers
kcc
samsonov
Summary

Treat the fake stack as live memory.

Diff Detail

Event Timeline

Somewhat ugly but does the job.

We still need to somehow solve (in a separate CL) the issue of stale pointers in reused frames, by doing one of the following:

  • clearing memory in FakeStack::Allocate(). Likely prohibitively expensive, but could improve leak detection beyond what we have currently (with UAR disabled).
  • storing the true size for each fake frame, and ignoring the unused space. This would be equivalent to what we have without UAR. Can we afford one extra uptr field in FakeFrame? Looks like the size of that structure is hardcoded into the instrumentation code somehow?
lib/asan/asan_fake_stack.cc
146

Some frames might be pending garbage collection. I wonder if it would be safe to filter them out based on the stack pointer value that we have from ptrace? (I.e. the same way as in FakeStack::GC() above.)

kcc added inline comments.Oct 14 2013, 4:13 AM
lib/asan/asan_fake_stack.cc
146

Can we just call GC here?
(this could be done separately)

lib/asan/asan_fake_stack.h
151

maybe ForEachExtraRootChunk or similar?
I don't like the ide of exposing the name "FakeFrame" in the interface that has nothing to do with fake frames

lib/asan/asan_thread.cc
326

you should use has_fake_stack()

earthdok added inline comments.Oct 14 2013, 5:00 AM
lib/asan/asan_fake_stack.cc
146

We can. What I'm wondering is whether it would be safe to pass the SP value as the real_stack parameter (or whether this could go wrong if we froze the threads at an unfortunate point).

lib/asan/asan_fake_stack.h
151

I share this sentiment. Unfortunately, the way we use this function already assumes that it deals with stack (i.e. we call it under "if (flags()->use_stack)", and log the range as a stack range). Perhaps ForEachExtraStackRange?

kcc added inline comments.Oct 14 2013, 6:36 AM
lib/asan/asan_fake_stack.cc
146

OTOMH I can't see how it will break things... But this part is indeed tricky, let's not touch it for now.

lib/asan/asan_fake_stack.h
151

ForEachExtraStackRange --- SGTM

earthdok updated this revision to Unknown Object (????).Oct 14 2013, 6:47 AM
  • address kcc's comments
earthdok updated this revision to Unknown Object (????).Oct 14 2013, 7:03 AM
  • fix one more use of ForEachFakeFrame
kcc accepted this revision.Oct 14 2013, 7:05 AM

LGTM

earthdok closed this revision.Dec 5 2014, 9:38 AM