Page MenuHomePhabricator

[asan] Improve thread lifetime tracking on POSIX systems.
ClosedPublic

Authored by earthdok on Oct 11 2013, 7:43 AM.

Details

Reviewers
kcc
samsonov
Summary

Call AsanThread::Destroy() from a late-running TSD destructor.
Previously we called it before any user-registered TSD destructors, which caused
false positives in LeakSanitizer.

Diff Detail

Event Timeline

I'm OK with this change, but let's wait for Kostya's opinion as well.

lib/asan/asan_thread.cc
168

I prefer

if (!SANITIZER_POSIX)

Elaborate whan "unhappy" means (thread will be treated as dead too early, so we'll fail to see live memory from this thread while it's still running TSD destructors).

lib/asan/asan_thread.h
133

Consider moving this to asan_internal.h next to other TSD-related functions.

earthdok updated this revision to Unknown Object (????).Oct 11 2013, 8:33 AM
  • address samsonov's comments
kcc added inline comments.Oct 14 2013, 2:49 AM
lib/asan/asan_thread.cc
173

I don't like this 'if'. What will happen on Windos? Won't PlatformTSDDtor be called?

samsonov added inline comments.Oct 14 2013, 4:22 AM
lib/asan/asan_thread.cc
173

It won't be called on Windows:

void AsanTSDInit(void (*destructor)(void *tsd)) {
  // FIXME: we're ignoring the destructor for now.
  tsd_key_inited = true;
}

It shouldn't break in a bad way if PlatformTSDDtor will be called on Windows: if thread is already destroyed, AsanThread::Destroy() is a no-op.

kcc accepted this revision.Oct 14 2013, 4:35 AM

LGTM
I still don't like this 'if' but see no better way

Committed r192585

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