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

Issue 119453002: sandbox: Remove a TEXTREL on Linux ARM. (Closed)

Created:
7 years ago by Robert Sesek
Modified:
7 years ago
CC:
chromium-reviews, agl, jln+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

sandbox: Remove a TEXTREL on Linux ARM. On Android, the sandbox logic is put into a shared library and the linker errors out on having a TEXTREL. The LDR pseudo-instruction directs the assembler to place the address of the label in a "literal pool"[1], and then use a PC-relative load from that pool to get the address during execution. The pool is responsible for generating the R_ARM_RELATIVE TEXTREL. Using the ADR instruction[2] does not produce the TEXTREL. This instruction directs the assembler to calculate the PC-relative address to the label using an immediate. The text makes this difference more clear: ... 8: 4805 ldr r0, [pc, #20] ; (20 <SyscallAsm+0x20>) a: e007 b.n 1c <SyscallAsm+0x1c> ... 1c: bd80 pop {r7, pc} 1e: 0000 .short 0x0000 20: 0000001c .word 0x0000001c Versus: ... 8: 4804 ldr r0, [pc, #16] ; (1c <SyscallAsm+0x1c>) a: e007 b.n 1c <SyscallAsm+0x1c> ... 1c: bd80 pop {r7, pc} 1e: bf00 nop [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0041c/Babbfdih.html [2] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0040d/Cihdhgbe.html BUG=308763 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=242141

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -6 lines) Patch
M sandbox/linux/sandbox_linux.gypi View 1 chunk +0 lines, -5 lines 0 comments Download
M sandbox/linux/seccomp-bpf/syscall.cc View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 6 (0 generated)
Robert Sesek
7 years ago (2013-12-19 21:14:43 UTC) #1
jln (very slow on Chromium)
lgtm Nice bug hunting!
7 years ago (2013-12-19 23:38:42 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rsesek@chromium.org/119453002/1
7 years ago (2013-12-20 15:46:41 UTC) #3
commit-bot: I haz the power
Retried try job too often on win_rel for step(s) unit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=239043
7 years ago (2013-12-20 17:48:33 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rsesek@chromium.org/119453002/1
7 years ago (2013-12-20 17:51:15 UTC) #5
commit-bot: I haz the power
7 years ago (2013-12-20 19:35:02 UTC) #6
Message was sent while issue was closed.
Change committed as 242141

Powered by Google App Engine
This is Rietveld 408576698