Skip to content

Commit dec11f5

Browse files
hashseedCommit bot
hashseed
authored and
Commit bot
committed
Debugger: make debug code on-stack replacement more robust.
The new implemtation counts the number of calls (or continuations) before the PC to find the corresponding PC in the new code. R=mstarzinger@chromium.org BUG=chromium:507070 LOG=N Review URL: https://codereview.chromium.org/1235603002 Cr-Commit-Position: refs/heads/master@{#29636}
1 parent 8600649 commit dec11f5

21 files changed

Lines changed: 150 additions & 73 deletions

‎src/arm/assembler-arm.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1357,6 +1357,9 @@ class Assembler : public AssemblerBase {
13571357
// Mark address of the ExitJSFrame code.
13581358
void RecordJSReturn();
13591359

1360+
// Mark generator continuation.
1361+
void RecordGeneratorContinuation();
1362+
13601363
// Mark address of a debug break slot.
13611364
void RecordDebugBreakSlot();
13621365
void RecordDebugBreakSlotForCall(int argc);

‎src/arm/full-codegen-arm.cc‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2172,8 +2172,8 @@ void FullCodeGenerator::VisitYield(Yield* expr) {
21722172
Label suspend, continuation, post_runtime, resume;
21732173

21742174
__ jmp(&suspend);
2175-
21762175
__ bind(&continuation);
2176+
__ RecordGeneratorContinuation();
21772177
__ jmp(&resume);
21782178

21792179
__ bind(&suspend);
@@ -2244,9 +2244,12 @@ void FullCodeGenerator::VisitYield(Yield* expr) {
22442244
EnterTryBlock(handler_index, &l_catch);
22452245
const int try_block_size = TryCatch::kElementCount * kPointerSize;
22462246
__ push(r0); // result
2247+
22472248
__ jmp(&l_suspend);
22482249
__ bind(&l_continuation);
2250+
__ RecordGeneratorContinuation();
22492251
__ jmp(&l_resume);
2252+
22502253
__ bind(&l_suspend);
22512254
const int generator_object_depth = kPointerSize + try_block_size;
22522255
__ ldr(r0, MemOperand(sp, generator_object_depth));

‎src/arm64/assembler-arm64.cc‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2905,16 +2905,15 @@ void Assembler::RecordRelocInfo(RelocInfo::Mode rmode, intptr_t data) {
29052905
(rmode <= RelocInfo::DEBUG_BREAK_SLOT_AT_CONSTRUCT_CALL)) ||
29062906
(rmode == RelocInfo::INTERNAL_REFERENCE) ||
29072907
(rmode == RelocInfo::CONST_POOL) || (rmode == RelocInfo::VENEER_POOL) ||
2908-
(rmode == RelocInfo::DEOPT_REASON)) {
2908+
(rmode == RelocInfo::DEOPT_REASON) ||
2909+
(rmode == RelocInfo::GENERATOR_CONTINUATION)) {
29092910
// Adjust code for new modes.
2910-
DCHECK(RelocInfo::IsDebugBreakSlot(rmode)
2911-
|| RelocInfo::IsJSReturn(rmode)
2912-
|| RelocInfo::IsComment(rmode)
2913-
|| RelocInfo::IsDeoptReason(rmode)
2914-
|| RelocInfo::IsPosition(rmode)
2915-
|| RelocInfo::IsInternalReference(rmode)
2916-
|| RelocInfo::IsConstPool(rmode)
2917-
|| RelocInfo::IsVeneerPool(rmode));
2911+
DCHECK(RelocInfo::IsDebugBreakSlot(rmode) || RelocInfo::IsJSReturn(rmode) ||
2912+
RelocInfo::IsComment(rmode) || RelocInfo::IsDeoptReason(rmode) ||
2913+
RelocInfo::IsPosition(rmode) ||
2914+
RelocInfo::IsInternalReference(rmode) ||
2915+
RelocInfo::IsConstPool(rmode) || RelocInfo::IsVeneerPool(rmode) ||
2916+
RelocInfo::IsGeneratorContinuation(rmode));
29182917
// These modes do not need an entry in the constant pool.
29192918
} else {
29202919
constpool_.RecordEntry(data, rmode);

‎src/arm64/assembler-arm64.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,6 +1025,9 @@ class Assembler : public AssemblerBase {
10251025
// Mark address of the ExitJSFrame code.
10261026
void RecordJSReturn();
10271027

1028+
// Mark generator continuation.
1029+
void RecordGeneratorContinuation();
1030+
10281031
// Mark address of a debug break slot.
10291032
void RecordDebugBreakSlot();
10301033
void RecordDebugBreakSlotForCall(int argc);

‎src/arm64/full-codegen-arm64.cc‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5099,11 +5099,11 @@ void FullCodeGenerator::VisitYield(Yield* expr) {
50995099
Label suspend, continuation, post_runtime, resume;
51005100

51015101
__ B(&suspend);
5102-
51035102
// TODO(jbramley): This label is bound here because the following code
51045103
// looks at its pos(). Is it possible to do something more efficient here,
51055104
// perhaps using Adr?
51065105
__ Bind(&continuation);
5106+
__ RecordGeneratorContinuation();
51075107
__ B(&resume);
51085108

51095109
__ Bind(&suspend);
@@ -5174,12 +5174,13 @@ void FullCodeGenerator::VisitYield(Yield* expr) {
51745174
EnterTryBlock(handler_index, &l_catch);
51755175
const int try_block_size = TryCatch::kElementCount * kPointerSize;
51765176
__ Push(x0); // result
5177-
__ B(&l_suspend);
51785177

5178+
__ B(&l_suspend);
51795179
// TODO(jbramley): This label is bound here because the following code
51805180
// looks at its pos(). Is it possible to do something more efficient here,
51815181
// perhaps using Adr?
51825182
__ Bind(&l_continuation);
5183+
__ RecordGeneratorContinuation();
51835184
__ B(&l_resume);
51845185

51855186
__ Bind(&l_suspend);

‎src/assembler.cc‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -774,7 +774,9 @@ const char* RelocInfo::RelocModeName(RelocInfo::Mode rmode) {
774774
case DEBUG_BREAK_SLOT_AT_CONSTRUCT_CALL:
775775
return "debug break slot at construct call";
776776
case CODE_AGE_SEQUENCE:
777-
return "code_age_sequence";
777+
return "code age sequence";
778+
case GENERATOR_CONTINUATION:
779+
return "generator continuation";
778780
case NUMBER_OF_MODES:
779781
case PC_JUMP:
780782
UNREACHABLE();
@@ -869,6 +871,7 @@ void RelocInfo::Verify(Isolate* isolate) {
869871
case DEBUG_BREAK_SLOT_AT_POSITION:
870872
case DEBUG_BREAK_SLOT_AT_CALL:
871873
case DEBUG_BREAK_SLOT_AT_CONSTRUCT_CALL:
874+
case GENERATOR_CONTINUATION:
872875
case NONE32:
873876
case NONE64:
874877
break;
@@ -1810,6 +1813,12 @@ void Assembler::RecordJSReturn() {
18101813
}
18111814

18121815

1816+
void Assembler::RecordGeneratorContinuation() {
1817+
EnsureSpace ensure_space(this);
1818+
RecordRelocInfo(RelocInfo::GENERATOR_CONTINUATION);
1819+
}
1820+
1821+
18131822
void Assembler::RecordDebugBreakSlot() {
18141823
EnsureSpace ensure_space(this);
18151824
RecordRelocInfo(RelocInfo::DEBUG_BREAK_SLOT_AT_POSITION);

‎src/assembler.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,9 @@ class RelocInfo {
386386
// Encoded internal reference, used only on MIPS, MIPS64 and PPC.
387387
INTERNAL_REFERENCE_ENCODED,
388388

389+
// Continuation points for a generator yield.
390+
GENERATOR_CONTINUATION,
391+
389392
// Marks constant and veneer pools. Only used on ARM and ARM64.
390393
// They use a custom noncompact encoding.
391394
CONST_POOL,
@@ -491,6 +494,9 @@ class RelocInfo {
491494
static inline bool IsCodeAgeSequence(Mode mode) {
492495
return mode == CODE_AGE_SEQUENCE;
493496
}
497+
static inline bool IsGeneratorContinuation(Mode mode) {
498+
return mode == GENERATOR_CONTINUATION;
499+
}
494500
static inline int ModeMask(Mode mode) { return 1 << mode; }
495501

496502
// Accessors

‎src/debug.cc‎

Lines changed: 51 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1436,56 +1436,58 @@ static void CollectActiveFunctionsFromThread(
14361436
}
14371437

14381438

1439-
// Figure out how many bytes of "pc_offset" correspond to actual code by
1440-
// subtracting off the bytes that correspond to constant/veneer pools. See
1441-
// Assembler::CheckConstPool() and Assembler::CheckVeneerPool(). Note that this
1442-
// is only useful for architectures using constant pools or veneer pools.
1443-
static int ComputeCodeOffsetFromPcOffset(Code *code, int pc_offset) {
1439+
// Count the number of calls before the current frame PC to find the
1440+
// corresponding PC in the newly recompiled code.
1441+
static Address ComputeNewPcForRedirect(Code* new_code, Code* old_code,
1442+
Address old_pc) {
1443+
DCHECK_EQ(old_code->kind(), Code::FUNCTION);
1444+
DCHECK_EQ(new_code->kind(), Code::FUNCTION);
1445+
DCHECK(!old_code->has_debug_break_slots());
1446+
DCHECK(new_code->has_debug_break_slots());
1447+
int mask = RelocInfo::kCodeTargetMask;
1448+
int index = 0;
1449+
intptr_t delta = 0;
1450+
for (RelocIterator it(old_code, mask); !it.done(); it.next()) {
1451+
RelocInfo* rinfo = it.rinfo();
1452+
Address current_pc = rinfo->pc();
1453+
// The frame PC is behind the call instruction by the call instruction size.
1454+
if (current_pc > old_pc) break;
1455+
index++;
1456+
delta = old_pc - current_pc;
1457+
}
1458+
1459+
RelocIterator it(new_code, mask);
1460+
for (int i = 1; i < index; i++) it.next();
1461+
return it.rinfo()->pc() + delta;
1462+
}
1463+
1464+
1465+
// Count the number of continuations at which the current pc offset is at.
1466+
static int ComputeContinuationIndexFromPcOffset(Code* code, int pc_offset) {
14441467
DCHECK_EQ(code->kind(), Code::FUNCTION);
14451468
DCHECK(!code->has_debug_break_slots());
1446-
DCHECK_LE(0, pc_offset);
1447-
DCHECK_LT(pc_offset, code->instruction_end() - code->instruction_start());
1448-
1449-
int mask = RelocInfo::ModeMask(RelocInfo::CONST_POOL) |
1450-
RelocInfo::ModeMask(RelocInfo::VENEER_POOL);
1451-
byte *pc = code->instruction_start() + pc_offset;
1452-
int code_offset = pc_offset;
1469+
Address pc = code->instruction_start() + pc_offset;
1470+
int mask = RelocInfo::ModeMask(RelocInfo::GENERATOR_CONTINUATION);
1471+
int index = 0;
14531472
for (RelocIterator it(code, mask); !it.done(); it.next()) {
1454-
RelocInfo* info = it.rinfo();
1455-
if (info->pc() >= pc) break;
1456-
DCHECK(RelocInfo::IsConstPool(info->rmode()));
1457-
code_offset -= static_cast<int>(info->data());
1458-
DCHECK_LE(0, code_offset);
1473+
index++;
1474+
RelocInfo* rinfo = it.rinfo();
1475+
Address current_pc = rinfo->pc();
1476+
if (current_pc == pc) break;
1477+
DCHECK(current_pc < pc);
14591478
}
1460-
1461-
return code_offset;
1479+
return index;
14621480
}
14631481

14641482

1465-
// The inverse of ComputeCodeOffsetFromPcOffset.
1466-
static int ComputePcOffsetFromCodeOffset(Code *code, int code_offset) {
1483+
// Find the pc offset for the given continuation index.
1484+
static int ComputePcOffsetFromContinuationIndex(Code* code, int index) {
14671485
DCHECK_EQ(code->kind(), Code::FUNCTION);
1468-
1469-
int mask = RelocInfo::kDebugBreakSlotMask |
1470-
RelocInfo::ModeMask(RelocInfo::CONST_POOL) |
1471-
RelocInfo::ModeMask(RelocInfo::VENEER_POOL);
1472-
int reloc = 0;
1473-
for (RelocIterator it(code, mask); !it.done(); it.next()) {
1474-
RelocInfo* info = it.rinfo();
1475-
if (info->pc() - code->instruction_start() - reloc >= code_offset) break;
1476-
if (RelocInfo::IsDebugBreakSlot(info->rmode())) {
1477-
reloc += Assembler::kDebugBreakSlotLength;
1478-
} else {
1479-
DCHECK(RelocInfo::IsConstPool(info->rmode()));
1480-
reloc += static_cast<int>(info->data());
1481-
}
1482-
}
1483-
1484-
int pc_offset = code_offset + reloc;
1485-
1486-
DCHECK_LT(code->instruction_start() + pc_offset, code->instruction_end());
1487-
1488-
return pc_offset;
1486+
DCHECK(code->has_debug_break_slots());
1487+
int mask = RelocInfo::ModeMask(RelocInfo::GENERATOR_CONTINUATION);
1488+
RelocIterator it(code, mask);
1489+
for (int i = 1; i < index; i++) it.next();
1490+
return static_cast<int>(it.rinfo()->pc() - code->instruction_start());
14891491
}
14901492

14911493

@@ -1510,13 +1512,8 @@ static void RedirectActivationsToRecompiledCodeOnThread(
15101512
continue;
15111513
}
15121514

1513-
int old_pc_offset =
1514-
static_cast<int>(frame->pc() - frame_code->instruction_start());
1515-
int code_offset = ComputeCodeOffsetFromPcOffset(*frame_code, old_pc_offset);
1516-
int new_pc_offset = ComputePcOffsetFromCodeOffset(*new_code, code_offset);
1517-
1518-
// Compute the equivalent pc in the new code.
1519-
byte* new_pc = new_code->instruction_start() + new_pc_offset;
1515+
Address new_pc =
1516+
ComputeNewPcForRedirect(*new_code, *frame_code, frame->pc());
15201517

15211518
if (FLAG_trace_deopt) {
15221519
PrintF("Replacing code %08" V8PRIxPTR " - %08" V8PRIxPTR " (%d) "
@@ -1603,8 +1600,8 @@ static void RecompileAndRelocateSuspendedGenerators(
16031600

16041601
EnsureFunctionHasDebugBreakSlots(fun);
16051602

1606-
int code_offset = generators[i]->continuation();
1607-
int pc_offset = ComputePcOffsetFromCodeOffset(fun->code(), code_offset);
1603+
int index = generators[i]->continuation();
1604+
int pc_offset = ComputePcOffsetFromContinuationIndex(fun->code(), index);
16081605
generators[i]->set_continuation(pc_offset);
16091606
}
16101607
}
@@ -1726,11 +1723,11 @@ void Debug::PrepareForBreakPoints() {
17261723
int pc_offset = gen->continuation();
17271724
DCHECK_LT(0, pc_offset);
17281725

1729-
int code_offset =
1730-
ComputeCodeOffsetFromPcOffset(fun->code(), pc_offset);
1726+
int index =
1727+
ComputeContinuationIndexFromPcOffset(fun->code(), pc_offset);
17311728

17321729
// This will be fixed after we recompile the functions.
1733-
gen->set_continuation(code_offset);
1730+
gen->set_continuation(index);
17341731

17351732
suspended_generators.Add(Handle<JSGeneratorObject>(gen, isolate_));
17361733
} else if (obj->IsSharedFunctionInfo()) {

‎src/ia32/assembler-ia32.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1436,6 +1436,9 @@ class Assembler : public AssemblerBase {
14361436
// Mark address of the ExitJSFrame code.
14371437
void RecordJSReturn();
14381438

1439+
// Mark generator continuation.
1440+
void RecordGeneratorContinuation();
1441+
14391442
// Mark address of a debug break slot.
14401443
void RecordDebugBreakSlot();
14411444
void RecordDebugBreakSlotForCall(int argc);

‎src/ia32/full-codegen-ia32.cc‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2093,8 +2093,8 @@ void FullCodeGenerator::VisitYield(Yield* expr) {
20932093
Label suspend, continuation, post_runtime, resume;
20942094

20952095
__ jmp(&suspend);
2096-
20972096
__ bind(&continuation);
2097+
__ RecordGeneratorContinuation();
20982098
__ jmp(&resume);
20992099

21002100
__ bind(&suspend);
@@ -2167,9 +2167,12 @@ void FullCodeGenerator::VisitYield(Yield* expr) {
21672167
EnterTryBlock(handler_index, &l_catch);
21682168
const int try_block_size = TryCatch::kElementCount * kPointerSize;
21692169
__ push(eax); // result
2170+
21702171
__ jmp(&l_suspend);
21712172
__ bind(&l_continuation);
2173+
__ RecordGeneratorContinuation();
21722174
__ jmp(&l_resume);
2175+
21732176
__ bind(&l_suspend);
21742177
const int generator_object_depth = kPointerSize + try_block_size;
21752178
__ mov(eax, Operand(esp, generator_object_depth));

0 commit comments

Comments
 (0)