Skip to content

fix: avoid UB left shift of negative ADRP immediate in decodeAdrTarget #848

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kaahos merged 2 commits into from
Oct 9, 2026
Merged

fix: avoid UB left shift of negative ADRP immediate in decodeAdrTarget #848

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
803e1ed
fix: avoid UB left shift of negative ADRP immediate in decodeAdrTarget
kaahos Oct 9, 2026
da2494a
test: add negative-ADRP regression for decodeAdrTarget's left-shift UB
kaahos Oct 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ddprof-lib/src/main/cpp/hotspot/stubUnwindInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ int decodeAdrTarget(uint32_t insn, const instruction_t* entry, int index, int co
if (target < start || target >= end || ((target - start) & (INSN_SIZE - 1)) != 0) return -1;
return (int)((target - start) / INSN_SIZE);
}
uintptr_t page = (pc & ~(uintptr_t)0xfff) + (uintptr_t)(imm << 12); // ADRP
uintptr_t page = (pc & ~(uintptr_t)0xfff) + (uintptr_t)((uint64_t)imm << 12); // ADRP
Comment thread
kaahos marked this conversation as resolved.
if (page < end && page + 0x1000 > start) adrp_into_stub = true;
return -1;
}
Expand Down
26 changes: 26 additions & 0 deletions ddprof-lib/src/test/cpp/stubUnwindInfo_ut.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,13 @@ inline uint32_t subSpRnImm(int rn, int imm) { return 0xd1000000 | 0x1f | (uint32
inline uint32_t call(int offInsns) { return 0x94000000 | (uint32_t)(offInsns & 0x03ffffff); }
inline uint32_t callReg(int rn) { return 0xd63f0000 | (uint32_t)rn << 5; }
inline uint32_t jumpReg(int rn) { return 0xd61f0000 | (uint32_t)rn << 5; }
// adrp Xd, #(pageImm * 4096) -- pageImm is the signed page-count offset,
// split across immlo (bits 30:29) and immhi (bits 23:5) the way the decoder
// in decodeAdrTarget() reassembles it.
inline uint32_t adrp(int rd, int pageImm) {
uint32_t imm21 = (uint32_t)pageImm & 0x1fffffu;
return 0x90000000u | ((imm21 & 0x3u) << 29) | (((imm21 >> 2) & 0x7ffffu) << 5) | (uint32_t)rd;
}

class StubUnwindTest : public ::testing::Test {
protected:
Expand Down Expand Up @@ -584,6 +591,25 @@ TEST_F(StubUnwindTest, AdrpIntoStubDisablesRestart) {
expectPhase(info, 2, SU_UNSUPPORTED, 0);
}

// Regression: decodeAdrTarget() computes an ADRP's target page as
// pc_page + (imm << 12), and imm is frequently negative (a page behind the
// current instruction, as here). Left-shifting a negative signed value is
// undefined behavior, and this build is compiled with
// -fno-sanitize-recover=all, so UBSan aborts the process the first time this
// path runs -- before this test's own assertions ever get to run. A target
// page outside the stub must also leave the mid-stub-br restart enabled,
// the same way AdrpIntoStubDisablesRestart's own-page target disables it.
TEST_F(StubUnwindTest, AdrpNegativePageOffsetAllowsRestart) {
// 0: adrp x9, #-4096 (one page behind this instruction -- outside the stub)
// 1: br x8
// 2: nop (second entry)
// 3: ret
StubUnwindInfo* info = analyze({adrp(9, -1), jumpReg(8), NOP, RET}, true);
ASSERT_NE(info, nullptr);
expectPhase(info, 1, SU_PC_TO_LR, 0);
expectPhase(info, 2, SU_PC_TO_LR, 0);
}

// x29/x30 writes from classes without a dedicated decoder: unscaled and
// register-offset loads, conditional select, base writeback.
TEST_F(StubUnwindTest, UndecodedGprWritesDropFrameOrLr) {
Expand Down
Loading