crazy linker: Add a Breakpad "guard region" to reserved space.

crazy linker: Add a Breakpad "guard region" to reserved space.

Breakpad requires currently that a library's reported start_addr
actually be its load_bias.  It will also complain if there is an
apparent overlap in the memory mappings it reads in from a microdump.
If something in the process mmaps into the address space between
load_bias and start_addr, this will break Breakpad's minidump
processor.

Work round by adding a "guard region" into the reserved address space
but ahead of the start_addr where the library is loaded.  Making this
part of the reserved address space for the library ensures that
nothing will later mmap into it.

BUG=504410, 499747
Committed: https://crrev.com/d91775bf59f883b75cce2071ac8b76f7aee062a2
Cr-Commit-Position: refs/heads/master@{#337035}

[simonb (inactive), 2015-06-26 16:49:26]

simonb@chromium.org changed reviewers:
+ primiano@chromium.org, rmcilroy@chromium.org

[simonb (inactive), 2015-06-26 16:49:27]

First swipe at this.  It works correctly both with and without relocation
packing on arm32, under quick smoke-test conditions.  More testing for arm64 and
a wider range of environments is needed.

Meanwhile, any initial thoughts on the code?  Thx.

[simonb (inactive), 2015-07-01 10:38:35]

Somewhat simplified/clarified version in patch set 2.  Tested on both arm32 and
arm64, with and without packing, and with and without load from apk.

Please let me know if this is good to go.  Thx.

--- Primiano: After investigation I've determined that we should not need
another guard region after the end of libchrome.  Breakpad client uploads
mappings in /proc/$$/maps address order.  On apparent mapping range overlap
breakpad processor discards the second one of the pair.  The guard region
between load_bias and load_start should ensure that libchrome is never the
second of a pair, only (potentially) the first.

[rmcilroy, 2015-07-01 14:36:54]

lgtm, thanks.

https://codereview.chromium.org/1218493004/diff/20001/third_party/android_cra...
File third_party/android_crazy_linker/src/src/crazy_linker_elf_loader.cpp
(right):

https://codereview.chromium.org/1218493004/diff/20001/third_party/android_cra...
third_party/android_crazy_linker/src/src/crazy_linker_elf_loader.cpp:246:
LOG("%s: breakpad: adjusted reservation by %d\n", __FUNCTION__, min_vaddr);
nit - don't put "breakpad:" otherwise it looks like the log call is coming from
breakpad. Something like "Adjusted reservation by %d to add Breakpad guard
region\n" ?

https://codereview.chromium.org/1218493004/diff/20001/third_party/android_cra...
third_party/android_crazy_linker/src/src/crazy_linker_elf_loader.cpp:279:
LOG("%s: breakpad: adjusted map by %d\n", __FUNCTION__, min_vaddr);
ditto

[simonb (inactive), 2015-07-01 14:57:06]

Thanks.

https://codereview.chromium.org/1218493004/diff/20001/third_party/android_cra...
File third_party/android_crazy_linker/src/src/crazy_linker_elf_loader.cpp
(right):

https://codereview.chromium.org/1218493004/diff/20001/third_party/android_cra...
third_party/android_crazy_linker/src/src/crazy_linker_elf_loader.cpp:246:
LOG("%s: breakpad: adjusted reservation by %d\n", __FUNCTION__, min_vaddr);
On 2015/07/01 14:36:54, rmcilroy wrote:
> nit - don't put "breakpad:" otherwise it looks like the log call is coming
from
> breakpad. Something like "Adjusted reservation by %d to add Breakpad guard
> region\n" ?

Done.

https://codereview.chromium.org/1218493004/diff/20001/third_party/android_cra...
third_party/android_crazy_linker/src/src/crazy_linker_elf_loader.cpp:279:
LOG("%s: breakpad: adjusted map by %d\n", __FUNCTION__, min_vaddr);
On 2015/07/01 14:36:54, rmcilroy wrote:
> ditto

Done.

[simonb (inactive), 2015-07-01 15:56:03]

The CQ bit was checked by simonb@chromium.org

[simonb (inactive), 2015-07-01 15:56:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/1218493004/#ps40001
(title: "Tweak for review feedback.")

[commit-bot: I haz the power, 2015-07-01 15:56:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1218493004/40001

[commit-bot: I haz the power, 2015-07-01 16:30:59]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-07-01 16:31:53]

Patchset 3 (id:??) landed as
https://crrev.com/d91775bf59f883b75cce2071ac8b76f7aee062a2
Cr-Commit-Position: refs/heads/master@{#337035}