[perf-jit] Don't use 0xFF for repeated script names

This was never supported to start with and can cause invalid script names.
This CL partially reverts https://crrev.com/c/3513892

Drive-by-fix: Dehandlify more code.
Change-Id: I96cf4c1244d9f00dc47738cd481b440e6bed0541
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4174074
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Jakob Linke <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/main@{#85609}
This commit is contained in:
Camillo Bruni 2023-02-01 20:13:02 +01:00 committed by V8 LUCI CQ
parent 972d103e4e
commit 7eff3cee05
8 changed files with 74 additions and 64 deletions

View File

@ -79,10 +79,10 @@ std::vector<SourcePositionInfo> SourcePosition::InliningStack(Isolate* isolate,
}
SourcePositionInfo SourcePosition::FirstInfo(Isolate* isolate,
Handle<Code> code) const {
Code code) const {
DisallowGarbageCollection no_gc;
DeoptimizationData deopt_data =
DeoptimizationData::cast(code->deoptimization_data());
DeoptimizationData::cast(code.deoptimization_data());
SourcePosition pos = *this;
if (pos.isInlined()) {
InliningPosition inl = deopt_data.InliningPositions().get(pos.InliningId());

View File

@ -83,7 +83,7 @@ class SourcePosition final {
Code code) const;
std::vector<SourcePositionInfo> InliningStack(
OptimizedCompilationInfo* cinfo) const;
SourcePositionInfo FirstInfo(Isolate* isolate, Handle<Code> code) const;
SourcePositionInfo FirstInfo(Isolate* isolate, Code code) const;
void Print(std::ostream& out, InstructionStream code) const;
void PrintJson(std::ostream& out) const;

View File

@ -117,7 +117,6 @@ const char LinuxPerfJitLogger::kFilenameFormatString[] = "./jit-%d.dump";
const int LinuxPerfJitLogger::kFilenameBufferPadding = 16;
static const char kStringTerminator[] = {'\0'};
static const char kRepeatedNameMarker[] = {'\xff', '\0'};
base::LazyRecursiveMutex LinuxPerfJitLogger::file_mutex_;
// The following static variables are protected by
@ -214,11 +213,11 @@ uint64_t LinuxPerfJitLogger::GetTimestamp() {
}
void LinuxPerfJitLogger::LogRecordedBuffer(
Handle<AbstractCode> abstract_code,
MaybeHandle<SharedFunctionInfo> maybe_shared, const char* name,
int length) {
AbstractCode abstract_code, MaybeHandle<SharedFunctionInfo> maybe_shared,
const char* name, int length) {
DisallowGarbageCollection no_gc;
if (v8_flags.perf_basic_prof_only_functions) {
CodeKind code_kind = abstract_code->kind(isolate_);
CodeKind code_kind = abstract_code.kind(isolate_);
if (code_kind != CodeKind::INTERPRETED_FUNCTION &&
code_kind != CodeKind::TURBOFAN && code_kind != CodeKind::MAGLEV &&
code_kind != CodeKind::BASELINE) {
@ -231,26 +230,27 @@ void LinuxPerfJitLogger::LogRecordedBuffer(
if (perf_output_handle_ == nullptr) return;
// We only support non-interpreted functions.
if (!abstract_code->IsCode(isolate_)) return;
Handle<Code> code = Handle<Code>::cast(abstract_code);
if (!abstract_code.IsCode(isolate_)) return;
Code code = Code::cast(abstract_code);
// Debug info has to be emitted first.
Handle<SharedFunctionInfo> shared;
if (v8_flags.perf_prof && maybe_shared.ToHandle(&shared)) {
// TODO(herhut): This currently breaks for js2wasm/wasm2js functions.
if (code->kind() != CodeKind::JS_TO_WASM_FUNCTION &&
code->kind() != CodeKind::WASM_TO_JS_FUNCTION) {
CodeKind kind = code.kind();
if (kind != CodeKind::JS_TO_WASM_FUNCTION &&
kind != CodeKind::WASM_TO_JS_FUNCTION) {
LogWriteDebugInfo(code, shared);
}
}
const char* code_name = name;
uint8_t* code_pointer = reinterpret_cast<uint8_t*>(code->InstructionStart());
uint8_t* code_pointer = reinterpret_cast<uint8_t*>(code.InstructionStart());
// Unwinding info comes right after debug info.
if (v8_flags.perf_prof_unwinding_info) LogWriteUnwindingInfo(*code);
if (v8_flags.perf_prof_unwinding_info) LogWriteUnwindingInfo(code);
WriteJitCodeLoadEntry(code_pointer, code->InstructionSize(), code_name,
WriteJitCodeLoadEntry(code_pointer, code.InstructionSize(), code_name,
length);
}
@ -319,11 +319,11 @@ base::Vector<const char> GetScriptName(Object maybeScript,
} // namespace
SourcePositionInfo GetSourcePositionInfo(Isolate* isolate, Handle<Code> code,
SourcePositionInfo GetSourcePositionInfo(Isolate* isolate, Code code,
Handle<SharedFunctionInfo> function,
SourcePosition pos) {
DisallowGarbageCollection disallow;
if (code->is_turbofanned()) {
if (code.is_turbofanned()) {
return pos.FirstInfo(isolate, code);
} else {
return SourcePositionInfo(pos, function);
@ -332,23 +332,25 @@ SourcePositionInfo GetSourcePositionInfo(Isolate* isolate, Handle<Code> code,
} // namespace
void LinuxPerfJitLogger::LogWriteDebugInfo(Handle<Code> code,
void LinuxPerfJitLogger::LogWriteDebugInfo(Code code,
Handle<SharedFunctionInfo> shared) {
// Line ends of all scripts have been initialized prior to this.
DisallowGarbageCollection no_gc;
// The WasmToJS wrapper stubs have source position entries.
if (!shared->HasSourceCode()) return;
SharedFunctionInfo raw_shared = *shared;
if (!raw_shared.HasSourceCode()) return;
PerfJitCodeDebugInfo debug_info;
uint32_t size = sizeof(debug_info);
ByteArray source_position_table =
code->SourcePositionTable(isolate_, *shared);
code.SourcePositionTable(isolate_, raw_shared);
// Compute the entry count and get the names of all scripts.
// Avoid additional work if the script name is repeated. Multiple script
// names only occur for cross-script inlining.
uint32_t entry_count = 0;
Object last_script = Smi::zero();
size_t last_script_name_size = 0;
std::vector<base::Vector<const char>> script_names;
for (SourcePositionTableIterator iterator(source_position_table);
!iterator.done(); iterator.Advance()) {
@ -357,13 +359,15 @@ void LinuxPerfJitLogger::LogWriteDebugInfo(Handle<Code> code,
Object current_script = *info.script;
if (current_script != last_script) {
std::unique_ptr<char[]> name_storage;
auto name = GetScriptName(shared->script(), &name_storage, no_gc);
auto name = GetScriptName(raw_shared.script(), &name_storage, no_gc);
script_names.push_back(name);
// Add the size of the name after each entry.
size += name.size() + sizeof(kStringTerminator);
last_script_name_size = name.size() + sizeof(kStringTerminator);
size += last_script_name_size;
last_script = current_script;
} else {
size += sizeof(kRepeatedNameMarker);
DCHECK_LT(0, last_script_name_size);
size += last_script_name_size;
}
entry_count++;
}
@ -371,7 +375,7 @@ void LinuxPerfJitLogger::LogWriteDebugInfo(Handle<Code> code,
debug_info.event_ = PerfJitCodeLoad::kDebugInfo;
debug_info.time_stamp_ = GetTimestamp();
debug_info.address_ = code->InstructionStart();
debug_info.address_ = code.InstructionStart();
debug_info.entry_count_ = entry_count;
// Add the sizes of fixed parts of entries.
@ -381,7 +385,7 @@ void LinuxPerfJitLogger::LogWriteDebugInfo(Handle<Code> code,
debug_info.size_ = size + padding;
LogWriteBytes(reinterpret_cast<const char*>(&debug_info), sizeof(debug_info));
Address code_start = code->InstructionStart();
Address code_start = code.InstructionStart();
last_script = Smi::zero();
int script_names_index = 0;
@ -398,16 +402,13 @@ void LinuxPerfJitLogger::LogWriteDebugInfo(Handle<Code> code,
entry.column_ = info.column + 1;
LogWriteBytes(reinterpret_cast<const char*>(&entry), sizeof(entry));
Object current_script = *info.script;
if (current_script != last_script) {
auto name_string = script_names[script_names_index];
LogWriteBytes(name_string.begin(),
static_cast<uint32_t>(name_string.size()));
LogWriteBytes(kStringTerminator, sizeof(kStringTerminator));
script_names_index++;
if (current_script != last_script) {
if (last_script != Smi::zero()) script_names_index++;
last_script = current_script;
} else {
// Use the much shorter kRepeatedNameMarker for repeated names.
LogWriteBytes(kRepeatedNameMarker, sizeof(kRepeatedNameMarker));
}
}
char padding_bytes[8] = {0};

View File

@ -58,7 +58,7 @@ class LinuxPerfJitLogger : public CodeEventLogger {
void CloseMarkerFile(void* marker_address);
uint64_t GetTimestamp();
void LogRecordedBuffer(Handle<AbstractCode> code,
void LogRecordedBuffer(AbstractCode code,
MaybeHandle<SharedFunctionInfo> maybe_shared,
const char* name, int length) override;
#if V8_ENABLE_WEBASSEMBLY
@ -79,7 +79,7 @@ class LinuxPerfJitLogger : public CodeEventLogger {
void LogWriteBytes(const char* bytes, int size);
void LogWriteHeader();
void LogWriteDebugInfo(Handle<Code> code, Handle<SharedFunctionInfo> shared);
void LogWriteDebugInfo(Code code, Handle<SharedFunctionInfo> shared);
#if V8_ENABLE_WEBASSEMBLY
void LogWriteDebugInfo(const wasm::WasmCode* code);
#endif // V8_ENABLE_WEBASSEMBLY

View File

@ -19,6 +19,7 @@
#include "src/codegen/bailout-reason.h"
#include "src/codegen/macro-assembler.h"
#include "src/codegen/source-position-table.h"
#include "src/common/assert-scope.h"
#include "src/deoptimizer/deoptimizer.h"
#include "src/diagnostics/perf-jit.h"
#include "src/execution/isolate.h"
@ -248,7 +249,8 @@ void CodeEventLogger::CodeCreateEvent(CodeTag tag, Handle<AbstractCode> code,
DCHECK(is_listening_to_code_events());
name_buffer_->Init(tag);
name_buffer_->AppendBytes(comment);
LogRecordedBuffer(code, MaybeHandle<SharedFunctionInfo>(),
DisallowGarbageCollection no_gc;
LogRecordedBuffer(*code, MaybeHandle<SharedFunctionInfo>(),
name_buffer_->get(), name_buffer_->size());
}
@ -257,7 +259,8 @@ void CodeEventLogger::CodeCreateEvent(CodeTag tag, Handle<AbstractCode> code,
DCHECK(is_listening_to_code_events());
name_buffer_->Init(tag);
name_buffer_->AppendName(*name);
LogRecordedBuffer(code, MaybeHandle<SharedFunctionInfo>(),
DisallowGarbageCollection no_gc;
LogRecordedBuffer(*code, MaybeHandle<SharedFunctionInfo>(),
name_buffer_->get(), name_buffer_->size());
}
@ -269,7 +272,8 @@ void CodeEventLogger::CodeCreateEvent(CodeTag tag, Handle<AbstractCode> code,
name_buffer_->AppendBytes(ComputeMarker(*shared, *code));
name_buffer_->AppendByte(' ');
name_buffer_->AppendName(*script_name);
LogRecordedBuffer(code, shared, name_buffer_->get(), name_buffer_->size());
DisallowGarbageCollection no_gc;
LogRecordedBuffer(*code, shared, name_buffer_->get(), name_buffer_->size());
}
void CodeEventLogger::CodeCreateEvent(CodeTag tag, Handle<AbstractCode> code,
@ -292,7 +296,8 @@ void CodeEventLogger::CodeCreateEvent(CodeTag tag, Handle<AbstractCode> code,
name_buffer_->AppendInt(line);
name_buffer_->AppendByte(':');
name_buffer_->AppendInt(column);
LogRecordedBuffer(code, shared, name_buffer_->get(), name_buffer_->size());
DisallowGarbageCollection no_gc;
LogRecordedBuffer(*code, shared, name_buffer_->get(), name_buffer_->size());
}
#if V8_ENABLE_WEBASSEMBLY
@ -312,6 +317,7 @@ void CodeEventLogger::CodeCreateEvent(CodeTag tag, const wasm::WasmCode* code,
}
name_buffer_->AppendByte('-');
name_buffer_->AppendBytes(ExecutionTierToString(code->tier()));
DisallowGarbageCollection no_gc;
LogRecordedBuffer(code, name_buffer_->get(), name_buffer_->size());
}
#endif // V8_ENABLE_WEBASSEMBLY
@ -321,7 +327,8 @@ void CodeEventLogger::RegExpCodeCreateEvent(Handle<AbstractCode> code,
DCHECK(is_listening_to_code_events());
name_buffer_->Init(LogEventListener::CodeTag::kRegExp);
name_buffer_->AppendString(*source);
LogRecordedBuffer(code, MaybeHandle<SharedFunctionInfo>(),
DisallowGarbageCollection no_gc;
LogRecordedBuffer(*code, MaybeHandle<SharedFunctionInfo>(),
name_buffer_->get(), name_buffer_->size());
}
@ -338,7 +345,7 @@ class LinuxPerfBasicLogger : public CodeEventLogger {
Handle<SharedFunctionInfo> shared) override {}
private:
void LogRecordedBuffer(Handle<AbstractCode> code,
void LogRecordedBuffer(AbstractCode code,
MaybeHandle<SharedFunctionInfo> maybe_shared,
const char* name, int length) override;
#if V8_ENABLE_WEBASSEMBLY
@ -414,18 +421,19 @@ void LinuxPerfBasicLogger::WriteLogRecordedBuffer(uintptr_t address, int size,
size, name_length, name);
}
void LinuxPerfBasicLogger::LogRecordedBuffer(Handle<AbstractCode> code,
void LinuxPerfBasicLogger::LogRecordedBuffer(AbstractCode code,
MaybeHandle<SharedFunctionInfo>,
const char* name, int length) {
DisallowGarbageCollection no_gc;
PtrComprCageBase cage_base(isolate_);
if (v8_flags.perf_basic_prof_only_functions &&
CodeKindIsBuiltinOrJSFunction(code->kind(cage_base))) {
CodeKindIsBuiltinOrJSFunction(code.kind(cage_base))) {
return;
}
WriteLogRecordedBuffer(
static_cast<uintptr_t>(code->InstructionStart(cage_base)),
code->InstructionSize(cage_base), name, length);
static_cast<uintptr_t>(code.InstructionStart(cage_base)),
code.InstructionSize(cage_base), name, length);
}
#if V8_ENABLE_WEBASSEMBLY
@ -637,7 +645,7 @@ class LowLevelLogger : public CodeEventLogger {
void CodeMovingGCEvent() override;
private:
void LogRecordedBuffer(Handle<AbstractCode> code,
void LogRecordedBuffer(AbstractCode code,
MaybeHandle<SharedFunctionInfo> maybe_shared,
const char* name, int length) override;
#if V8_ENABLE_WEBASSEMBLY
@ -727,19 +735,19 @@ void LowLevelLogger::LogCodeInfo() {
LogWriteBytes(arch, sizeof(arch));
}
void LowLevelLogger::LogRecordedBuffer(Handle<AbstractCode> code,
void LowLevelLogger::LogRecordedBuffer(AbstractCode code,
MaybeHandle<SharedFunctionInfo>,
const char* name, int length) {
DisallowGarbageCollection no_gc;
PtrComprCageBase cage_base(isolate_);
CodeCreateStruct event;
event.name_size = length;
event.code_address = code->InstructionStart(cage_base);
event.code_size = code->InstructionSize(cage_base);
event.code_address = code.InstructionStart(cage_base);
event.code_size = code.InstructionSize(cage_base);
LogWriteStruct(event);
LogWriteBytes(name, length);
LogWriteBytes(
reinterpret_cast<const char*>(code->InstructionStart(cage_base)),
code->InstructionSize(cage_base));
LogWriteBytes(reinterpret_cast<const char*>(code.InstructionStart(cage_base)),
code.InstructionSize(cage_base));
}
#if V8_ENABLE_WEBASSEMBLY
@ -801,7 +809,7 @@ class JitLogger : public CodeEventLogger {
JitCodeEvent::CodeType code_type);
private:
void LogRecordedBuffer(Handle<AbstractCode> code,
void LogRecordedBuffer(AbstractCode code,
MaybeHandle<SharedFunctionInfo> maybe_shared,
const char* name, int length) override;
#if V8_ENABLE_WEBASSEMBLY
@ -818,16 +826,17 @@ JitLogger::JitLogger(Isolate* isolate, JitCodeEventHandler code_event_handler)
DCHECK_NOT_NULL(code_event_handler);
}
void JitLogger::LogRecordedBuffer(Handle<AbstractCode> code,
void JitLogger::LogRecordedBuffer(AbstractCode code,
MaybeHandle<SharedFunctionInfo> maybe_shared,
const char* name, int length) {
DisallowGarbageCollection no_gc;
PtrComprCageBase cage_base(isolate_);
JitCodeEvent event;
event.type = JitCodeEvent::CODE_ADDED;
event.code_start = reinterpret_cast<void*>(code->InstructionStart(cage_base));
event.code_type = code->IsCode(cage_base) ? JitCodeEvent::JIT_CODE
: JitCodeEvent::BYTE_CODE;
event.code_len = code->InstructionSize(cage_base);
event.code_start = reinterpret_cast<void*>(code.InstructionStart(cage_base));
event.code_type =
code.IsCode(cage_base) ? JitCodeEvent::JIT_CODE : JitCodeEvent::BYTE_CODE;
event.code_len = code.InstructionSize(cage_base);
Handle<SharedFunctionInfo> shared;
if (maybe_shared.ToHandle(&shared) &&
shared->script(cage_base).IsScript(cage_base)) {

View File

@ -448,7 +448,7 @@ class V8_EXPORT_PRIVATE CodeEventLogger : public LogEventListener {
private:
class NameBuffer;
virtual void LogRecordedBuffer(Handle<AbstractCode> code,
virtual void LogRecordedBuffer(AbstractCode code,
MaybeHandle<SharedFunctionInfo> maybe_shared,
const char* name, int length) = 0;
#if V8_ENABLE_WEBASSEMBLY

View File

@ -118,10 +118,10 @@ class CodeAddressMap : public CodeEventLogger {
base::HashMap impl_;
};
void LogRecordedBuffer(Handle<AbstractCode> code,
MaybeHandle<SharedFunctionInfo>, const char* name,
int length) override {
address_to_name_map_.Insert(code->address(), name, length);
void LogRecordedBuffer(AbstractCode code, MaybeHandle<SharedFunctionInfo>,
const char* name, int length) override {
DisallowGarbageCollection no_gc;
address_to_name_map_.Insert(code.address(), name, length);
}
#if V8_ENABLE_WEBASSEMBLY

View File

@ -453,7 +453,7 @@ TEST_F(LogTest, Issue539892) {
}
private:
void LogRecordedBuffer(i::Handle<i::AbstractCode> code,
void LogRecordedBuffer(i::AbstractCode code,
i::MaybeHandle<i::SharedFunctionInfo> maybe_shared,
const char* name, int length) override {}
#if V8_ENABLE_WEBASSEMBLY