Fix memory corruption in base_unittests in some configuration.

Fix memory corruption in base_unittests in some configuration.

On some combination of devices and version of OS (currently 64-bit iPad
running iOS 10), sysctlbyname("vm.pagesize", ...) fails. This cause the
wrapper ProcessMemoryDump::GetSystemPageSize to call base::GetPageSize,
which is incorrect when used for mincore() on iOS.

Instead use vm_kernel_page_size as recommended by Apple Staff on Apple
developer forums: https://forums.developer.apple.com/thread/47532.

BUG=542671
Committed: https://crrev.com/8bb015fe2ae98f0c019cda45096d8bfa0fc82d72
Cr-Commit-Position: refs/heads/master@{#421205}

[sdefresne, 2016-09-26 12:40:20]

The CQ bit was checked by sdefresne@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-26 12:40:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sdefresne, 2016-09-26 12:43:09]

sdefresne@chromium.org changed reviewers:
+ primiano@chromium.org

[sdefresne, 2016-09-26 12:43:09]

Please take a look.

[sdefresne, 2016-09-26 12:43:53]

On 2016/09/26 12:43:09, sdefresne wrote:
> Please take a look.

Note that "vm.pagesize" is not documented, so I think that we should not rely on
it, and this is why I unconditionally use vm_kernel_page_size here.

[commit-bot: I haz the power, 2016-09-26 13:50:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-26 13:50:38]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Primiano Tucci (use gerrit), 2016-09-26 14:24:05]

Feels like a deja-vu from https://codereview.chromium.org/1793943002 where
somebody pointed out it was corrupting memory and allegedly fixed this.
I have no idea how iOS works, so I'm going to trust whatever my iOS friends tell
me. 
On a side note I am more and more scared by iOS, if I can't even rely on how
bigs pages are.

LGTM :)

https://codereview.chromium.org/2365333002/diff/1/base/trace_event/process_me...
File base/trace_event/process_memory_dump.cc (left):

https://codereview.chromium.org/2365333002/diff/1/base/trace_event/process_me...
base/trace_event/process_memory_dump.cc:72: return base::GetPageSize();
I liked this the old way, i.e.
#if special case
...
return x
#endif
return x // general case

instead of the #else,
but not a huge deal.

[sdefresne, 2016-09-26 14:43:19]

On 2016/09/26 14:24:05, Primiano Tucci wrote:
> Feels like a deja-vu from https://codereview.chromium.org/1793943002 where
> somebody pointed out it was corrupting memory and allegedly fixed this.
> I have no idea how iOS works, so I'm going to trust whatever my iOS friends
tell
> me. 
> On a side note I am more and more scared by iOS, if I can't even rely on how
> bigs pages are.
> 
> LGTM :)
> 
>
https://codereview.chromium.org/2365333002/diff/1/base/trace_event/process_me...
> File base/trace_event/process_memory_dump.cc (left):
> 
>
https://codereview.chromium.org/2365333002/diff/1/base/trace_event/process_me...
> base/trace_event/process_memory_dump.cc:72: return base::GetPageSize();
> I liked this the old way, i.e.
> #if special case
> ...
> return x
> #endif
> return x // general case
> 
> instead of the #else,
> but not a huge deal.

This is exactly the same issue (in fact, I did reopen the same bug).

The issue is that iOS has two different "pagesize". The real physical page size
(returned by getpagesize() or sysctlbyname("hw.pagesize", ...). But as mentioned
in the linked thread, so low level mach API (like mincore()) use a different
pagesize. The previous fix use "vm.pagesize" to get that other pagesize, but
this function may fail, and in that case, the incorrect value returned by
getpagesize() is used leading to the same memory corruption.

The new CL use a global variable that is initialized as part of initializing the
mach runtime library
(https://opensource.apple.com/source/xnu/xnu-2050.7.9/libsyscall/mach/mach_init.c).
This is the same value returned by "vm.pagesize" when it works (though it
sometimes fails, which we noticed when we started running builds on iOS 10
iPads).

TL;DR: page size is returned by getpagesize() but mach API does not respect it
and use a different value that is exported as vm_kernel_page_size.

[sdefresne, 2016-09-26 14:44:36]

https://codereview.chromium.org/2365333002/diff/1/base/trace_event/process_me...
File base/trace_event/process_memory_dump.cc (left):

https://codereview.chromium.org/2365333002/diff/1/base/trace_event/process_me...
base/trace_event/process_memory_dump.cc:72: return base::GetPageSize();
On 2016/09/26 14:24:05, Primiano Tucci wrote:
> I liked this the old way, i.e.
> #if special case
> ...
> return x
> #endif
> return x // general case
> 
> instead of the #else,
> but not a huge deal.

Won't this cause a warning by the compiler about unreachable code? And returning
base::GetPageSize() here on iOS is incorrect. I'd prefer to make it clear that
it should have a different implementation on iOS.

[sdefresne, 2016-09-26 15:12:47]

The CQ bit was checked by sdefresne@chromium.org

[commit-bot: I haz the power, 2016-09-26 15:13:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-26 16:37:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-26 16:37:10]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[sdefresne, 2016-09-27 13:37:30]

The CQ bit was checked by sdefresne@chromium.org

[sdefresne, 2016-09-27 13:37:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
primiano@chromium.org
Link to the patchset: https://codereview.chromium.org/2365333002/#ps20001
(title: "Rebase.")

[commit-bot: I haz the power, 2016-09-27 13:37:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-27 14:39:55]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-27 14:42:25]

Description was changed from

==========
Fix memory corruption in base_unittests in some configuration.

On some combination of devices and version of OS (currently 64-bit iPad
running iOS 10), sysctlbyname("vm.pagesize", ...) fails. This cause the
wrapper ProcessMemoryDump::GetSystemPageSize to call base::GetPageSize,
which is incorrect when used for mincore() on iOS.

Instead use vm_kernel_page_size as recommended by Apple Staff on Apple
developer forums: https://forums.developer.apple.com/thread/47532.

BUG=542671
==========

to

==========
Fix memory corruption in base_unittests in some configuration.

On some combination of devices and version of OS (currently 64-bit iPad
running iOS 10), sysctlbyname("vm.pagesize", ...) fails. This cause the
wrapper ProcessMemoryDump::GetSystemPageSize to call base::GetPageSize,
which is incorrect when used for mincore() on iOS.

Instead use vm_kernel_page_size as recommended by Apple Staff on Apple
developer forums: https://forums.developer.apple.com/thread/47532.

BUG=542671

Committed: https://crrev.com/8bb015fe2ae98f0c019cda45096d8bfa0fc82d72
Cr-Commit-Position: refs/heads/master@{#421205}
==========

[commit-bot: I haz the power, 2016-09-27 14:42:25]

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