Skip to content

Commit f8224f6

Browse files
rmacnak-googleCommit Queue
authored andcommitted
[vm] Fix profiler crash caused by the profiler observing a partially-executed load-multiple in the ARM simulators.
TEST=dartfuzz Bug: #60876 Bug: #60810 Bug: #60850 Change-Id: I11c43412970ee3fa161326661684bc8ccf7153eb Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/441641 Commit-Queue: Ryan Macnak <[email protected]> Reviewed-by: Derek Xu <[email protected]> Reviewed-by: Tess Strickland <[email protected]>
1 parent db0c646 commit f8224f6

File tree

3 files changed

+40
-13
lines changed

3 files changed

+40
-13
lines changed

runtime/vm/compiler/assembler/assembler_riscv.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4691,8 +4691,10 @@ void Assembler::LeaveDartFrame() {
46914691
}
46924692
set_constant_pool_allowed(false);
46934693
subi(SP, FP, 2 * target::kWordSize);
4694-
lx(FP, Address(SP, 0 * target::kWordSize));
4694+
// Update RA first so the profiler can identify the frame as the entry frame
4695+
// after FP is updated but before the return instruction.
46954696
lx(RA, Address(SP, 1 * target::kWordSize));
4697+
lx(FP, Address(SP, 0 * target::kWordSize));
46964698
addi(SP, SP, 2 * target::kWordSize);
46974699
}
46984700

runtime/vm/simulator_arm.cc

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1348,18 +1348,33 @@ void Simulator::HandleRList(Instr* instr, bool load) {
13481348
if (instr->HasW()) {
13491349
set_register(rn, rn_val);
13501350
}
1351-
int reg = 0;
1352-
while (rlist != 0) {
1353-
if ((rlist & 1) != 0) {
1354-
if (load) {
1355-
set_register(static_cast<Register>(reg), ReadW(address, instr));
1356-
} else {
1357-
WriteW(address, get_register(static_cast<Register>(reg)), instr);
1351+
1352+
if (rlist == ((1 << FP) | (1 << PC))) {
1353+
// Special case `ldmia {fp, pc}` for LeaveDartFrame so that the profiler
1354+
// does not get confused by observing the update to fp before pc when our
1355+
// caller is the entry stub, i.e., failing to identify the entry frame. On
1356+
// real hardware, the profiler's signal handler cannot observe a partially
1357+
// executued load-multiple instruction.
1358+
int32_t new_fp = ReadW(address, instr);
1359+
address += 4;
1360+
int32_t new_pc = ReadW(address, instr);
1361+
address += 4;
1362+
set_register(PC, new_pc);
1363+
set_register(FP, new_fp);
1364+
} else {
1365+
int reg = 0;
1366+
while (rlist != 0) {
1367+
if ((rlist & 1) != 0) {
1368+
if (load) {
1369+
set_register(static_cast<Register>(reg), ReadW(address, instr));
1370+
} else {
1371+
WriteW(address, get_register(static_cast<Register>(reg)), instr);
1372+
}
1373+
address += 4;
13581374
}
1359-
address += 4;
1375+
reg++;
1376+
rlist >>= 1;
13601377
}
1361-
reg++;
1362-
rlist >>= 1;
13631378
}
13641379
ASSERT(end_address == address);
13651380
}

runtime/vm/simulator_arm64.cc

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2407,8 +2407,18 @@ void Simulator::DecodeLoadStoreRegPair(Instr* instr) {
24072407
}
24082408
// Write to register.
24092409
if (instr->Bit(31) == 1) {
2410-
set_register(instr, rt, val1, R31IsZR);
2411-
set_register(instr, rt2, val2, R31IsZR);
2410+
if (rt == FP && rt2 == LR) {
2411+
// Special case `ldp fp, lr` for LeaveDartFrame so that the profiler
2412+
// does not get confused by observing the update to fp before lr when
2413+
// our caller is the entry stub, i.e., failing to identify the entry
2414+
// frame. On real hardware, the profiler's signal handler cannot
2415+
// observe a partially executed load-pair instruction.
2416+
set_register(instr, rt2, val2, R31IsZR);
2417+
set_register(instr, rt, val1, R31IsZR);
2418+
} else {
2419+
set_register(instr, rt, val1, R31IsZR);
2420+
set_register(instr, rt2, val2, R31IsZR);
2421+
}
24122422
} else {
24132423
set_wregister(rt, static_cast<int32_t>(val1), R31IsZR);
24142424
set_wregister(rt2, static_cast<int32_t>(val2), R31IsZR);

0 commit comments

Comments
 (0)