AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[wasm] Align different write protection scopes

Browse Source 
Currently, we have two different classes for switching the WebAssembly
generated code space to writable (e.g., before patching jump tables, or
when adding or removing code): `CodeSpaceWriteScope` (with the macro
`CODE_SPACE_WRITE_SCOPE`) and `NativeModuleModificationScope`.
The former was introduced for Apple Silicon ARM64 hardware ("Apple M1"),
which uses `MAP_JIT` + `pthread_jit_write_protect_np()` to change memory
permissions. The latter uses either Intel PKU (aka. memory protection
keys) to switch permissions (fast and thread-local, like on M1), and
alternatively `mprotect()`, on systems that do not have PKU support.

Since both classes serve the same purpose just with different
implementations on different platforms, we want to merge them in
follow-up CLs. As a first step, here we align all uses of
`CODE_SPACE_WRITE_SCOPE` with existing `NativeModuleModificationScope`s.
The two had diverged due to optimization work, where we moved
`NativeModuleModificationScope`s around (pulling them out of loops and
across function boundaries) to lower the amount of mprotect switches.

This should have none, or at best a very small positive performance
impact on Apple M1, since we now also switch less often (even though
switching should be very cheap). In terms of security, this in theory
makes the code space writable for longer time spans, but this is
probably not a large effect because
(1) we often moved the scope outside of loops, where it was open for
every iteration anyway, or
(2) in some cases a CODE_SPACE_WRITE_SCOPE was open somewhere on the
call stack already.

R=jkummerow@chromium.org
CC=clemensb@chromium.org

Bug: v8:11714
Change-Id: Id8744429e1183e118ab5e078750d294a99c9dce0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2968946
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Daniel Lehmann <dlehmann@google.com>
Cr-Commit-Position: refs/heads/master@{#75230}
This commit is contained in:
Daniel Lehmann 2021-06-17 15:47:04 +00:00 committed by V8 LUCI CQ
parent f74e02becb
commit d0cfa6aab5
1 changed files with 7 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/wasm/wasm-code-manager.cc
View File

@ -795,7 +795,6 @@ void WasmCodeAllocator::FreeCode(Vector<WasmCode* const> codes) {
// Zap code area and collect freed code regions.
DisjointAllocationPool freed_regions;
size_t code_size = 0;
CODE_SPACE_WRITE_SCOPE
for (WasmCode* code : codes) {
ZapCode(code->instruction_start(), code->instructions().size());
FlushInstructionCache(code->instruction_start(),
@ -1035,11 +1034,11 @@ void NativeModule::UseLazyStub(uint32_t func_index) {
module_->num_imported_functions + module_->num_declared_functions);
base::RecursiveMutexGuard guard(&allocation_mutex_);
CODE_SPACE_WRITE_SCOPE
NativeModuleModificationScope native_module_modification_scope(this);
if (!lazy_compile_table_) {
uint32_t num_slots = module_->num_declared_functions;
WasmCodeRefScope code_ref_scope;
CODE_SPACE_WRITE_SCOPE
DCHECK_EQ(1, code_space_data_.size());
base::AddressRegion single_code_space_region = code_space_data_[0].region;
lazy_compile_table_ = CreateEmptyJumpTableInRegionLocked(
@ -1077,6 +1076,7 @@ std::unique_ptr<WasmCode> NativeModule::AddCode(
jump_table_ref =
FindJumpTablesForRegionLocked(base::AddressRegionOf(code_space));
}
CODE_SPACE_WRITE_SCOPE
NativeModuleModificationScope native_module_modification_scope(this);
return AddCodeWithCodeSpace(index, desc, stack_slots, tagged_parameter_slots,
protected_instructions_data,
@ -1105,7 +1105,6 @@ std::unique_ptr<WasmCode> NativeModule::AddCodeWithCodeSpace(
const int code_comments_offset = desc.code_comments_offset;
const int instr_size = desc.instr_size;
CODE_SPACE_WRITE_SCOPE
memcpy(dst_code_bytes.begin(), desc.buffer,
static_cast<size_t>(desc.instr_size));
@ -1157,6 +1156,7 @@ WasmCode* NativeModule::PublishCode(std::unique_ptr<WasmCode> code) {
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.wasm.detailed"),
"wasm.PublishCode");
base::RecursiveMutexGuard lock(&allocation_mutex_);
CODE_SPACE_WRITE_SCOPE
NativeModuleModificationScope native_module_modification_scope(this);
return PublishCodeLocked(std::move(code));
}
@ -1168,6 +1168,7 @@ std::vector<WasmCode*> NativeModule::PublishCode(
std::vector<WasmCode*> published_code;
published_code.reserve(codes.size());
base::RecursiveMutexGuard lock(&allocation_mutex_);
CODE_SPACE_WRITE_SCOPE
// Get writable permission already here (and not inside the loop in
// {PatchJumpTablesLocked}), to avoid switching for each {code} individually.
NativeModuleModificationScope native_module_modification_scope(this);
@ -1279,6 +1280,7 @@ void NativeModule::ReinstallDebugCode(WasmCode* code) {
code_table_[slot_idx] = code;
code->IncRef();
CODE_SPACE_WRITE_SCOPE
NativeModuleModificationScope native_module_modification_scope(this);
PatchJumpTablesLocked(slot_idx, code->instruction_start());
}
@ -1391,7 +1393,6 @@ void NativeModule::UpdateCodeSize(size_t size, ExecutionTier tier,
void NativeModule::PatchJumpTablesLocked(uint32_t slot_index, Address target) {
allocation_mutex_.AssertHeld();
CODE_SPACE_WRITE_SCOPE
for (auto& code_space_data : code_space_data_) {
DCHECK_IMPLIES(code_space_data.jump_table, code_space_data.far_jump_table);
if (!code_space_data.jump_table) continue;
@ -2179,6 +2180,7 @@ std::vector<int> NativeModule::FindFunctionsToRecompile(
TieringState new_tiering_state) {
WasmCodeRefScope code_ref_scope;
base::RecursiveMutexGuard guard(&allocation_mutex_);
CODE_SPACE_WRITE_SCOPE
// Get writable permission already here (and not inside the loop in
// {PatchJumpTablesLocked}), to avoid switching for each slot individually.
NativeModuleModificationScope native_module_modification_scope(this);
@ -2218,6 +2220,7 @@ std::vector<int> NativeModule::FindFunctionsToRecompile(
void NativeModule::FreeCode(Vector<WasmCode* const> codes) {
base::RecursiveMutexGuard guard(&allocation_mutex_);
CODE_SPACE_WRITE_SCOPE
// Get writable permission already here (and not inside the loop in
// {WasmCodeAllocator::FreeCode}), to avoid switching for each {code}
// individually.