win: Don't run x64 specific test unless built for x64

win: Don't run x64 specific test unless built for x64

R=mark@chromium.org
BUG=crashpad:50
Committed: https://chromium.googlesource.com/crashpad/crashpad/+/d62866f734e65bf19c4e06430e98175529ae7ce0

[Mark Mentovai, 2015-09-11 22:40:58]

LGTM

https://codereview.chromium.org/1343433003/diff/20001/util/win/process_info_t...
File util/win/process_info_test.cc (right):

https://codereview.chromium.org/1343433003/diff/20001/util/win/process_info_t...
util/win/process_info_test.cc:165: // child if it is x86 and the child is x64,
so it only makes sense to run this
“can’t read from the child if it is x86 and the child is x64”

because of no Crashpad support or no Windows support?

[scottmg, 2015-09-11 22:45:51]

https://codereview.chromium.org/1343433003/diff/20001/util/win/process_info_t...
File util/win/process_info_test.cc (right):

https://codereview.chromium.org/1343433003/diff/20001/util/win/process_info_t...
util/win/process_info_test.cc:165: // child if it is x86 and the child is x64,
so it only makes sense to run this
On 2015/09/11 22:40:58, Mark Mentovai - August is over wrote:
> “can’t read from the child if it is x86 and the child is x64”
> 
> because of no Crashpad support or no Windows support?

A bit of both. There's undocumented equivalents to ReadProcessMemory that can
read x64 from x86 but I think it'll get messy quickly, as it adds to the WOW64
permutations. I don't think it's too useful for Crashpad to support an x86 crash
handler reading x64 processes in any case?

[Mark Mentovai, 2015-09-11 22:51:17]

Still LGTM

https://codereview.chromium.org/1343433003/diff/20001/util/win/process_info_t...
File util/win/process_info_test.cc (right):

https://codereview.chromium.org/1343433003/diff/20001/util/win/process_info_t...
util/win/process_info_test.cc:165: // child if it is x86 and the child is x64,
so it only makes sense to run this
On 2015/09/11 22:45:51, scottmg wrote:
> On 2015/09/11 22:40:58, Mark Mentovai - August is over wrote:
> > “can’t read from the child if it is x86 and the child is x64”
> > 
> > because of no Crashpad support or no Windows support?
> 
> A bit of both. There's undocumented equivalents to ReadProcessMemory that can
> read x64 from x86 but I think it'll get messy quickly, as it adds to the WOW64
> permutations. I don't think it's too useful for Crashpad to support an x86
crash
> handler reading x64 processes in any case?

Maybe not useful, but interesting, and if it’s just ReadProcessMemory(), it
should be reasonably self-contained. (And we’ve already got the WinVMAddress
type, so most of our code should already be ready for it from the address-width
perspective.)

I’m not saying that I really need to see it supported, although I’d personally
consider it fun, and it might make us more resilient and allow us to catch
crashes from the small number of users with the weird cross-bitted installs that
we’ll inevitably see. (How likely this is depends on whether Crashpad and Chrome
can wind up coming from different places, but I’m a little scarred by the
Chrome-Omaha versioning nightmare.)

[scottmg, 2015-09-11 23:09:55]

On 2015/09/11 22:51:17, Mark Mentovai - August is over wrote:
> Still LGTM
> 
>
https://codereview.chromium.org/1343433003/diff/20001/util/win/process_info_t...
> File util/win/process_info_test.cc (right):
> 
>
https://codereview.chromium.org/1343433003/diff/20001/util/win/process_info_t...
> util/win/process_info_test.cc:165: // child if it is x86 and the child is x64,
> so it only makes sense to run this
> On 2015/09/11 22:45:51, scottmg wrote:
> > On 2015/09/11 22:40:58, Mark Mentovai - August is over wrote:
> > > “can’t read from the child if it is x86 and the child is x64”
> > > 
> > > because of no Crashpad support or no Windows support?
> > 
> > A bit of both. There's undocumented equivalents to ReadProcessMemory that
can
> > read x64 from x86 but I think it'll get messy quickly, as it adds to the
WOW64
> > permutations. I don't think it's too useful for Crashpad to support an x86
> crash
> > handler reading x64 processes in any case?
> 
> Maybe not useful, but interesting, and if it’s just ReadProcessMemory(), it
> should be reasonably self-contained. (And we’ve already got the WinVMAddress
> type, so most of our code should already be ready for it from the
address-width
> perspective.)
> 
> I’m not saying that I really need to see it supported, although I’d personally
> consider it fun, and it might make us more resilient and allow us to catch
> crashes from the small number of users with the weird cross-bitted installs
that
> we’ll inevitably see. (How likely this is depends on whether Crashpad and
Chrome
> can wind up coming from different places, but I’m a little scarred by the
> Chrome-Omaha versioning nightmare.)

Yeah, that's all true and scary. This test would only need a ReadProcessMemory64
(I think) which is roughly a matter of escaping WOW64 to get the x64 version of
NtReadVirtualMemory. That's pretty awful by itself, see e.g.
http://blog.rewolf.pl/blog/?p=102 (the getLdrGetProcedureAddress function).

My bigger concern is that it just gets complicated to deal with WOW64 in
general, and we're going to have ~4 permutations to test already.

But anyway, yeah, it'd be kind of neat to support all of them. Maybe someday
once we have the basic ones done.

[scottmg, 2015-09-11 23:11:06]

Committed patchset #2 (id:20001) manually as
d62866f734e65bf19c4e06430e98175529ae7ce0 (presubmit successful).

[Mark Mentovai, 2015-09-11 23:18:02]

scottmg wrote:
> Yeah, that's all true and scary. This test would only need a
ReadProcessMemory64
> (I think) which is roughly a matter of escaping WOW64 to get the x64 version
of
> NtReadVirtualMemory. That's pretty awful by itself, see e.g.
> http://blog.rewolf.pl/blog/?p=102 (the getLdrGetProcedureAddress function).

That’s both horrible and amazing at the same time.