Skip to content

Commit 133cc49

Browse files
committed
fix(compiler): only force OOG on failure paths, not success exits
OutOfGas is a generic-failure placeholder. STOP/RETURN/REVERT are not errors and must keep their original InstructionResult codes; only genuine failure paths (stack under/overflow, invalid jump, real OOG, invalid opcode, etc.) collapse to the OOG constant. This restores observable correctness: success paths now match the interpreter, and the strict equality assertion in sanity_check works again. Drops the relaxed comparison and the PreparedBench plumbing introduced in the previous commit.
1 parent f858fb7 commit 133cc49

3 files changed

Lines changed: 18 additions & 50 deletions

File tree

crates/revmc-cli/src/fixture.rs

Lines changed: 2 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,6 @@ pub struct PreparedBench {
106106
cfg: CfgEnv,
107107
tx: TxEnv,
108108
functions: B256Map<RawEvmCompilerFn>,
109-
/// Set when the JIT was compiled with [`EvmCompiler::force_out_of_gas`].
110-
/// Sanity checks tolerate the resulting OOG halt instead of demanding result equality.
111-
force_out_of_gas: bool,
112109
}
113110

114111
/// Caller address used for synthetic bytecode benchmarks.
@@ -168,15 +165,7 @@ impl PreparedBench {
168165
}
169166
compiler.clear_ir().expect("clear_ir failed");
170167

171-
Self {
172-
name: bench.name,
173-
accounts,
174-
block,
175-
cfg,
176-
tx,
177-
functions,
178-
force_out_of_gas: compiler.is_force_out_of_gas(),
179-
}
168+
Self { name: bench.name, accounts, block, cfg, tx, functions }
180169
}
181170

182171
/// Load a benchmark from a pre-compiled shared library instead of JIT-compiling.
@@ -207,10 +196,7 @@ impl PreparedBench {
207196
functions.insert(acct.code_hash, (*f).into_inner());
208197
}
209198

210-
(
211-
Self { name: bench.name, accounts, block, cfg, tx, functions, force_out_of_gas: false },
212-
lib,
213-
)
199+
(Self { name: bench.name, accounts, block, cfg, tx, functions }, lib)
214200
}
215201

216202
/// Convert a bytecode [`Bench`] into fixture state.
@@ -417,25 +403,9 @@ impl PreparedBench {
417403
}
418404

419405
/// Sanity-check that interpreter and JIT produce matching results.
420-
///
421-
/// When [`Self::force_out_of_gas`] is set, the JIT and interpreter routinely diverge
422-
/// because every JIT exit is rewritten to `OutOfGas`. Sub-calls that succeed under the
423-
/// interpreter can revert under the JIT, which then propagates to the caller. The check
424-
/// is relaxed to "JIT must not claim success when the interpreter failed" — any other
425-
/// divergence is expected.
426406
pub fn sanity_check(&self) {
427407
let interp = self.run_interpreter();
428408
let jit = self.run_jit();
429-
if self.force_out_of_gas {
430-
assert!(
431-
interp.result.is_success() || !jit.result.is_success(),
432-
"force_out_of_gas: JIT succeeded where the interpreter failed:\n \
433-
interpreter: {:?}\n JIT: {:?}",
434-
interp.result,
435-
jit.result,
436-
);
437-
return;
438-
}
439409
assert_eq!(
440410
interp.result.is_success(),
441411
jit.result.is_success(),

crates/revmc-codegen/src/compiler/mod.rs

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -414,26 +414,21 @@ impl<B: Backend> EvmCompiler<B> {
414414
self.config.gas_metering = yes;
415415
}
416416

417-
/// Sets whether all return paths in the JIT-compiled function should yield
418-
/// [`OutOfGas`](crate::interpreter::InstructionResult::OutOfGas) instead of their normal
419-
/// [`InstructionResult`](crate::interpreter::InstructionResult).
417+
/// Sets whether to collapse every JIT failure path to a single
418+
/// [`OutOfGas`](crate::interpreter::InstructionResult::OutOfGas) constant.
420419
///
421-
/// Useful for benchmarking the cost of return-value materialization and the phi node merging
422-
/// every return path through.
420+
/// Failures (stack under/overflow, invalid jump, real OOG, invalid opcode, etc.) are
421+
/// semantically interchangeable for callers that only branch on success vs failure, so
422+
/// this lets LLVM DCE the per-failure-site materialization and the failure-block phi.
423+
/// Successful exits (`STOP`/`RETURN`/`REVERT`) keep their original codes.
423424
///
424-
/// Note that this changes program behavior: the function will report `OutOfGas` for every
425-
/// exit, including normal `STOP`/`RETURN`/`REVERT`.
425+
/// Useful for benchmarking the cost of failure-result materialization.
426426
///
427427
/// Defaults to `false`.
428428
pub fn force_out_of_gas(&mut self, yes: bool) {
429429
self.config.force_out_of_gas = yes;
430430
}
431431

432-
/// Returns whether [`force_out_of_gas`](Self::force_out_of_gas) is enabled.
433-
pub fn is_force_out_of_gas(&self) -> bool {
434-
self.config.force_out_of_gas
435-
}
436-
437432
/// Sets custom gas parameters.
438433
///
439434
/// Overrides the default gas schedule derived from the spec_id.

crates/revmc-codegen/src/compiler/translate/mod.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,14 @@ impl<'a, B: Backend> FunctionCx<'a, B> {
439439
// Finalize the failure block.
440440
fx.bcx.switch_to_block(fx.failure_block.unwrap());
441441
if !fx.incoming_failures.is_empty() {
442-
let failure_value = fx.bcx.phi(fx.i8_type, &fx.incoming_failures);
442+
// `force_out_of_gas` collapses every error path to a generic OOG; this is
443+
// safe because failures are semantically interchangeable for callers that
444+
// only branch on success vs failure.
445+
let failure_value = if config.force_out_of_gas {
446+
fx.bcx.iconst(fx.i8_type, InstructionResult::OutOfGas as i64)
447+
} else {
448+
fx.bcx.phi(fx.i8_type, &fx.incoming_failures)
449+
};
443450
fx.bcx.set_current_block_cold();
444451
fx.build_return(failure_value);
445452
} else {
@@ -449,11 +456,7 @@ impl<'a, B: Backend> FunctionCx<'a, B> {
449456
// Finalize the return block.
450457
fx.bcx.switch_to_block(fx.return_block.unwrap());
451458
if !fx.incoming_returns.is_empty() {
452-
let return_value = if config.force_out_of_gas {
453-
fx.bcx.iconst(fx.i8_type, InstructionResult::OutOfGas as i64)
454-
} else {
455-
fx.bcx.phi(fx.i8_type, &fx.incoming_returns)
456-
};
459+
let return_value = fx.bcx.phi(fx.i8_type, &fx.incoming_returns);
457460
if config.inspect_stack {
458461
fx.copy_stack_to_arg();
459462
fx.save_stack_len();

0 commit comments

Comments
 (0)