Skip to content

Commit 05277c6

Browse files
committed
ZJIT: Fix spurious CompileError:OutOfMemory
We were accidentally marking OutOfMemory when actually the problem was that the native stack was too big. Slightly re-think emit APIs to make this mistake harder to make. Now protoboeuf looks like this: Top-1 compile error reasons (100.0% of total 5,798,926): native_stack_too_large: 5,798,926 (100.0%)
1 parent 2707b49 commit 05277c6

2 files changed

Lines changed: 23 additions & 29 deletions

File tree

zjit/src/backend/arm64/mod.rs

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -902,7 +902,7 @@ impl Assembler {
902902

903903
/// Emit platform-specific machine code
904904
/// Returns a list of GC offsets. Can return failure to signal caller to retry.
905-
fn arm64_emit(&mut self, cb: &mut CodeBlock) -> Option<Vec<CodePtr>> {
905+
fn arm64_emit(&mut self, cb: &mut CodeBlock) -> Result<Vec<CodePtr>, CompileError> {
906906
/// Determine how many instructions it will take to represent moving
907907
/// this value into a register. Note that the return value of this
908908
/// function must correspond to how many instructions are used to
@@ -1165,7 +1165,9 @@ impl Assembler {
11651165
if slot_count > 0 {
11661166
let slot_offset = (slot_count * SIZEOF_VALUE) as u64;
11671167
// Bail when asked to reserve too many slots in one instruction.
1168-
ShiftedImmediate::try_from(slot_offset).ok()?;
1168+
if ShiftedImmediate::try_from(slot_offset).is_err() {
1169+
return Err(CompileError::NativeStackTooLarge);
1170+
}
11691171
sub(cb, C_SP_REG, C_SP_REG, A64Opnd::new_uimm(slot_offset));
11701172
}
11711173
}
@@ -1587,7 +1589,7 @@ impl Assembler {
15871589

15881590
// Error if we couldn't write out everything
15891591
if cb.has_dropped_bytes() {
1590-
None
1592+
Err(CompileError::OutOfMemory)
15911593
} else {
15921594
// No bytes dropped, so the pos markers point to valid code
15931595
for (insn_idx, pos) in pos_markers {
@@ -1598,7 +1600,7 @@ impl Assembler {
15981600
}
15991601
}
16001602

1601-
Some(gc_offsets)
1603+
Ok(gc_offsets)
16021604
}
16031605
}
16041606

@@ -1700,19 +1702,15 @@ impl Assembler {
17001702
}
17011703

17021704
let start_ptr = cb.get_write_ptr();
1703-
let gc_offsets = asm.arm64_emit(cb);
1705+
let gc_offsets = asm.arm64_emit(cb).inspect_err(|_| cb.clear_labels())?;
1706+
assert!(!cb.has_dropped_bytes(), "emit should not drop bytes without error");
17041707

1705-
if let (Some(gc_offsets), false) = (gc_offsets, cb.has_dropped_bytes()) {
1706-
cb.link_labels().or(Err(CompileError::LabelLinkingFailure))?;
1708+
cb.link_labels().or(Err(CompileError::LabelLinkingFailure))?;
17071709

1708-
// Invalidate icache for newly written out region so we don't run stale code.
1709-
unsafe { rb_jit_icache_invalidate(start_ptr.raw_ptr(cb) as _, cb.get_write_ptr().raw_ptr(cb) as _) };
1710+
// Invalidate icache for newly written out region so we don't run stale code.
1711+
unsafe { rb_jit_icache_invalidate(start_ptr.raw_ptr(cb) as _, cb.get_write_ptr().raw_ptr(cb) as _) };
17101712

1711-
Ok((start_ptr, gc_offsets))
1712-
} else {
1713-
cb.clear_labels();
1714-
Err(CompileError::OutOfMemory)
1715-
}
1713+
Ok((start_ptr, gc_offsets))
17161714
}
17171715
}
17181716

zjit/src/backend/x86_64/mod.rs

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -678,7 +678,7 @@ impl Assembler {
678678
}
679679

680680
/// Emit platform-specific machine code
681-
pub fn x86_emit(&mut self, cb: &mut CodeBlock) -> Option<Vec<CodePtr>> {
681+
pub fn x86_emit(&mut self, cb: &mut CodeBlock) -> Result<Vec<CodePtr>, CompileError> {
682682
fn emit_csel(
683683
cb: &mut CodeBlock,
684684
truthy: Opnd,
@@ -1122,7 +1122,7 @@ impl Assembler {
11221122

11231123
// Error if we couldn't write out everything
11241124
if cb.has_dropped_bytes() {
1125-
None
1125+
Err(CompileError::OutOfMemory)
11261126
} else {
11271127
// No bytes dropped, so the pos markers point to valid code
11281128
for (insn_idx, pos) in pos_markers {
@@ -1133,7 +1133,7 @@ impl Assembler {
11331133
}
11341134
}
11351135

1136-
Some(gc_offsets)
1136+
Ok(gc_offsets)
11371137
}
11381138
}
11391139

@@ -1235,15 +1235,11 @@ impl Assembler {
12351235
}
12361236

12371237
let start_ptr = cb.get_write_ptr();
1238-
let gc_offsets = asm.x86_emit(cb);
1238+
let gc_offsets = asm.x86_emit(cb).inspect_err(|_| cb.clear_labels())?;
1239+
assert!(!cb.has_dropped_bytes(), "emit should not drop bytes without error");
12391240

1240-
if let (Some(gc_offsets), false) = (gc_offsets, cb.has_dropped_bytes()) {
1241-
cb.link_labels().or(Err(CompileError::LabelLinkingFailure))?;
1242-
Ok((start_ptr, gc_offsets))
1243-
} else {
1244-
cb.clear_labels();
1245-
Err(CompileError::OutOfMemory)
1246-
}
1241+
cb.link_labels().or(Err(CompileError::LabelLinkingFailure))?;
1242+
Ok((start_ptr, gc_offsets))
12471243
}
12481244
}
12491245

@@ -1317,7 +1313,7 @@ mod tests {
13171313
for name in &asm.label_names {
13181314
cb.new_label(name.to_string());
13191315
}
1320-
assert!(asm.x86_emit(&mut cb).is_some(), "{kind:?}: x86_emit failed");
1316+
assert!(asm.x86_emit(&mut cb).is_ok(), "{kind:?}: x86_emit failed");
13211317

13221318
cb.disasm()
13231319
.lines()
@@ -2304,7 +2300,7 @@ mod tests {
23042300
for name in &asm.label_names {
23052301
cb.new_label(name.to_string());
23062302
}
2307-
assert!(asm.x86_emit(&mut cb).is_some());
2303+
assert!(asm.x86_emit(&mut cb).is_ok());
23082304

23092305
assert_disasm_snapshot!(cb.disasm(), @"
23102306
0x0: mov r10, qword ptr [rbp - 0x10]
@@ -2330,7 +2326,7 @@ mod tests {
23302326
for name in &asm.label_names {
23312327
cb.new_label(name.to_string());
23322328
}
2333-
assert!(asm.x86_emit(&mut cb).is_some());
2329+
assert!(asm.x86_emit(&mut cb).is_ok());
23342330

23352331
assert_disasm_snapshot!(cb.disasm(), @"
23362332
0x0: mov r10, qword ptr [r13 + 0x10]
@@ -2356,7 +2352,7 @@ mod tests {
23562352
for name in &asm.label_names {
23572353
cb.new_label(name.to_string());
23582354
}
2359-
assert!(asm.x86_emit(&mut cb).is_some());
2355+
assert!(asm.x86_emit(&mut cb).is_ok());
23602356

23612357
assert_disasm_snapshot!(cb.disasm(), @"
23622358
0x0: mov r11, qword ptr [rbp - 8]

0 commit comments

Comments
 (0)