This is an archive of the discontinued LLVM Phabricator instance.

[winasan] Unpoison the stack in NtTerminateThread
ClosedPublic

Authored by dmajor on Sep 14 2018, 5:58 AM.

Details

Summary

In long-running builds we've seen some ASan complaints during thread creation that we suspect are due to leftover poisoning from previous threads whose stacks occupied that memory. This patch adds a hook that unpoisons the stack just before the NtTerminateThread syscall.

Diff Detail

Event Timeline

dmajor created this revision.Sep 14 2018, 5:58 AM
Herald added subscribers: Restricted Project, llvm-commits, kubamracek. · View Herald TranscriptSep 14 2018, 5:58 AM
rnk accepted this revision.Sep 19 2018, 1:40 PM

lgtm

This revision is now accepted and ready to land.Sep 19 2018, 1:40 PM
This revision was automatically updated to reflect the committed changes.
hans added a subscriber: hans.Sep 28 2018, 7:42 AM

This appears to have caused the thread exit code to get clobbered, as shown by some Chromium test failures: https://bugs.chromium.org/p/chromium/issues/detail?id=890310

I've reverted in r343322.

rnk added a comment.Sep 28 2018, 9:10 AM

I think NtTerminateThread has a richer prototype:
NTEXPORT NTSTATUS NTAPI
NtTerminateThread(IN HANDLE ThreadHandle OPTIONAL, IN NTSTATUS ExitStatus)
https://github.com/DynamoRIO/dynamorio/blob/f1713ec4a9a856d1038c6095da67a5bd95b6a1c7/core/win32/ntdll_imports.c#L154
http://codewarrior.cn/ntdoc/win2k/ps/NtTerminateThread.htm

It should be fine to recommit with a better prototype.

dmajor added a comment.Oct 3 2018, 2:53 PM

Relanded in r343606.