Skip to content

Commit 7766baf

Browse files
committed
deps: cherry-pick ba752ea from upstream V8
Original commit message: [cpu-profiler] Use instruction start as the key for the CodeMap Previously we used the start address of the AbstractCode object. This doesn't make sense for off-heap builtins, where the code isn't contained in the object itself. It also hides other potential problems - sometimes the sample.pc is inside the AbstractCode object header - this is never valid. There were a few changes necessary to make this happen: - Change the interface of CodeMoveEvent. Now 'to' and 'from' are both AbstractCode objects, which is nice because many users were taking 'to' and adding the header offset to it to try and find the instruction start address. This isn't valid for off-heap builtins. - Fix a bug in CodeMap::MoveCode where we didn't update the CodeEntry object to reflect the new instruction_start. - Rename the 'start' field in all of the CodeEventRecord sub-classes to make it clear that this is the address of the first instruction. - Fix the confusion in RecordTickSample between 'tos' and 'pc' which caused pc_offset to be calculated incorrectly. Bug: v8:7983 Change-Id: I3e9dddf74e4b2e96a5f031d216ef7008d6f184d1 Reviewed-on: https://chromium-review.googlesource.com/1148457 Commit-Queue: Peter Marshall <petermarshall@chromium.org> Reviewed-by: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Cr-Commit-Position: refs/heads/master@{#54749} Refs: v8/v8@ba752ea PR-URL: #21983 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 56d7411 commit 7766baf

18 files changed

+102
-88
lines changed

common.gypi

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929

3030
# Reset this number to 0 on major V8 upgrades.
3131
# Increment by one for each non-official patch applied to deps/v8.
32-
'v8_embedder_string': '-node.5',
32+
'v8_embedder_string': '-node.6',
3333

3434
# Enable disassembler for `--print-code` v8 options
3535
'v8_enable_disassembler': 1,

