perf(evm): implement EVM lazy compile (segment-based compilation)#416
perf(evm): implement EVM lazy compile (segment-based compilation)#416ys8888john wants to merge 1 commit intoDTVMStack:mainfrom
Conversation
⚡ Performance Regression Check Results✅ Performance Check Passed (interpreter)Performance Benchmark Results (threshold: 25%)
Summary: 194 benchmarks, 0 regressions ✅ Performance Check Passed (multipass)Performance Benchmark Results (threshold: 25%)
Summary: 194 benchmarks, 0 regressions |
There was a problem hiding this comment.
Pull request overview
This PR introduces an EVM “lazy compile” mode (implemented as Multipass JIT + a lazy flag) and adds initial infrastructure for segment-based compilation (bytecode segmentation at JUMPDEST boundaries), along with basic unit tests.
Changes:
- Add
lazymode handling and a newDTVM_EVM_LAZY_COMPILEenv var /enable_lazy_compileEVMC option to toggle Multipass lazy compilation. - Implement
LazyEVMJITCompiler+EVMSegmentAnalyzerscaffolding and wire lazy compilation intoperformEVMJITCompile(). - Add a new
evmLazyCompileTeststest target with segment analyzer coverage.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vm/dt_evmc_vm.cpp | Adds lazy mode parsing and env/option toggles for lazy compilation. |
| src/action/compiler.cpp | Switches Multipass+lazy EVM compilation path to use LazyEVMJITCompiler. |
| src/runtime/evm_module.h | Adds storage/accessors for a per-module LazyEVMJITCompiler. |
| src/runtime/evm_module.cpp | Implements EVMModule::newLazyEVMJITCompiler(). |
| src/compiler/evm_lazy_compiler.h | Declares EVMSegmentAnalyzer and LazyEVMJITCompiler APIs. |
| src/compiler/evm_lazy_compiler.cpp | Implements segment analysis and the initial “lazy” compiler behavior. |
| src/compiler/CMakeLists.txt | Adds evm_lazy_compiler.cpp to the compiler library sources. |
| src/tests/evm_lazy_compile_tests.cpp | Adds unit tests for the segment analyzer. |
| src/tests/CMakeLists.txt | Builds/runs the new evmLazyCompileTests when Multipass JIT is enabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Bytecode: JUMPDEST at PC=3 | ||
| std::vector<uint8_t> Bytecode = { | ||
| 0x60, 0x01, // PUSH1 0x01 (PC 0-1) | ||
| 0x60, 0x02, // PUSH1 0x02 (PC 2-3) | ||
| 0x5B, // JUMPDEST (PC 4) | ||
| 0x00 // STOP (PC 5) |
| if (SegmentIdx < SegmentAnalyzer.getNumSegments() && | ||
| CompileStatuses[SegmentIdx] == CompileStatus::Done) { | ||
| return SegmentCodePtrs[SegmentIdx]; | ||
| } | ||
|
|
||
| // Fallback: compile the full bytecode if not yet done | ||
| std::lock_guard<std::mutex> Lock(ForegroundMutex); | ||
|
|
||
| auto Timer = | ||
| Stats.startRecord(zen::utils::StatisticPhase::JITLazyFgCompilation); | ||
|
|
||
| // Re-check after acquiring lock | ||
| if (CompileStatuses[SegmentIdx] == CompileStatus::Done) { | ||
| Stats.stopRecord(Timer); | ||
| return SegmentCodePtrs[SegmentIdx]; |
| /// Instead of compiling the entire EVM bytecode at once, this compiler: | ||
| /// 1. Analyzes the bytecode to identify segments (JUMPDEST boundaries) | ||
| /// 2. Compiles only the entry segment initially | ||
| /// 3. Creates stubs for other segments that trigger on-demand compilation | ||
| /// 4. Optionally dispatches background compilation tasks for remaining segments | ||
| /// | ||
| /// This is analogous to the WASM LazyJITCompiler but adapted for EVM's | ||
| /// single-function, segment-based structure. | ||
| class LazyEVMJITCompiler final : public EVMJITCompiler { | ||
| public: | ||
| LazyEVMJITCompiler(runtime::EVMModule *EVMMod); | ||
| ~LazyEVMJITCompiler() override; | ||
|
|
||
| /// Perform initial precompilation: | ||
| /// - Analyze bytecode into segments | ||
| /// - Compile the entry segment eagerly | ||
| /// - Create stubs for remaining segments | ||
| /// - Optionally dispatch background compilation tasks |
There was a problem hiding this comment.
Pull request overview
This PR introduces an initial implementation of an EVM “lazy compile” mode intended to support segment-based (JUMPDEST-delimited) compilation in the multipass JIT pipeline.
Changes:
- Add
lazymode /EnableMultipassLazytoggles for the EVMC VM (options + env var). - Introduce
EVMSegmentAnalyzerandLazyEVMJITCompiler, and wire it into EVM JIT compilation selection. - Add a new
evmLazyCompileTeststarget with unit tests for the segment analyzer.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vm/dt_evmc_vm.cpp | Adds EVMC options/env var to enable lazy compilation mode. |
| src/action/compiler.cpp | Switches EVM multipass compilation to use LazyEVMJITCompiler when enabled. |
| src/runtime/evm_module.h | Stores/exports a lazy EVM JIT compiler instance on EVMModule. |
| src/runtime/evm_module.cpp | Implements EVMModule::newLazyEVMJITCompiler(). |
| src/compiler/evm_lazy_compiler.h | Declares segment analyzer + lazy EVM JIT compiler APIs. |
| src/compiler/evm_lazy_compiler.cpp | Implements segment analysis and the lazy compiler (currently eager-full compile). |
| src/compiler/CMakeLists.txt | Adds evm_lazy_compiler.cpp to compiler sources when EVM is enabled. |
| src/tests/evm_lazy_compile_tests.cpp | Adds segment analyzer unit tests. |
| src/tests/CMakeLists.txt | Builds/runs evmLazyCompileTests when ZEN_ENABLE_MULTIPASS_JIT is enabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #ifdef ZEN_ENABLE_JIT | ||
| common::CodeMemPool JITCodeMemPool; | ||
| void *JITCode = nullptr; | ||
| size_t JITCodeSize = 0; | ||
| std::unique_ptr<COMPILER::LazyEVMJITCompiler> LazyJITCompiler; | ||
| #endif // ZEN_ENABLE_JIT |
There was a problem hiding this comment.
LazyJITCompiler is declared as std::unique_ptr<COMPILER::LazyEVMJITCompiler> under ZEN_ENABLE_JIT, but ZEN_ENABLE_JIT is defined for both singlepass and multipass builds (see src/CMakeLists.txt). In a singlepass-only build, LazyEVMJITCompiler is only forward-declared here, so EVMModule’s destructor will need a complete type and this will not compile. Guard the lazy-compiler member (and related APIs) with ZEN_ENABLE_MULTIPASS_JIT (or provide a custom deleter / include the full definition whenever ZEN_ENABLE_JIT is set).
| /// Lazy JIT compiler for EVM bytecode. | ||
| /// | ||
| /// Instead of compiling the entire EVM bytecode at once, this compiler: | ||
| /// 1. Analyzes the bytecode to identify segments (JUMPDEST boundaries) | ||
| /// 2. Compiles only the entry segment initially | ||
| /// 3. Creates stubs for other segments that trigger on-demand compilation | ||
| /// 4. Optionally dispatches background compilation tasks for remaining segments | ||
| /// | ||
| /// This is analogous to the WASM LazyJITCompiler but adapted for EVM's | ||
| /// single-function, segment-based structure. | ||
| class LazyEVMJITCompiler final : public EVMJITCompiler { |
There was a problem hiding this comment.
The header comment describes stub-based, segment-level lazy compilation (entry-only compile + on-demand compilation for other segments), but precompile() currently compiles the full bytecode eagerly and marks all segments Done. Either update the documentation to match the current behavior or implement the described stub/on-demand compilation so callers aren’t misled about what “lazy compile” does.
| EVMSegmentAnalyzer Analyzer; | ||
| Analyzer.analyze(reinterpret_cast<const std::byte *>(Bytecode.data()), | ||
| Bytecode.size(), EVMC_OSAKA); | ||
|
|
There was a problem hiding this comment.
These tests pass reinterpret_cast<const std::byte*> into APIs expecting zen::common::Byte*. This will break when ZEN_DISABLE_CXX17_STL is enabled (where zen::common::Byte is libcxx::byte, not std::byte). Prefer using reinterpret_cast<const zen::common::Byte*> (or a helper) so the tests compile in both configurations.
| EVMMod->setJITCodeAndSize(CodePtr, MainContext->CodeSize); | ||
|
|
||
| size_t CodeSize = CodeMPool.getMemEnd() - JITCode; | ||
| zen::platform::mprotect(JITCode, TO_MPROTECT_CODE_SIZE_LAZY(CodeSize), | ||
| PROT_READ | PROT_EXEC); |
There was a problem hiding this comment.
In the fallback path of compileSegmentOnRequest(), after mprotect() you never call EVMMod->setJITCodeAndSize(JITCode, CodeSize) (unlike precompile() and EagerEVMJITCompiler). This leaves JITCode/JITCodeSize potentially inconsistent (e.g., trap handler code-range checks) depending on what CodePtr/MainContext->CodeSize are. Consider mirroring the precompile()/eager sequence: set entry pointer/size if needed, then set module JIT code to the pool start + protected size.
| EVMMod->setJITCodeAndSize(CodePtr, MainContext->CodeSize); | |
| size_t CodeSize = CodeMPool.getMemEnd() - JITCode; | |
| zen::platform::mprotect(JITCode, TO_MPROTECT_CODE_SIZE_LAZY(CodeSize), | |
| PROT_READ | PROT_EXEC); | |
| size_t CodeSize = CodeMPool.getMemEnd() - JITCode; | |
| zen::platform::mprotect(JITCode, TO_MPROTECT_CODE_SIZE_LAZY(CodeSize), | |
| PROT_READ | PROT_EXEC); | |
| EVMMod->setJITCodeAndSize(JITCode, CodeSize); |
| COMPILER::LazyEVMJITCompiler *newLazyEVMJITCompiler(); | ||
|
|
||
| COMPILER::LazyEVMJITCompiler *getLazyEVMJITCompiler() const { | ||
| return LazyJITCompiler.get(); | ||
| } | ||
|
|
||
| bool hasLazyJITCompiler() const { return LazyJITCompiler != nullptr; } |
There was a problem hiding this comment.
newLazyEVMJITCompiler() is declared under ZEN_ENABLE_JIT but defined in evm_module.cpp only under ZEN_ENABLE_MULTIPASS_JIT. To avoid mismatched feature-guards (and the incomplete-type issue from the lazy compiler member), put this API under the same ZEN_ENABLE_MULTIPASS_JIT guard as its implementation (or provide a definition for all ZEN_ENABLE_JIT builds).
| } else if (std::strcmp(Value, "multipass") == 0) { | ||
| VM->Config.Mode = RunMode::MultipassMode; | ||
| return EVMC_SET_OPTION_SUCCESS; | ||
| } else if (std::strcmp(Value, "lazy") == 0) { | ||
| VM->Config.Mode = RunMode::MultipassMode; | ||
| VM->Config.EnableMultipassLazy = true; | ||
| return EVMC_SET_OPTION_SUCCESS; |
There was a problem hiding this comment.
Selecting mode=multipass does not clear EnableMultipassLazy. If the VM was previously configured with mode=lazy (or enable_lazy_compile=true), switching back to multipass will still run in lazy mode. Consider explicitly setting Config.EnableMultipassLazy = false in the multipass (and interpreter) branches so the mode option is self-contained and deterministic.
| uint8_t *LazyEVMJITCompiler::compileSegmentOnRequest(uint32_t SegmentIdx) { | ||
| // In the current implementation, all segments are compiled during precompile. | ||
| // This method is a placeholder for future segment-level lazy compilation. | ||
| if (SegmentIdx < SegmentAnalyzer.getNumSegments() && | ||
| CompileStatuses[SegmentIdx] == CompileStatus::Done) { | ||
| return SegmentCodePtrs[SegmentIdx]; | ||
| } | ||
|
|
||
| // Fallback: compile the full bytecode if not yet done | ||
| std::lock_guard<std::mutex> Lock(ForegroundMutex); | ||
|
|
||
| auto Timer = | ||
| Stats.startRecord(zen::utils::StatisticPhase::JITLazyFgCompilation); | ||
|
|
||
| // Re-check after acquiring lock | ||
| if (CompileStatuses[SegmentIdx] == CompileStatus::Done) { | ||
| Stats.stopRecord(Timer); | ||
| return SegmentCodePtrs[SegmentIdx]; | ||
| } |
There was a problem hiding this comment.
compileSegmentOnRequest() can read out of bounds / dereference null: after the initial bounds check, the code unconditionally evaluates CompileStatuses[SegmentIdx] again (and later writes it) even when SegmentIdx >= getNumSegments() or when precompile() was never called (leaving CompileStatuses null). Add an early guard that validates SegmentIdx < getNumSegments() and that per-segment state has been initialized (or make this function unreachable before precompile()).
| // Get instruction metrics for opcode length calculation | ||
| const auto *Metrics = evmc_get_instruction_metrics_table(Rev); | ||
| if (!Metrics) { | ||
| Metrics = evmc_get_instruction_metrics_table(zen::evm::DEFAULT_REVISION); | ||
| } | ||
|
|
There was a problem hiding this comment.
Metrics is computed but never used. If opcode-length calculation only needs PUSH handling, remove the unused metrics lookup; otherwise, use Metrics to advance PC safely/consistently for non-PUSH opcodes.
| // Get instruction metrics for opcode length calculation | |
| const auto *Metrics = evmc_get_instruction_metrics_table(Rev); | |
| if (!Metrics) { | |
| Metrics = evmc_get_instruction_metrics_table(zen::evm::DEFAULT_REVISION); | |
| } |
| // Bytecode: JUMPDEST at PC=3 | ||
| std::vector<uint8_t> Bytecode = { | ||
| 0x60, 0x01, // PUSH1 0x01 (PC 0-1) | ||
| 0x60, 0x02, // PUSH1 0x02 (PC 2-3) | ||
| 0x5B, // JUMPDEST (PC 4) | ||
| 0x00 // STOP (PC 5) | ||
| }; |
There was a problem hiding this comment.
The comment says “JUMPDEST at PC=3” but with two PUSH1 instructions (2 bytes each) the JUMPDEST is actually at PC=4. Please fix the comment to match the bytecode layout to avoid confusion when maintaining these tests.
ys8888john
force-pushed
the
evm-lazy-compile
branch
3 times, most recently
from
ece08ab to
1512729
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| EVMMod->setJITCodeAndSize(EntryCodePtr, 0); | ||
|
|
There was a problem hiding this comment.
precompile() sets EVMModule JIT code size to 0. The runtime/trap handlers use getJITCodeSize() to compute the JIT address range (e.g. for backtraces), so a zero size breaks that logic even though the entry stub is callable. Consider setting JITCode to the code pool start (or entry stub, if it is the pool start) and JITCodeSize to CodeMemPool.getMemEnd() - getMemStart() after stub/resolver allocation, and updating it again after the first compilation when the pool grows.
| EVMMod->setJITCodeAndSize(EntryCodePtr, 0); | |
| // Initialize JIT code size based on the code memory pool so that | |
| // runtime/trap handlers see a valid JIT address range (for backtraces, etc.) | |
| size_t JITSize = 0; | |
| if (MainContext && MainContext->CodeMPool) { | |
| void *MemStart = MainContext->CodeMPool->getMemStart(); | |
| void *MemEnd = MainContext->CodeMPool->getMemEnd(); | |
| if (MemStart && MemEnd && MemEnd > MemStart) { | |
| auto *Start = static_cast<uint8_t *>(MemStart); | |
| auto *End = static_cast<uint8_t *>(MemEnd); | |
| JITSize = static_cast<size_t>(End - Start); | |
| } | |
| } | |
| EVMMod->setJITCodeAndSize(EntryCodePtr, JITSize); |
| uint8_t *LazyEVMJITCompiler::compileSegmentOnRequest(uint32_t SegmentIdx) { | ||
| uint32_t NumSegments = SegmentAnalyzer.getNumSegments(); | ||
|
|
||
| // Fast path: already compiled | ||
| if (SegmentIdx < NumSegments && | ||
| CompileStatuses[SegmentIdx] == CompileStatus::Done) { | ||
| return SegmentCodePtrs[SegmentIdx]; | ||
| } | ||
|
|
||
| // Slow path: compile on demand with thread safety | ||
| std::lock_guard<std::mutex> Lock(ForegroundMutex); | ||
|
|
||
| auto Timer = | ||
| Stats.startRecord(zen::utils::StatisticPhase::JITLazyFgCompilation); | ||
|
|
||
| // Double-check after acquiring lock | ||
| if (CompileStatuses[SegmentIdx] == CompileStatus::Done) { | ||
| Stats.stopRecord(Timer); | ||
| return SegmentCodePtrs[SegmentIdx]; |
There was a problem hiding this comment.
compileSegmentOnRequest() uses CompileStatuses[SegmentIdx] without validating SegmentIdx < NumSegments after taking the lock. If an invalid segment index is ever passed (e.g. corrupted stub address), this is out-of-bounds UB. Add a bounds check early in the slow path and return/abort with an error if SegmentIdx is invalid.
| EnableLazy != nullptr) { | ||
| bool ParsedEnableLazy = false; | ||
| if (parseBoolEnvValue(EnableLazy, ParsedEnableLazy)) { | ||
| Config.EnableMultipassLazy = ParsedEnableLazy; |
There was a problem hiding this comment.
DTVM_EVM_LAZY_COMPILE can enable EnableMultipassLazy without switching Config.Mode to MultipassMode, even though the option comment says lazy compile requires multipass. To avoid confusing/partial configurations, consider forcing Config.Mode = RunMode::MultipassMode when the env var parses to true (mirroring set_option(enable_lazy_compile)).
| Config.EnableMultipassLazy = ParsedEnableLazy; | |
| Config.EnableMultipassLazy = ParsedEnableLazy; | |
| if (ParsedEnableLazy) { | |
| Config.Mode = RunMode::MultipassMode; | |
| } |
| static uint64_t | ||
| compileSegmentOnRequestTrampoline([[maybe_unused]] zen::runtime::Instance *Inst, | ||
| uint8_t *NextStubCodePtr) { | ||
| // Validate instance pointer | ||
| if (!Inst) { | ||
| ZEN_LOG_ERROR("EVM lazy compile: Instance pointer is null in trampoline"); | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
In the trampoline error paths you return 0, but the stub resolver uses the return value as the re-entry address (it overwrites the saved RIP). Returning 0 will jump to address 0 and crash. If these conditions are truly impossible, prefer ZEN_ASSERT/ZEN_ABORT; otherwise return a valid failure stub/throw an exception so control flow can’t continue with a null target.
| uint32_t SegmentStartPC = 0; | ||
| uint32_t SegmentEndPC = static_cast<uint32_t>(BytecodeSize); | ||
| const EVMFrontendContext *EvmCtx = | ||
| dynamic_cast<const EVMFrontendContext *>(Ctx); | ||
| if (EvmCtx) { | ||
| SegmentStartPC = EvmCtx->getSegmentStartPC(); | ||
| SegmentEndPC = EvmCtx->getSegmentEndPC(); | ||
| if (SegmentEndPC == UINT32_MAX || | ||
| SegmentEndPC > static_cast<uint32_t>(BytecodeSize)) { | ||
| SegmentEndPC = static_cast<uint32_t>(BytecodeSize); | ||
| } |
There was a problem hiding this comment.
dynamic_cast requires RTTI, but the project is built with -fno-rtti (see top-level CMakeLists). This will fail to compile. Since Ctx in the EVM JIT pipeline is an EVMFrontendContext, use a static_cast (or add an explicit kind/tag on CompileContext) instead of dynamic_cast.
| uint32_t SegmentStartPC = 0; | |
| uint32_t SegmentEndPC = static_cast<uint32_t>(BytecodeSize); | |
| const EVMFrontendContext *EvmCtx = | |
| dynamic_cast<const EVMFrontendContext *>(Ctx); | |
| if (EvmCtx) { | |
| SegmentStartPC = EvmCtx->getSegmentStartPC(); | |
| SegmentEndPC = EvmCtx->getSegmentEndPC(); | |
| if (SegmentEndPC == UINT32_MAX || | |
| SegmentEndPC > static_cast<uint32_t>(BytecodeSize)) { | |
| SegmentEndPC = static_cast<uint32_t>(BytecodeSize); | |
| } | |
| const EVMFrontendContext *EvmCtx = | |
| static_cast<const EVMFrontendContext *>(Ctx); | |
| uint32_t SegmentStartPC = EvmCtx->getSegmentStartPC(); | |
| uint32_t SegmentEndPC = EvmCtx->getSegmentEndPC(); | |
| if (SegmentEndPC == UINT32_MAX || | |
| SegmentEndPC > static_cast<uint32_t>(BytecodeSize)) { | |
| SegmentEndPC = static_cast<uint32_t>(BytecodeSize); |
Co-authored-by: Aone Copilot <noreply@alibaba-inc.com>
ys8888john
force-pushed
the
evm-lazy-compile
branch
from
53656f3 to
d4cc938
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| dynamic_cast<const EVMFrontendContext *>(&Ctx); | ||
| if (IsImplicit && EvmCtx && EvmCtx->Lazy) { |
There was a problem hiding this comment.
dynamic_cast requires RTTI, but the project is built with -fno-rtti (see top-level CMakeLists). This will not compile. Since EVMMirBuilder is already tied to EVMFrontendContext, replace this with a static_cast<const EVMFrontendContext*>(&Ctx) (or equivalent) and remove RTTI usage.
| dynamic_cast<const EVMFrontendContext *>(&Ctx); | |
| if (IsImplicit && EvmCtx && EvmCtx->Lazy) { | |
| static_cast<const EVMFrontendContext *>(&Ctx); | |
| if (IsImplicit && EvmCtx->Lazy) { |
| const EVMFrontendContext *EvmCtx = | ||
| dynamic_cast<const EVMFrontendContext *>(Ctx); | ||
| if (EvmCtx) { |
There was a problem hiding this comment.
dynamic_cast requires RTTI, but the build uses -fno-rtti, so this will fail to compile. If this visitor is only used with EVMFrontendContext, use a static_cast (and optionally an assert) instead of RTTI-based casting.
| SmallVector<CgOperand, 2> JumpOperands; | ||
| JumpOperands.push_back(CgOperand::createRegOperand(TargetCodeReg)); | ||
| JumpOperands.push_back( | ||
| CgOperand::createRegOperand(X86::RDI, /*isDef=*/false)); |
There was a problem hiding this comment.
The comment says RDI is added as an implicit use, but it is created as an explicit operand (IsImplicit=false by default). X86MCInstLower::lowerMachineOperand will then emit it into the MCInst, and after you lower TAILJMPr64 to JMP64r this extra operand can break encoding / operand count expectations. Mark the RDI operand as implicit (or use the RegState::Implicit flag) so it participates in liveness but is not encoded.
| CgOperand::createRegOperand(X86::RDI, /*isDef=*/false)); | |
| CgOperand::createRegOperand(X86::RDI, /*isDef=*/false, | |
| /*isImplicit=*/true)); |
| // -5 because the jmp instructions has 5 bytes | ||
| int64_t CallRelOffset = TargetPtr - CurStubCodePtr - 5; | ||
| ZEN_ASSERT(CallRelOffset <= UINT32_MAX); | ||
| int32_t CallRelOffsetI32 = static_cast<int32_t>(CallRelOffset); | ||
|
|
There was a problem hiding this comment.
Relative JMP rel32 offsets can be negative and must fit in a signed 32-bit range. The current check CallRelOffset <= UINT32_MAX is ineffective for negative offsets (it will always pass) and does not prevent values < INT32_MIN from overflowing when cast to int32_t. Validate INT32_MIN <= CallRelOffset && CallRelOffset <= INT32_MAX before patching.
| // not from the call instruction itself. Since call is 5 bytes (E8 + 4-byte | ||
| // offset), we subtract 5 to get the correct relative offset. | ||
| int64_t CallRelOffset = StubResolverPtr - NewStubTmplPatchPointPtr - 5; | ||
| ZEN_ASSERT(CallRelOffset <= UINT32_MAX); | ||
|
|
||
| // StubResolver not too far, use call(0xe8) offset | ||
| int32_t CallRelOffsetI32 = static_cast<int32_t>(CallRelOffset); | ||
| std::memcpy(NewStubTmplPatchPointPtr + 1, &CallRelOffsetI32, 4); |
There was a problem hiding this comment.
Same issue as above: the RIP-relative call offset is signed 32-bit. CallRelOffset <= UINT32_MAX does not correctly validate range and allows negative offsets that overflow int32_t. Validate against INT32_MIN..INT32_MAX (and consider handling the out-of-range case rather than asserting in release builds).
| // Allocate stub space and compile resolver | ||
| StubBuilder->allocateStubSpace(NumSegments); | ||
| StubBuilder->compileStubResolver(this); | ||
|
|
||
| // Create stubs for all segments | ||
| for (const auto &Seg : AnalyzerSegments) { | ||
| StubBuilder->compileSegmentToStub(Seg.SegmentIdx, Seg.StartPC); | ||
| uint8_t *StubPtr = StubBuilder->getSegmentStubCodePtr(Seg.SegmentIdx); |
There was a problem hiding this comment.
allocateStubSpace() can fail and returns early leaving StubsCodePtr null, but precompile() continues and unconditionally calls compileSegmentToStub(), which will pointer-arithmetic on a null base and likely crash. Propagate allocation failure (e.g., make allocateStubSpace() return bool) and abort lazy setup if stub allocation fails.
| // In lazy mode, gas chunks must not cross segment boundaries. | ||
| // UINT32_MAX means no segment boundary restriction (eager mode). | ||
| uint32_t LazySegmentEndPC = UINT32_MAX; |
There was a problem hiding this comment.
LazySegmentEndPC is introduced in EVMFrontendContext but is not referenced anywhere (gas-chunk boundary checking uses EVMMirBuilder::LazySegmentEndPC). This looks like dead state and increases confusion about which field is authoritative. Either wire this field into the logic (with accessors and correct initialization/copy) or remove it.
| // In lazy mode, gas chunks must not cross segment boundaries. | |
| // UINT32_MAX means no segment boundary restriction (eager mode). | |
| uint32_t LazySegmentEndPC = UINT32_MAX; |
| // Check if we're in lazy compilation mode (segment doesn't start at PC=0) | ||
| // In lazy mode, we skip stack check for the entry block to avoid creating | ||
| // control flow edges that could make entry block a loop header | ||
| const EVMFrontendContext *EvmCtx = | ||
| static_cast<const EVMFrontendContext *>(&Ctx); | ||
| if (EvmCtx && EvmCtx->getSegmentStartPC() != 0) { | ||
| // In lazy compilation mode, skip stack check | ||
| return; | ||
| } |
There was a problem hiding this comment.
The comment says “skip stack check for the entry block”, but the condition actually skips stack checks when segmentStartPC != 0 (i.e., for non-entry segments). If the intent is to skip checks for non-entry segments, please fix the comment; if the intent is different, adjust the condition accordingly.
1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):
2. What is the scope of this PR (e.g. component or file name):
3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):
4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):
5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:
6. Release note