Recv specialization: type-aware calldata loading, direct scalar return, view types#1462
Recv specialization: type-aware calldata loading, direct scalar return, view types#1462micahscopes wants to merge 4 commits into
Conversation
…t scalar return, view types Add three new RuntimeInputPlan variants alongside the existing DecodeHostPayload: - DirectCalldataLoad: for non-dynamic msg types where all fields are word-sized primitives, loads each field directly via calldataload at computed offsets, bypassing malloc/calldatacopy/MemoryBytes/SolDecoder entirely - LazyCalldataLoad: hybrid path where non-mut scalar fields use calldataload, ArrayView fields get zero-copy offset passing, BytesView/StringView fields get lazy dynamic view construction, and remaining fields go through decode - FieldLoadStrategy enum (Direct, Decode, SkipWithOffset, LazyDynamicView) Add DirectScalarReturn variant on RuntimeReturnPlan that emits mstore+return instead of calling encode_single_root_alloc, for u256/usize return types with DIRECT_ENCODE == true. Add compile-time const predicate queries that evaluate ABI trait associated constants via CTFE: query_head_size, query_is_dynamic, query_direct_encode, is_direct_return_eligible, direct_calldata_load_info, lazy_field_strategies. Add view types to core abi.fe: - ArrayView<T, N, I> with AbiSize impl, zero-copy element access, .over() - BytesView/StringView get from_parts constructors, .over() for late binding, and AbiSize impls for unbound () variants - skip_field helper and AbiDecoder::skip_bytes default method Update synthetic codegen to handle all new plan variants including the push_array_view_value and push_dynamic_view_from_calldata helpers. Update verify/package.rs with tree-walking dispatch verification that handles both SwitchScalar and Branch-based dispatch patterns.
- Panic on missing projected field in LazyCalldataLoad instead of silently dropping it (was producing wrong call args) - Add debug_assert verifying accumulated_offset invariant: no Direct/SkipWithOffset/LazyDynamicView after a field with unknown static size - Document ArrayView::get returns raw u256 word, caller must cast - Move `four` const allocation inside LazyCalldataLoad arm (only consumer) - Deduplicate scalar primitive matching: direct_calldata_load_info now delegates to is_primitive_word_scalar - Add depth limit (max 10) to static_abi_head_size to prevent stack overflow on pathological recursive types - Add doc comment to verify_contract_runtime_dispatch_block noting it handles future dispatch patterns (Branch/Goto/SwitchScalar tree walking) - Tests now assert DirectCalldataLoad specifically for raw(uint256) and swap(uint64,uint64) instead of accepting any input plan variant
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48afe06f4d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| cont_bb, | ||
| TyId::u256(self.db), | ||
| RuntimeClass::Scalar(word_scalar_class()), | ||
| RuntimeBuiltin::CallDataLoad { offset }, | ||
| ); |
There was a problem hiding this comment.
The DirectCalldataLoad path reads arguments with raw calldataload but never checks that calldata contains the required 32-byte heads for all fields. If a caller supplies only a selector or otherwise truncated calldata, EVM returns zero for out-of-range loads and the wrapper can call user code with forged zero values instead of reverting. The old decode path enforced length via AbiDecoder::read_word, so this changes observable behavior for malformed inputs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f908e98. Added emit_calldata_length_guard that reverts if calldatasize < 4 + sum(field_head_sizes). Both DirectCalldataLoad and LazyCalldataLoad paths now validate calldata length before any raw loads.
| expr: RExpr::Cast { | ||
| value: raw, | ||
| to: field_scalar, | ||
| }, |
There was a problem hiding this comment.
This cast-based decode bypasses ABI canonicality checks for bool and narrow ints (u8..u128, i8..i128, etc.) that were previously enforced by A::decode_*_word. With this path, non-canonical words (for example bool = 2 or oversized unsigned words) are silently accepted via truncation/nonzero-cast instead of reverting, which can change contract behavior on malformed calldata.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f908e98. Restricted is_primitive_word_scalar to u256/i256/usize only. Narrow types (bool, u8, i8, etc.) now fall through to the decode path which enforces canonicality validation. The three failing canonicality tests all pass again.
| let rel_offset = self.push_builtin_value( | ||
| bb, | ||
| TyId::u256(self.db), | ||
| RuntimeClass::Scalar(word_scalar_class()), | ||
| RuntimeBuiltin::CallDataLoad { offset: head_pos }, |
There was a problem hiding this comment.
push_dynamic_view_from_calldata trusts the head offset and tail length from calldata without validating that tail_abs, tail_abs + 32, and data_start + data_len stay within calldata. A malicious offset/length can therefore create BytesView/StringView pointing past the payload; subsequent reads on calldata-backed inputs return zeroed data rather than reverting, so invalid ABI can be treated as valid data.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f908e98. push_dynamic_view_from_calldata now has four sequential checks before constructing the view: (1) overflow on rel_offset+4, (2) overflow on tail_abs+32, (3) overflow on data_start+data_len, (4) data_end > calldatasize. All branch to revert on failure.
…lity restriction - Add calldata length guard before DirectCalldataLoad and LazyCalldataLoad (revert if calldatasize < expected minimum) - Restrict is_primitive_word_scalar to u256/i256/usize only, preventing narrow types (bool, u8, i8, etc.) from bypassing ABI canonicality checks - Add overflow checks on attacker-controlled offset arithmetic in push_dynamic_view_from_calldata (rel_offset+4, tail_abs+32, data_start+data_len) - Use checked_mul for array head size computation - Promote accumulated_offset invariant from debug_assert to assert - Fix test to expect DecodeHostPayload for u64 fields (no longer Direct-eligible) - Update codegen snapshots to reflect new guard blocks
1 participant
Summary
DirectCalldataLoad(all fields are fixed-size scalars),LazyCalldataLoad(mixed, some fields from calldataload, others decoded), and the existing decode path as fallbackDirectScalarReturnfor single-word return values: mstore + return instead of the full encode-alloc pipelineArrayView,BytesView,StringViewin the stdlib for zero-copy calldata-backed accessTest plan
raw(uint256)andswap(uint64,uint64)fixture tests assertDirectCalldataLoadselection