Skip to content

Commit

Permalink
Use new assembler to support global invalidation on A64
Browse files Browse the repository at this point in the history
Previously, we patched in an x64 JMP even on A64, which resulted in
invalid machine code. Use the new assembler to generate a jump instead.

Add an assert to make sure patches don't step on each other since it's
less clear cut on A64, where the size of the jump varies depending on
its placement relative to the target.

Fixes a lot of tests that use `set_trace_func` in `test_insns.rb`.

PR: Shopify#379
  • Loading branch information
XrXr authored and k0kubun committed Aug 29, 2022
1 parent 726a451 commit a375784
Showing 1 changed file with 10 additions and 7 deletions.
17 changes: 10 additions & 7 deletions yjit/src/invariants.rs
Expand Up @@ -528,8 +528,6 @@ pub extern "C" fn rb_yjit_tracing_invalidate_all() {
return;
}

use crate::asm::x86_64::jmp_ptr;

// Stop other ractors since we are going to patch machine code.
with_vm_lock(src_loc!(), || {
// Make it so all live block versions are no longer valid branch targets
Expand Down Expand Up @@ -561,13 +559,18 @@ pub extern "C" fn rb_yjit_tracing_invalidate_all() {

// Apply patches
let old_pos = cb.get_write_pos();
let patches = CodegenGlobals::take_global_inval_patches();
let mut patches = CodegenGlobals::take_global_inval_patches();
patches.sort_by_cached_key(|patch| patch.inline_patch_pos.raw_ptr());
let mut last_patch_end = std::ptr::null();
for patch in &patches {
cb.set_write_ptr(patch.inline_patch_pos);
jmp_ptr(cb, patch.outlined_target_pos);
assert!(last_patch_end <= patch.inline_patch_pos.raw_ptr(), "patches should not overlap");

// FIXME: Can't easily check we actually wrote out the JMP at the moment.
// assert!(!cb.has_dropped_bytes(), "patches should have space and jump offsets should fit in JMP rel32");
let mut asm = crate::backend::ir::Assembler::new();
asm.jmp(patch.outlined_target_pos.into());

cb.set_write_ptr(patch.inline_patch_pos);
asm.compile(cb);
last_patch_end = cb.get_write_ptr().raw_ptr();
}
cb.set_pos(old_pos);

Expand Down

0 comments on commit a375784

Please sign in to comment.