Skip to content

Commit

Permalink
jit: correctly load the StableResult discriminant in emit_result_is_e…
Browse files Browse the repository at this point in the history
…rr (#514)

StableResult is #[repr(C, u64)] which means that the layout is

[u64 discriminant]
[union of all the data carried by the variants]

We were accessing the data of the Ok variant.
  • Loading branch information
alessandrod authored Sep 25, 2023
1 parent c10ce31 commit dd1ecc0
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 4 deletions.
4 changes: 2 additions & 2 deletions src/jit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1279,9 +1279,9 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {

fn emit_result_is_err(&mut self, destination: u8) {
let ok = ProgramResult::Ok(0);
let err_kind = unsafe { *(&ok as *const _ as *const u64).add(1) };
let ok_discriminant = ok.discriminant();
self.emit_ins(X86Instruction::lea(OperandSize::S64, RBP, destination, Some(X86IndirectAccess::Offset(self.slot_on_environment_stack(RuntimeEnvironmentSlot::ProgramResult)))));
self.emit_ins(X86Instruction::cmp_immediate(OperandSize::S64, destination, err_kind as i64, Some(X86IndirectAccess::Offset(0))));
self.emit_ins(X86Instruction::cmp_immediate(OperandSize::S64, destination, ok_discriminant as i64, Some(X86IndirectAccess::Offset(0))));
}

fn emit_subroutines(&mut self) {
Expand Down
16 changes: 14 additions & 2 deletions src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,18 @@ impl<T: Debug, E: Debug> StableResult<T, E> {
Self::Err(error) => error,
}
}

#[cfg_attr(
any(
not(feature = "jit"),
target_os = "windows",
not(target_arch = "x86_64")
),
allow(dead_code)
)]
pub(crate) fn discriminant(&self) -> u64 {
unsafe { *(self as *const _ as *const u64) }
}
}

impl<T, E> From<StableResult<T, E>> for Result<T, E> {
Expand Down Expand Up @@ -558,9 +570,9 @@ mod tests {
#[test]
fn test_program_result_is_stable() {
let ok = ProgramResult::Ok(42);
assert_eq!(unsafe { *(&ok as *const _ as *const u64) }, 0);
assert_eq!(ok.discriminant(), 0);
let err = ProgramResult::Err(Box::new(EbpfError::JitNotCompiled));
assert_eq!(unsafe { *(&err as *const _ as *const u64) }, 1);
assert_eq!(err.discriminant(), 1);
}

#[test]
Expand Down

0 comments on commit dd1ecc0

Please sign in to comment.