3 Commits

Author SHA1 Message Date
Mark Mentovai
6d5bd1d04d win: Go back to using ml.exe for SafeTerminateProcess()
This reverts 55133d332b6c and adds a broken dummy SafeTerminateProcess()
for cross builds instead. It’s similar to 2f4516f93838, which was for
CaptureContext().

This upstreams
af5f31ed61
(slightly modified).

The dummy implementation in the “broken” file affords no protection
against third-party code patching TerminateProcess() badly. The “broken”
file is not used by Crashpad anywhere at all, and is only used by
Crashpad in Chromium during a cross build targeting Windows without the
benefit of Microsoft’s ml.exe assembler. Strictly speaking, this file
does not need to be checked in to the Crashpad repository, but since
Chromium needs it to unblock its not-production-ready cross build for
Windows, it’s being landed here to avoid Chromium’s copy of Crashpad
appearing as modified or “dirty” relative to this upstream copy.

Bug: chromium:762167, chromium:777924
Change-Id: Iba68c0cab142fbe9541ea254a9a856b8263e4c70
Reviewed-on: https://chromium-review.googlesource.com/735078
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-10-24 19:07:38 +00:00
Mark Mentovai
55133d332b win: Use inline asm instead of ml.exe for SafeTerminateProcess()
This upstreams
912c9907d5
(slightly modified).

Bug: chromium:762167
Change-Id: I69c605f693da8691d32222b5617f62637c1c2dcd
Reviewed-on: https://chromium-review.googlesource.com/734100
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-10-23 19:14:05 +00:00
Mark Mentovai
e04194afd9 win: Wrap TerminateProcess() to accept cdecl patches on x86
TerminateProcess(), like most of the Windows API, is declared WINAPI,
which is __stdcall on 32-bit x86. That means that the callee,
TerminateProcess() itself, is responsible for cleaning up parameters on
the stack on return. In https://crashpad.chromium.org/bug/179, crashes
in ExceptionHandlerServer::OnNonCrashDumpEvent() were observed in ways
that make it evident that TerminateProcess() has been patched with a
__cdecl routine. The crucial difference between __stdcall and __cdecl is
that the caller is responsible for stack parameter cleanup in __cdecl.
The mismatch means that nobody cleans parameters from the stack, and the
stack pointer has an unexpected value, which in the case of the Crashpad
handler crash, results in TerminateProcess()’s second argument
erroneously being used as the lock address in the call to
ReleaseSRWLockExclusive() or LeaveCriticalSection().

As a workaround, on 32-bit x86, call through SafeTerminateProcess(), a
custom assembly routine that’s compatible with either __stdcall or
__cdecl implementations of TerminateProcess() by not trusting the value
of the stack pointer on return from that function. Instead, the stack
pointer is restored directly from the frame pointer.

Bug: crashpad:179
Test: crashpad_util_test SafeTerminateProcess.*, others
Change-Id: If9508f4eb7631020ea69ddbbe4a22eb335cdb325
Reviewed-on: https://chromium-review.googlesource.com/481180
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-04-19 17:45:32 +00:00