Bug 1230910 Remove some __try blocks that are unneeded in our use case
MozReview-Commit-ID: FHtNniEp8pQ
--- a/security/sandbox/chromium/base/threading/platform_thread_win.cc
+++ b/security/sandbox/chromium/base/threading/platform_thread_win.cc
@@ -25,30 +25,37 @@ const DWORD kVCThreadNameException = 0x4
typedef struct tagTHREADNAME_INFO {
DWORD dwType; // Must be 0x1000.
LPCSTR szName; // Pointer to name (in user addr space).
DWORD dwThreadID; // Thread ID (-1=caller thread).
DWORD dwFlags; // Reserved for future use, must be zero.
} THREADNAME_INFO;
-// This function has try handling, so it is separated out of its caller.
+#if 0
+ //This function is only used for debugging purposes, as you can find by its caller
+ //Here is the commit it was added in:
+ //https://codereview.chromium.org/8565036
+ //https://codereview.chromium.org/8565036/diff/6002/base/threading/platform_thread_win.cc
+
+ // This function has try handling, so it is separated out of its caller.
void SetNameInternal(PlatformThreadId thread_id, const char* name) {
THREADNAME_INFO info;
info.dwType = 0x1000;
info.szName = name;
info.dwThreadID = thread_id;
info.dwFlags = 0;
__try {
RaiseException(kVCThreadNameException, 0, sizeof(info)/sizeof(DWORD),
reinterpret_cast<DWORD_PTR*>(&info));
} __except(EXCEPTION_CONTINUE_EXECUTION) {
}
}
+#endif
struct ThreadParams {
PlatformThread::Delegate* delegate;
bool joinable;
ThreadPriority priority;
};
DWORD __stdcall ThreadFunc(void* params) {
@@ -174,17 +181,19 @@ void PlatformThread::SetName(const std::
// The debugger needs to be around to catch the name in the exception. If
// there isn't a debugger, we are just needlessly throwing an exception.
// If this image file is instrumented, we raise the exception anyway
// to provide the profiler with human-readable thread names.
if (!::IsDebuggerPresent() && !base::debug::IsBinaryInstrumented())
return;
+ #if 0
SetNameInternal(CurrentId(), name.c_str());
+ #endif
}
// static
const char* PlatformThread::GetName() {
return ThreadIdNameManager::GetInstance()->GetName(CurrentId());
}
// static
--- a/security/sandbox/chromium/sandbox/win/src/handle_closer_agent.cc
+++ b/security/sandbox/chromium/sandbox/win/src/handle_closer_agent.cc
@@ -19,22 +19,36 @@ namespace {
NTSTATUS QueryObjectTypeInformation(HANDLE handle,
void* buffer,
ULONG* size) {
static NtQueryObject QueryObject = NULL;
if (!QueryObject)
ResolveNTFunctionPtr("NtQueryObject", &QueryObject);
NTSTATUS status = STATUS_UNSUCCESSFUL;
- __try {
+ /*
+ * Removing this exception handler should be safe. It is used to suppress
+ * STATUS_INVALID_HANDLE exceptions and continue execution normally.
+ * Those exceptions are only raised when handle tracing is enabled.
+ * (This may be done by AppVerifier for example)
+ *
+ * If we don't enable handle tracing, we shouldn't see these exceptions
+ * and therefore it's as if this __except wasn't here (since it passes
+ * all other exceptions up the chain.)
+ *
+ * Ref:
+ * https://chromium.googlesource.com/chromium/src/+blame/4ec79b7f2379a60cdc15599e93255c0fa417f1ed/sandbox/win/src/handle_closer_agent.cc
+ * https://chromium.googlesource.com/chromium/src/+/4d3f6a99781f7bdca5237c49fccdc6c796e6abcc
+ */
+ //__try {
status = QueryObject(handle, ObjectTypeInformation, buffer, *size, size);
- } __except(GetExceptionCode() == STATUS_INVALID_HANDLE ?
- EXCEPTION_EXECUTE_HANDLER : EXCEPTION_CONTINUE_SEARCH) {
- status = STATUS_INVALID_HANDLE;
- }
+ //} __except(GetExceptionCode() == STATUS_INVALID_HANDLE ?
+ // EXCEPTION_EXECUTE_HANDLER : EXCEPTION_CONTINUE_SEARCH) {
+ //status = STATUS_INVALID_HANDLE;
+ //}
return status;
}
} // namespace
namespace sandbox {
// Memory buffer mapped from the parent, with the list of handles.