deps/v8/src/code-events.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ class CodeEventListener {
8383
virtual void GetterCallbackEvent(Name* name, Address entry_point) = 0;
8484
virtual void SetterCallbackEvent(Name* name, Address entry_point) = 0;
8585
virtual void RegExpCodeCreateEvent(AbstractCode* code, String* source) = 0;
86-
virtual void CodeMoveEvent(AbstractCode* from, Address to) = 0;
86+
virtual void CodeMoveEvent(AbstractCode* from, AbstractCode* to) = 0;
8787
virtual void SharedFunctionInfoMoveEvent(Address from, Address to) = 0;
8888
virtual void CodeMovingGCEvent() = 0;
8989
virtual void CodeDisableOptEvent(AbstractCode* code,
@@ -154,7 +154,7 @@ class CodeEventDispatcher {
154154
void RegExpCodeCreateEvent(AbstractCode* code, String* source) {
155155
CODE_EVENT_DISPATCH(RegExpCodeCreateEvent(code, source));
156156
}
157-
void CodeMoveEvent(AbstractCode* from, Address to) {
157+
void CodeMoveEvent(AbstractCode* from, AbstractCode* to) {
158158
CODE_EVENT_DISPATCH(CodeMoveEvent(from, to));
159159
}
160160
void SharedFunctionInfoMoveEvent(Address from, Address to) {

deps/v8/src/heap/mark-compact.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -1149,7 +1149,7 @@ class ProfilingMigrationObserver final : public MigrationObserver {
11491149
int size) final {
11501150
if (dest == CODE_SPACE || (dest == OLD_SPACE && dst->IsBytecodeArray())) {
11511151
PROFILE(heap_->isolate(),
1152-
CodeMoveEvent(AbstractCode::cast(src), dst->address()));
1152+
CodeMoveEvent(AbstractCode::cast(src), AbstractCode::cast(dst)));
11531153
}
11541154
heap_->OnMoveEvent(dst, src, size);
11551155
}

deps/v8/src/log.cc

+10-15
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ class PerfBasicLogger : public CodeEventLogger {
270270
explicit PerfBasicLogger(Isolate* isolate);
271271
~PerfBasicLogger() override;
272272

273-
void CodeMoveEvent(AbstractCode* from, Address to) override {}
273+
void CodeMoveEvent(AbstractCode* from, AbstractCode* to) override {}
274274
void CodeDisableOptEvent(AbstractCode* code,
275275
SharedFunctionInfo* shared) override {}
276276

@@ -496,7 +496,7 @@ class LowLevelLogger : public CodeEventLogger {
496496
LowLevelLogger(Isolate* isolate, const char* file_name);
497497
~LowLevelLogger() override;
498498

499-
void CodeMoveEvent(AbstractCode* from, Address to) override;
499+
void CodeMoveEvent(AbstractCode* from, AbstractCode* to) override;
500500
void CodeDisableOptEvent(AbstractCode* code,
501501
SharedFunctionInfo* shared) override {}
502502
void SnapshotPositionEvent(HeapObject* obj, int pos);
@@ -615,11 +615,10 @@ void LowLevelLogger::LogRecordedBuffer(const wasm::WasmCode* code,
615615
code->instructions().length());
616616
}
617617

618-
void LowLevelLogger::CodeMoveEvent(AbstractCode* from, Address to) {
618+
void LowLevelLogger::CodeMoveEvent(AbstractCode* from, AbstractCode* to) {
619619
CodeMoveStruct event;
620620
event.from_address = from->InstructionStart();
621-
size_t header_size = from->InstructionStart() - from->address();
622-
event.to_address = to + header_size;
621+
event.to_address = to->InstructionStart();
623622
LogWriteStruct(event);
624623
}
625624

@@ -641,7 +640,7 @@ class JitLogger : public CodeEventLogger {
641640
public:
642641
JitLogger(Isolate* isolate, JitCodeEventHandler code_event_handler);
643642

644-
void CodeMoveEvent(AbstractCode* from, Address to) override;
643+
void CodeMoveEvent(AbstractCode* from, AbstractCode* to) override;
645644
void CodeDisableOptEvent(AbstractCode* code,
646645
SharedFunctionInfo* shared) override {}
647646
void AddCodeLinePosInfoEvent(void* jit_handler_data, int pc_offset,
@@ -700,7 +699,7 @@ void JitLogger::LogRecordedBuffer(const wasm::WasmCode* code, const char* name,
700699
code_event_handler_(&event);
701700
}
702701

703-
void JitLogger::CodeMoveEvent(AbstractCode* from, Address to) {
702+
void JitLogger::CodeMoveEvent(AbstractCode* from, AbstractCode* to) {
704703
base::LockGuard<base::Mutex> guard(&logger_mutex_);
705704

706705
JitCodeEvent event;
@@ -709,12 +708,7 @@ void JitLogger::CodeMoveEvent(AbstractCode* from, Address to) {
709708
from->IsCode() ? JitCodeEvent::JIT_CODE : JitCodeEvent::BYTE_CODE;
710709
event.code_start = reinterpret_cast<void*>(from->InstructionStart());
711710
event.code_len = from->InstructionSize();
712-
713-
// Calculate the header size.
714-
const size_t header_size = from->InstructionStart() - from->address();
715-
716-
// Calculate the new start address of the instructions.
717-
event.new_code_start = reinterpret_cast<void*>(to + header_size);
711+
event.new_code_start = reinterpret_cast<void*>(to->InstructionStart());
718712
event.isolate = reinterpret_cast<v8::Isolate*>(isolate_);
719713

720714
code_event_handler_(&event);
@@ -1431,9 +1425,10 @@ void Logger::RegExpCodeCreateEvent(AbstractCode* code, String* source) {
14311425
msg.WriteToLogFile();
14321426
}
14331427

1434-
void Logger::CodeMoveEvent(AbstractCode* from, Address to) {
1428+
void Logger::CodeMoveEvent(AbstractCode* from, AbstractCode* to) {
14351429
if (!is_listening_to_code_events()) return;
1436-
MoveEventInternal(CodeEventListener::CODE_MOVE_EVENT, from->address(), to);
1430+
MoveEventInternal(CodeEventListener::CODE_MOVE_EVENT, from->address(),
1431+
to->address());
14371432
}
14381433

14391434
namespace {

deps/v8/src/log.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ class Logger : public CodeEventListener {
222222
// Emits a code create event for a RegExp.
223223
void RegExpCodeCreateEvent(AbstractCode* code, String* source);
224224
// Emits a code move event.
225-
void CodeMoveEvent(AbstractCode* from, Address to);
225+
void CodeMoveEvent(AbstractCode* from, AbstractCode* to);
226226
// Emits a code line info record event.
227227
void CodeLinePosInfoRecordEvent(Address code_start,
228228
ByteArray* source_position_table);
@@ -486,7 +486,7 @@ class ExternalCodeEventListener : public CodeEventListener {
486486
void GetterCallbackEvent(Name* name, Address entry_point) override {}
487487
void SetterCallbackEvent(Name* name, Address entry_point) override {}
488488
void SharedFunctionInfoMoveEvent(Address from, Address to) override {}
489-
void CodeMoveEvent(AbstractCode* from, Address to) override {}
489+
void CodeMoveEvent(AbstractCode* from, AbstractCode* to) override {}
490490
void CodeDisableOptEvent(AbstractCode* code,
491491
SharedFunctionInfo* shared) override {}
492492
void CodeMovingGCEvent() override {}

deps/v8/src/perf-jit.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,7 @@ void PerfJitLogger::LogWriteUnwindingInfo(Code* code) {
420420
LogWriteBytes(padding_bytes, static_cast<int>(padding_size));
421421
}
422422

423-
void PerfJitLogger::CodeMoveEvent(AbstractCode* from, Address to) {
423+
void PerfJitLogger::CodeMoveEvent(AbstractCode* from, AbstractCode* to) {
424424
// We may receive a CodeMove event if a BytecodeArray object moves. Otherwise
425425
// code relocation is not supported.
426426
CHECK(from->IsBytecodeArray());

deps/v8/src/perf-jit.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class PerfJitLogger : public CodeEventLogger {
4141
explicit PerfJitLogger(Isolate* isolate);
4242
virtual ~PerfJitLogger();
4343

44-
void CodeMoveEvent(AbstractCode* from, Address to) override;
44+
void CodeMoveEvent(AbstractCode* from, AbstractCode* to) override;
4545
void CodeDisableOptEvent(AbstractCode* code,
4646
SharedFunctionInfo* shared) override {}
4747

@@ -120,7 +120,7 @@ class PerfJitLogger : public CodeEventLogger {
120120
public:
121121
explicit PerfJitLogger(Isolate* isolate) : CodeEventLogger(isolate) {}
122122

123-
void CodeMoveEvent(AbstractCode* from, Address to) override {
123+
void CodeMoveEvent(AbstractCode* from, AbstractCode* to) override {
124124
UNIMPLEMENTED();
125125
}
126126

deps/v8/src/profiler/cpu-profiler-inl.h

+5-5
Original file line numberDiff line numberDiff line change
@@ -16,25 +16,25 @@ namespace v8 {
1616
namespace internal {
1717

1818
void CodeCreateEventRecord::UpdateCodeMap(CodeMap* code_map) {
19-
code_map->AddCode(start, entry, size);
19+
code_map->AddCode(instruction_start, entry, instruction_size);
2020
}
2121

2222

2323
void CodeMoveEventRecord::UpdateCodeMap(CodeMap* code_map) {
24-
code_map->MoveCode(from, to);
24+
code_map->MoveCode(from_instruction_start, to_instruction_start);
2525
}
2626

2727

2828
void CodeDisableOptEventRecord::UpdateCodeMap(CodeMap* code_map) {
29-
CodeEntry* entry = code_map->FindEntry(start);
29+
CodeEntry* entry = code_map->FindEntry(instruction_start);
3030
if (entry != nullptr) {
3131
entry->set_bailout_reason(bailout_reason);
3232
}
3333
}
3434

3535

3636
void CodeDeoptEventRecord::UpdateCodeMap(CodeMap* code_map) {
37-
CodeEntry* entry = code_map->FindEntry(start);
37+
CodeEntry* entry = code_map->FindEntry(instruction_start);
3838
if (entry == nullptr) return;
3939
std::vector<CpuProfileDeoptFrame> frames_vector(
4040
deopt_frames, deopt_frames + deopt_frame_count);
@@ -44,7 +44,7 @@ void CodeDeoptEventRecord::UpdateCodeMap(CodeMap* code_map) {
4444

4545

4646
void ReportBuiltinEventRecord::UpdateCodeMap(CodeMap* code_map) {
47-
CodeEntry* entry = code_map->FindEntry(start);
47+
CodeEntry* entry = code_map->FindEntry(instruction_start);
4848
if (!entry) {
4949
// Code objects for builtins should already have been added to the map but
5050
// some of them have been filtered out by CpuProfiler.

deps/v8/src/profiler/cpu-profiler.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ void CpuProfiler::LogBuiltins() {
426426
CodeEventsContainer evt_rec(CodeEventRecord::REPORT_BUILTIN);
427427
ReportBuiltinEventRecord* rec = &evt_rec.ReportBuiltinEventRecord_;
428428
Builtins::Name id = static_cast<Builtins::Name>(i);
429-
rec->start = builtins->builtin(id)->address();
429+
rec->instruction_start = builtins->builtin(id)->InstructionStart();
430430
rec->builtin_id = id;
431431
processor_->Enqueue(evt_rec);
432432
}

deps/v8/src/profiler/cpu-profiler.h

+7-7
Original file line numberDiff line numberDiff line change
@@ -53,26 +53,26 @@ class CodeEventRecord {
5353

5454
class CodeCreateEventRecord : public CodeEventRecord {
5555
public:
56-
Address start;
56+
Address instruction_start;
5757
CodeEntry* entry;
58-
unsigned size;
58+
unsigned instruction_size;
5959

6060
V8_INLINE void UpdateCodeMap(CodeMap* code_map);
6161
};
6262

6363

6464
class CodeMoveEventRecord : public CodeEventRecord {
6565
public:
66-
Address from;
67-
Address to;
66+
Address from_instruction_start;
67+
Address to_instruction_start;
6868

6969
V8_INLINE void UpdateCodeMap(CodeMap* code_map);
7070
};
7171

7272

7373
class CodeDisableOptEventRecord : public CodeEventRecord {
7474
public:
75-
Address start;
75+
Address instruction_start;
7676
const char* bailout_reason;
7777

7878
V8_INLINE void UpdateCodeMap(CodeMap* code_map);
@@ -81,7 +81,7 @@ class CodeDisableOptEventRecord : public CodeEventRecord {
8181

8282
class CodeDeoptEventRecord : public CodeEventRecord {
8383
public:
84-
Address start;
84+
Address instruction_start;
8585
const char* deopt_reason;
8686
int deopt_id;
8787
Address pc;
@@ -95,7 +95,7 @@ class CodeDeoptEventRecord : public CodeEventRecord {
9595

9696
class ReportBuiltinEventRecord : public CodeEventRecord {
9797
public:
98-
Address start;
98+
Address instruction_start;
9999
Builtins::Name builtin_id;
100100

101101
V8_INLINE void UpdateCodeMap(CodeMap* code_map);

deps/v8/src/profiler/profile-generator.cc

+28-13
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,8 @@ void CodeMap::AddCode(Address addr, CodeEntry* entry, unsigned size) {
529529
ClearCodesInRange(addr, addr + size);
530530
unsigned index = AddCodeEntry(addr, entry);
531531
code_map_.emplace(addr, CodeEntryMapInfo{index, size});
532+
DCHECK(entry->instruction_start() == kNullAddress ||
533+
addr == entry->instruction_start());
532534
}
533535

534536
void CodeMap::ClearCodesInRange(Address start, Address end) {
@@ -550,8 +552,14 @@ CodeEntry* CodeMap::FindEntry(Address addr) {
550552
auto it = code_map_.upper_bound(addr);
551553
if (it == code_map_.begin()) return nullptr;
552554
--it;
553-
Address end_address = it->first + it->second.size;
554-
return addr < end_address ? entry(it->second.index) : nullptr;
555+
Address start_address = it->first;
556+
Address end_address = start_address + it->second.size;
557+
CodeEntry* ret = addr < end_address ? entry(it->second.index) : nullptr;
558+
if (ret && ret->instruction_start() != kNullAddress) {
559+
DCHECK_EQ(start_address, ret->instruction_start());
560+
DCHECK(addr >= start_address && addr < end_address);
561+
}
562+
return ret;
555563
}
556564

557565
void CodeMap::MoveCode(Address from, Address to) {
@@ -563,6 +571,9 @@ void CodeMap::MoveCode(Address from, Address to) {
563571
DCHECK(from + info.size <= to || to + info.size <= from);
564572
ClearCodesInRange(to, to + info.size);
565573
code_map_.emplace(to, info);
574+
575+
CodeEntry* entry = code_entries_[info.index].entry;
576+
entry->set_instruction_start(to);
566577
}
567578

568579
unsigned CodeMap::AddCodeEntry(Address start, CodeEntry* entry) {
@@ -693,26 +704,29 @@ void ProfileGenerator::RecordTickSample(const TickSample& sample) {
693704
if (sample.pc != nullptr) {
694705
if (sample.has_external_callback && sample.state == EXTERNAL) {
695706
// Don't use PC when in external callback code, as it can point
696-
// inside callback's code, and we will erroneously report
707+
// inside a callback's code, and we will erroneously report
697708
// that a callback calls itself.
698709
stack_trace.push_back(
699710
{FindEntry(reinterpret_cast<Address>(sample.external_callback_entry)),
700711
no_line_info});
701712
} else {
702-
CodeEntry* pc_entry = FindEntry(reinterpret_cast<Address>(sample.pc));
703-
// If there is no pc_entry we're likely in native code.
704-
// Find out, if top of stack was pointing inside a JS function
705-
// meaning that we have encountered a frameless invocation.
713+
Address attributed_pc = reinterpret_cast<Address>(sample.pc);
714+
CodeEntry* pc_entry = FindEntry(attributed_pc);
715+
// If there is no pc_entry, we're likely in native code. Find out if the
716+
// top of the stack (the return address) was pointing inside a JS
717+
// function, meaning that we have encountered a frameless invocation.
706718
if (!pc_entry && !sample.has_external_callback) {
707-
pc_entry = FindEntry(reinterpret_cast<Address>(sample.tos));
719+
attributed_pc = reinterpret_cast<Address>(sample.tos);
720+
pc_entry = FindEntry(attributed_pc);
708721
}
709722
// If pc is in the function code before it set up stack frame or after the
710-
// frame was destroyed SafeStackFrameIterator incorrectly thinks that
711-
// ebp contains return address of the current function and skips caller's
712-
// frame. Check for this case and just skip such samples.
723+
// frame was destroyed, SafeStackFrameIterator incorrectly thinks that
724+
// ebp contains the return address of the current function and skips the
725+
// caller's frame. Check for this case and just skip such samples.
713726
if (pc_entry) {
714-
int pc_offset = static_cast<int>(reinterpret_cast<Address>(sample.pc) -
715-
pc_entry->instruction_start());
727+
int pc_offset =
728+
static_cast<int>(attributed_pc - pc_entry->instruction_start());
729+
DCHECK_GE(pc_offset, 0);
716730
src_line = pc_entry->GetSourceLine(pc_offset);
717731
if (src_line == v8::CpuProfileNode::kNoLineNumberInfo) {
718732
src_line = pc_entry->line_number();
@@ -744,6 +758,7 @@ void ProfileGenerator::RecordTickSample(const TickSample& sample) {
744758
// Find out if the entry has an inlining stack associated.
745759
int pc_offset =
746760
static_cast<int>(stack_pos - entry->instruction_start());
761+
DCHECK_GE(pc_offset, 0);
747762
const std::vector<std::unique_ptr<CodeEntry>>* inline_stack =
748763
entry->GetInlineStack(pc_offset);
749764
if (inline_stack) {

deps/v8/src/profiler/profile-generator.h

+2
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,9 @@ class CodeEntry {
108108
const std::vector<std::unique_ptr<CodeEntry>>* GetInlineStack(
109109
int pc_offset) const;
110110

111+
void set_instruction_start(Address start) { instruction_start_ = start; }
111112
Address instruction_start() const { return instruction_start_; }
113+
112114
CodeEventListener::LogEventsAndTags tag() const {
113115
return TagField::decode(bit_field_);
114116
}

0 commit comments

Comments
 (0)