Enable handle tracing in Canary, Dev, and all debug builds of Chrome (Windows only).

Enable handle tracing in Canary, Dev, and all debug builds of Chrome (Windows only).

BUG=131699, 153148
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=159850

[alexeypa (please no reviews), 2012-10-01 23:33:39]

The 2nd attempt to land the CL that enables handle tracing on Windows. Patch #1
is the original change reverted because of http://crbug.com/153148. Patch #2 is
the fix.

The fix does two things:
- It catches all exceptions because, really, it is a sole NtQueryObject call we
are wrapping here, so any exception means the handle is bad, not just
STATUS_INVALID_HANDLE.
- Code within __try and the exception handling code should not touch the same
variable because the latter can be invoked at any instruction.

I'm the most concerned about the case when someone is hooking NtQueryObject in
user-mode. If the hook does not handle exceptions properly we will be in
trouble. What do you think?

rvargas@, p[lease take a look.
cpu@, please rubber stamp if it looks good.

[cpu_(ooo_6.6-7.5), 2012-10-02 19:21:37]

http://codereview.chromium.org/11035012/diff/2001/sandbox/win/src/handle_clos...
File sandbox/win/src/handle_closer_agent.cc (right):

http://codereview.chromium.org/11035012/diff/2001/sandbox/win/src/handle_clos...
sandbox/win/src/handle_closer_agent.cc:33: &exception_code)) {
can't you call GetExceptionCode() from the body of the __except and not need
block 12-17 ?

[alexeypa (please no reviews), 2012-10-02 19:23:22]

http://codereview.chromium.org/11035012/diff/2001/sandbox/win/src/handle_clos...
File sandbox/win/src/handle_closer_agent.cc (right):

http://codereview.chromium.org/11035012/diff/2001/sandbox/win/src/handle_clos...
sandbox/win/src/handle_closer_agent.cc:33: &exception_code)) {
On 2012/10/02 19:21:37, cpu wrote:
> can't you call GetExceptionCode() from the body of the __except and not need
> block 12-17 ?

GetExceptionCode() can only be called from the filter expression. It is not a
real function. Calling it from any other place will result in compiler error.

[cpu_(ooo_6.6-7.5), 2012-10-03 00:26:35]

lgtm

[commit-bot: I haz the power, 2012-10-03 00:27:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alexeypa@chromium.org/11035012/2001

[rvargas (doing something else), 2012-10-03 03:01:16]

http://codereview.chromium.org/11035012/diff/2001/sandbox/win/src/handle_clos...
File sandbox/win/src/handle_closer_agent.cc (right):

http://codereview.chromium.org/11035012/diff/2001/sandbox/win/src/handle_clos...
sandbox/win/src/handle_closer_agent.cc:16: return EXCEPTION_EXECUTE_HANDLER;
I'm a little concerned here about the actual handling of the exception, given
that not all exceptions are created equal. Should't we at least filter
breakpoints?

http://codereview.chromium.org/11035012/diff/2001/sandbox/win/src/handle_clos...
sandbox/win/src/handle_closer_agent.cc:29: NTSTATUS exception_code =
STATUS_UNSUCCESSFUL;
I would prefer setting exeption_code, but I don't think I fully understand your
concern.

If NtQueryOpen is patched, and there's an exception in that code, what changes?
(I mean, we are not assigning the value)... if it is being patched? If it is
executing the assignment? I don't see it. Stack corruption? let it crash :)

[commit-bot: I haz the power, 2012-10-03 05:59:29]

Change committed as 159850