Page MenuHomePhabricator

[AArch64] Fix range check of R_AARCH64_TLSLE_ADD_TPREL_HI12
ClosedPublic

Authored by rprichard on Sep 25 2018, 5:31 PM.

Details

Summary

An AArch64 LE relocation is a positive ("variant 1") offset. This
relocation is used to write the upper 12 bits of a 24-bit offset into an
add instruction:

add x0, x0, :tprel_hi12:v1

The comment in the ARM docs for R_AARCH64_TLSLE_ADD_TPREL_HI12 is:

"Set an ADD immediate field to bits [23:12] of X; check 0 <= X < 2^24."

Diff Detail

Repository
rL LLVM

Event Timeline

rprichard created this revision.Sep 25 2018, 5:31 PM

The original line was added in https://reviews.llvm.org/rL260677.

I didn't see any other similar problems with other relocation overflow checks.

rprichard edited the summary of this revision. (Show Details)Sep 25 2018, 5:46 PM

For context, I didn't notice this bug because I needed more than 2^23 bytes of TLS LE memory, but because I was experimenting with using a variant 2 TLS layout with Android/AArch64. The standard layout (variant 1 with a 2-word TCB) is a problem because Bionic has allocated some memory that overlaps with the first TLS segment, accessible using TLS LE. In particular, the stack cookie is a problem (see getStackCookieLocation in https://reviews.llvm.org/D18632), because third-party binaries may be assuming a variant-1 incompatible TLS layout.

This range check bug isn't related to Android, though.

This change looks correct to me. The docs link is http://arminfo.emea.arm.com/help/topic/com.arm.doc.ihi0056b/IHI0056B_aaelf64.pdf search for relocation code 549 in Table 4-18, Local Exec TLS relocations. I've checked the other TPREL relocations that LLD has implemented and they are all of the NC (non range checked) kind so it looks like there aren't any others of this form.

ruiu accepted this revision.Sep 26 2018, 9:28 AM

LGTM

This revision is now accepted and ready to land.Sep 26 2018, 9:28 AM
This revision was automatically updated to reflect the committed changes.
This revision was automatically updated to reflect the committed changes.