Correct PROCESS_BASIC_INFORMATION for 64 bit Windows.

Correct PROCESS_BASIC_INFORMATION for 64 bit Windows.

Based on the structure given at https://msdn.microsoft.com/en-us/library/windows/desktop/ms684280(v=vs.85).aspx.

BUG=528450
Committed: https://crrev.com/45e371a2d7ce5048144695aa62d1e196ec19afd2
Cr-Commit-Position: refs/heads/master@{#347842}

[rickyz (no longer on Chrome), 2015-09-02 22:36:26]

rickyz@chromium.org changed reviewers:
+ wfh@chromium.org

[Will Harris, 2015-09-03 00:00:04]

https://codereview.chromium.org/1328703003/diff/20001/sandbox/win/BUILD.gn
File sandbox/win/BUILD.gn (right):

https://codereview.chromium.org/1328703003/diff/20001/sandbox/win/BUILD.gn#ne...
sandbox/win/BUILD.gn:249: "src/sandbox_nt_util.cc",
I think you mean _unittest.cc here?

[rickyz (no longer on Chrome), 2015-09-03 00:56:57]

https://codereview.chromium.org/1328703003/diff/20001/sandbox/win/BUILD.gn
File sandbox/win/BUILD.gn (right):

https://codereview.chromium.org/1328703003/diff/20001/sandbox/win/BUILD.gn#ne...
sandbox/win/BUILD.gn:249: "src/sandbox_nt_util.cc",
On 2015/09/03 00:00:04, Will Harris wrote:
> I think you mean _unittest.cc here?

Oops, yeah, forgot to run trybots

[Will Harris, 2015-09-03 04:25:02]

wfh@chromium.org changed reviewers:
+ scottmg@chromium.org

[Will Harris, 2015-09-03 04:25:42]

+scottmg

crashpad has this struct too

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/crashp...

scott - should ExitStatus be PVOID or DWORD here...?  It seems the structs do
say PVOID...

[scottmg, 2015-09-03 16:18:53]

https://codereview.chromium.org/1328703003/diff/60001/sandbox/win/src/nt_inte...
File sandbox/win/src/nt_internals.h (right):

https://codereview.chromium.org/1328703003/diff/60001/sandbox/win/src/nt_inte...
sandbox/win/src/nt_internals.h:314: PVOID ExitStatus;
This should be NTSTATUS on both x86 and x64, followed by 4bytes padding on x64.

MSDN documentation likely makes the reserved fields PVOID to imply that they're
opaque/internal but to make sure the size is correct.

https://codereview.chromium.org/1328703003/diff/60001/sandbox/win/src/nt_inte...
sandbox/win/src/nt_internals.h:317: KPRIORITY BasePriority;
KPRIORITY is also 4 bytes, so this should be followed by 4 bytes padding on
x64...

https://codereview.chromium.org/1328703003/diff/60001/sandbox/win/src/nt_inte...
sandbox/win/src/nt_internals.h:318: ULONG_PTR UniqueProcessId;
... because both the ProcessIds are both ULONG_PTR.

[rickyz (no longer on Chrome), 2015-09-03 19:16:29]

https://codereview.chromium.org/1328703003/diff/60001/sandbox/win/src/nt_inte...
File sandbox/win/src/nt_internals.h (right):

https://codereview.chromium.org/1328703003/diff/60001/sandbox/win/src/nt_inte...
sandbox/win/src/nt_internals.h:314: PVOID ExitStatus;
On 2015/09/03 16:18:52, scottmg wrote:
> This should be NTSTATUS on both x86 and x64, followed by 4bytes padding on
x64.
> 
> MSDN documentation likely makes the reserved fields PVOID to imply that
they're
> opaque/internal but to make sure the size is correct.

Done.

https://codereview.chromium.org/1328703003/diff/60001/sandbox/win/src/nt_inte...
sandbox/win/src/nt_internals.h:317: KPRIORITY BasePriority;
On 2015/09/03 16:18:52, scottmg wrote:
> KPRIORITY is also 4 bytes, so this should be followed by 4 bytes padding on
> x64...

Oops, fixed the definition of KPRIORITY in the file.

https://codereview.chromium.org/1328703003/diff/60001/sandbox/win/src/nt_inte...
sandbox/win/src/nt_internals.h:318: ULONG_PTR UniqueProcessId;
On 2015/09/03 16:18:52, scottmg wrote:
> ... because both the ProcessIds are both ULONG_PTR.

Yeah.. the ExitStatus one I can understand - that's just being explicit about
alignment. I have no idea why they reserve 8 bytes for process ids here - it's
like CLIENT_ID where the process/thread ids are HANDLEs instead of DWORDs.

Anyway, I switched to the union approach that crashpad uses.

[Will Harris, 2015-09-04 02:18:12]

lgtm % nit

https://codereview.chromium.org/1328703003/diff/120001/sandbox/win/src/sandbo...
File sandbox/win/src/sandbox_nt_util_unittest.cc (right):

https://codereview.chromium.org/1328703003/diff/120001/sandbox/win/src/sandbo...
sandbox/win/src/sandbox_nt_util_unittest.cc:24:
ASSERT_TRUE(current_process.IsValid());
some are EXPECT_TRUE and some ASSERT_TRUE - normally in tests we would just use
EXPECT_TRUE

[Will Harris, 2015-09-04 02:19:13]

is there a BUG= for this?

[rickyz (no longer on Chrome), 2015-09-08 21:51:39]

https://codereview.chromium.org/1328703003/diff/120001/sandbox/win/src/sandbo...
File sandbox/win/src/sandbox_nt_util_unittest.cc (right):

https://codereview.chromium.org/1328703003/diff/120001/sandbox/win/src/sandbo...
sandbox/win/src/sandbox_nt_util_unittest.cc:24:
ASSERT_TRUE(current_process.IsValid());
On 2015/09/04 02:18:12, Will Harris wrote:
> some are EXPECT_TRUE and some ASSERT_TRUE - normally in tests we would just
use
> EXPECT_TRUE

Discussed in person - I split the test up into smaller pieces so that the other
tests will still run even if we hit an assert.

[rickyz (no longer on Chrome), 2015-09-08 21:51:44]

The CQ bit was checked by rickyz@chromium.org

[rickyz (no longer on Chrome), 2015-09-08 21:51:44]

The patchset sent to the CQ was uploaded after l-g-t-m from wfh@chromium.org
Link to the patchset: https://codereview.chromium.org/1328703003/#ps140001
(title: "Split test into smaller pieces.")

[commit-bot: I haz the power, 2015-09-08 21:52:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1328703003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1328703003/140001

[commit-bot: I haz the power, 2015-09-09 00:13:22]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-09-09 00:13:58]

Patchset 8 (id:??) landed as
https://crrev.com/45e371a2d7ce5048144695aa62d1e196ec19afd2
Cr-Commit-Position: refs/heads/master@{#347842}