Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(110)

Issue 1155973005: crazy linker: Fix incorrect link map l_addr value. (Closed)

Created:
5 years, 6 months ago by simonb (inactive)
Modified:
5 years, 6 months ago
Reviewers:
rmcilroy
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

crazy linker: Fix incorrect link map l_addr value. Link map's l_addr field should contain the load bias: - https://android-review.googlesource.com/#/c/46470/2/linker/linker.cpp It currently contains the load address. Where the min vaddr of LOAD segments is zero, the load address and load bias are the same, and the problem remains hidden. Android's relocation packer generates a non-zero min vaddr, and this exposes the problem on arm64 platforms. The symptom is abort in uw_init_context_1, where uw_frame_state_for returns _URC_END_OF_STACK rather than (expected) _URC_NO_REASON. gcc's stack unwinding code does not find correct unwinding information after an incorrect l_addr has been used to convert from virtual to physical addresses. Arm32 does not show the problem because it uses dl_unwind_find_exidx in place of _Unwind_IteratePhdrCallback. See also line 196 of: - https://android.googlesource.com/platform/bionic/+/ lollipop-mr1-release/linker/linker.cpp BUG=385553 Committed: https://crrev.com/a0add2c099683dcfc16d3b67c6c1c438c36c2ce7 Cr-Commit-Position: refs/heads/master@{#333040}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -1 line) Patch
M third_party/android_crazy_linker/README.chromium View 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/android_crazy_linker/src/src/crazy_linker_library_list.cpp View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 7 (2 generated)
simonb (inactive)
5 years, 6 months ago (2015-06-05 12:32:08 UTC) #2
rmcilroy
On 2015/06/05 12:32:08, simonb wrote: Yay! lgtm, thanks!
5 years, 6 months ago (2015-06-05 12:34:20 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1155973005/1
5 years, 6 months ago (2015-06-05 12:54:54 UTC) #5
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 6 months ago (2015-06-05 13:23:07 UTC) #6
commit-bot: I haz the power
5 years, 6 months ago (2015-06-05 13:23:52 UTC) #7
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/a0add2c099683dcfc16d3b67c6c1c438c36c2ce7
Cr-Commit-Position: refs/heads/master@{#333040}

Powered by Google App Engine
This is Rietveld 408576698