This is an archive of the discontinued LLVM Phabricator instance.

[hwasan] Fix Thread reuse (try 2).
ClosedPublic

Authored by eugenis on Nov 12 2020, 3:39 PM.

Details

Summary

HwasanThreadList::DontNeedThread clobbers Thread::next_,
Breaking the freelist. As a result, only the top of the freelist ever
gets reused, and the rest of it is lost.

Since the Thread object with its associated ring buffer is only 8Kb, this is
typically only noticable in long running processes, such as fuzzers.

Fix the problem by switching from an intrusive linked list to a vector.

Diff Detail

Event Timeline

eugenis created this revision.Nov 12 2020, 3:39 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 12 2020, 3:39 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
eugenis requested review of this revision.Nov 12 2020, 3:39 PM

This time with a better test using pthread barriers.

vitalybuka added inline comments.
compiler-rt/lib/hwasan/hwasan_thread_list.h
113

maybe you can just flip some the t->dead = true
and do some garbage collection when e.g. live_list_ contains 50% of dead?

115

t2 = live_list_.back();
live_list_.pop_back();

eugenis added inline comments.Nov 12 2020, 4:10 PM
compiler-rt/lib/hwasan/hwasan_thread_list.h
113

this sounds like a premature optimization, and it would increase RAM by decreasing CPU use, which is not the direction we want to move on mobile

115

This would remove the last element, why?

vitalybuka added inline comments.Nov 12 2020, 4:59 PM
compiler-rt/lib/hwasan/hwasan_thread_list.h
115

no, the last element in t2 position now.
to avoid moving memory by live_list_.erase(&t2);

eugenis added inline comments.Nov 12 2020, 5:40 PM
compiler-rt/lib/hwasan/hwasan_thread_list.h
115

that's a really good idea, I can't believe I missed it

eugenis updated this revision to Diff 305922.Nov 17 2020, 4:24 PM

faster live list removal

Remove the implementation of erase(), which is no longer necessary.
PTAL.

hctim accepted this revision.Nov 18 2020, 11:20 AM
hctim added inline comments.
compiler-rt/test/hwasan/TestCases/Linux/reuse-threads.cpp
42

Nit: why create 4 threads if we only check that 2 of them are reused? Why not check 4 or only create 2?

This revision is now accepted and ready to land.Nov 18 2020, 11:20 AM
eugenis updated this revision to Diff 306242.Nov 18 2020, 3:33 PM

reduced the number of threads in the test to 2

eugenis marked an inline comment as done.Nov 18 2020, 3:33 PM
hctim accepted this revision.Nov 18 2020, 3:51 PM
This revision was automatically updated to reflect the committed changes.