Skip to content

Commit 26effb7

Browse files
cburgdorfcburgdorf
authored
fix: ICE when emitting #[event] structs with many fields (argotorg#1516)
Events with 8 or more total fields crashed MIR lowering with "semantic const should reify for runtime lowering". Two bugs combined: 1. The #[event] desugar hashes the signature via core::keccak over a tuple of 2*fields+2 string fragments, but the AsBytes tuple impls stop at arity 16, so the TOPIC0 const never evaluated. 2. The missing impl should have been a type error, but the final obligation pass discharged unsatisfied goals containing string literal types as "not concrete": string literals carry a string-sorted inference variable until the String<N> fallback is applied, so the diagnostic was silently suppressed and the unevaluated const reached MIR. Fixes: - hir: nest the TOPIC0 keccak tuple into chunks of at most 16 elements when the signature is longer. Concatenation is associative, so the hash is byte-identical and signatures of any length compile; tuples with <= 16 elements stay flat, leaving existing events unchanged. - core: raise the AbiSize/Encode/EventAbiEncode tuple impls from arity 12 to 16, bounding events at 16 data (non-indexed) fields. Nesting is not an option for the payload since ABI offsets are frame-relative. Exceeding the bound now reports a proper trait-bound diagnostic spanning the event definition. - ty_check: fold obligation goals through Prober (literal fallbacks) at the final pass so unsatisfied bounds that mention string literals are reported instead of silently discharged. - mir: the reify panic now names the const and expected type as a defensive backstop. Tests: fe_test fixture covering 6/7/8/16 data fields (with and without an indexed field) emitted via log.emit, with every TOPIC0 asserted against externally computed keccak256 values; desugar snapshot for the nested tuple; uitests for the two new diagnostics.
1 parent 2779a76 commit 26effb7

11 files changed

Lines changed: 1157 additions & 59 deletions

File tree

‎crates/fe/tests/fixtures/fe_test/event_many_fields.fe‎

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
// Regression test for events with many fields.
2+
//
3+
// The `#[event]` desugar hashes the signature with `core::keccak` over a
4+
// tuple of string fragments. That tuple has `2 * total_fields + 2` elements,
5+
// and the `AsBytes` tuple impls stop at arity 16, so events with 8 or more
6+
// total fields used to leave `TOPIC0` unevaluated and crash MIR lowering.
7+
// The desugar now nests the tuple into chunks of at most 16 elements, which
8+
// hashes the same bytes at any field count.
9+
//
10+
// The expected TOPIC0 values below are `keccak256` of the canonical event
11+
// signature (e.g. `E8Data(address,uint256,...)`), computed externally with
12+
// `cast keccak`.
13+
14+
use std::evm::{Evm, Log}
15+
16+
#[event]
17+
struct E6Data {
18+
#[indexed]
19+
who: Address,
20+
f0: u256,
21+
f1: u256,
22+
f2: u256,
23+
f3: u256,
24+
f4: u256,
25+
f5: u256,
26+
}
27+
28+
#[event]
29+
struct E7Data {
30+
#[indexed]
31+
who: Address,
32+
f0: u256,
33+
f1: u256,
34+
f2: u256,
35+
f3: u256,
36+
f4: u256,
37+
f5: u256,
38+
f6: u256,
39+
}
40+
41+
#[event]
42+
struct E8Data {
43+
#[indexed]
44+
who: Address,
45+
f0: u256,
46+
f1: u256,
47+
f2: u256,
48+
f3: u256,
49+
f4: u256,
50+
f5: u256,
51+
f6: u256,
52+
f7: u256,
53+
}
54+
55+
#[event]
56+
struct E16Data {
57+
#[indexed]
58+
who: Address,
59+
f0: u256,
60+
f1: u256,
61+
f2: u256,
62+
f3: u256,
63+
f4: u256,
64+
f5: u256,
65+
f6: u256,
66+
f7: u256,
67+
f8: u256,
68+
f9: u256,
69+
f10: u256,
70+
f11: u256,
71+
f12: u256,
72+
f13: u256,
73+
f14: u256,
74+
f15: u256,
75+
}
76+
77+
#[event]
78+
struct E7NoIdx {
79+
f0: u256,
80+
f1: u256,
81+
f2: u256,
82+
f3: u256,
83+
f4: u256,
84+
f5: u256,
85+
f6: u256,
86+
}
87+
88+
msg ProbeMsg {
89+
#[selector = sol("emitE6Data()")]
90+
EmitE6Data -> u256,
91+
#[selector = sol("emitE7Data()")]
92+
EmitE7Data -> u256,
93+
#[selector = sol("emitE8Data()")]
94+
EmitE8Data -> u256,
95+
#[selector = sol("emitE16Data()")]
96+
EmitE16Data -> u256,
97+
#[selector = sol("emitE7NoIdx()")]
98+
EmitE7NoIdx -> u256,
99+
}
100+
101+
pub contract Probe uses (log: mut Log) {
102+
recv ProbeMsg {
103+
EmitE6Data -> u256 uses (mut log) {
104+
log.emit(E6Data { who: Address::zero(), f0: 0, f1: 1, f2: 2, f3: 3, f4: 4, f5: 5 })
105+
let ret: u256 = 1
106+
ret
107+
}
108+
109+
EmitE7Data -> u256 uses (mut log) {
110+
log.emit(E7Data { who: Address::zero(), f0: 0, f1: 1, f2: 2, f3: 3, f4: 4, f5: 5, f6: 6 })
111+
let ret: u256 = 2
112+
ret
113+
}
114+
115+
EmitE8Data -> u256 uses (mut log) {
116+
log.emit(E8Data { who: Address::zero(), f0: 0, f1: 1, f2: 2, f3: 3, f4: 4, f5: 5, f6: 6, f7: 7 })
117+
let ret: u256 = 3
118+
ret
119+
}
120+
121+
EmitE16Data -> u256 uses (mut log) {
122+
log.emit(E16Data { who: Address::zero(), f0: 0, f1: 1, f2: 2, f3: 3, f4: 4, f5: 5, f6: 6, f7: 7, f8: 8, f9: 9, f10: 10, f11: 11, f12: 12, f13: 13, f14: 14, f15: 15 })
123+
let ret: u256 = 4
124+
ret
125+
}
126+
127+
EmitE7NoIdx -> u256 uses (mut log) {
128+
log.emit(E7NoIdx { f0: 0, f1: 1, f2: 2, f3: 3, f4: 4, f5: 5, f6: 6 })
129+
let ret: u256 = 5
130+
ret
131+
}
132+
}
133+
}
134+
135+
#[test]
136+
fn topic0_e6data() {
137+
assert!(E6Data::TOPIC0 == 0x7463918f4764895eb86587f42b18b64e71ab12ab37bbc6fe496d5fb1e6afcca1)
138+
}
139+
140+
#[test]
141+
fn topic0_e7data() {
142+
assert!(E7Data::TOPIC0 == 0x492ad8dcfdc2ae45e68d9860da28463849e0b3786764cf67ff41f547b722344f)
143+
}
144+
145+
#[test]
146+
fn topic0_e8data() {
147+
assert!(E8Data::TOPIC0 == 0x47ca2fecc9a425cabbe700e22b112fbd1cfe9a9b18d13ad3d3f369d4909a73f8)
148+
}
149+
150+
#[test]
151+
fn topic0_e16data() {
152+
assert!(E16Data::TOPIC0 == 0x7b8ff7a566201b2975dd0fe44b67e5c8c6cfe52a44562086e166c9cdd54d501a)
153+
}
154+
155+
#[test]
156+
fn topic0_e7noidx() {
157+
assert!(E7NoIdx::TOPIC0 == 0x34dd3fca85336d31367714898b156d6672983033983d23af9534a4f89df503c2)
158+
}
159+
160+
#[test]
161+
fn emit_many_field_events() uses (evm: mut Evm) {
162+
let c = evm.create2<Probe>(value: 0, args: (), salt: 0)
163+
let r0: u256 = evm.call(addr: c, gas: 5000000, value: 0, message: ProbeMsg::EmitE6Data {})
164+
assert!(r0 == 1)
165+
let r1: u256 = evm.call(addr: c, gas: 5000000, value: 0, message: ProbeMsg::EmitE7Data {})
166+
assert!(r1 == 2)
167+
let r2: u256 = evm.call(addr: c, gas: 5000000, value: 0, message: ProbeMsg::EmitE8Data {})
168+
assert!(r2 == 3)
169+
let r3: u256 = evm.call(addr: c, gas: 5000000, value: 0, message: ProbeMsg::EmitE16Data {})
170+
assert!(r3 == 4)
171+
let r4: u256 = evm.call(addr: c, gas: 5000000, value: 0, message: ProbeMsg::EmitE7NoIdx {})
172+
assert!(r4 == 5)
173+
}

‎crates/hir/src/analysis/ty/ty_check/mod.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1193,6 +1193,18 @@ impl<'db> TyChecker<'db> {
11931193
return TraitObligationOutcome::Discharged;
11941194
}
11951195

1196+
if final_pass {
1197+
// Apply literal fallbacks (e.g. string type variables defaulting to
1198+
// `String<N>`) before the last solving attempt. The typed body will
1199+
// contain the fallback types anyway, and without this an
1200+
// unsatisfied goal that mentions a literal variable would be
1201+
// discharged as "not concrete" below, silently suppressing the
1202+
// diagnostic and letting the error surface as an ICE in later
1203+
// lowering stages.
1204+
let mut prober = env::Prober::new(&mut self.table, scope);
1205+
obligation.goal = obligation.goal.fold_with(db, &mut prober);
1206+
}
1207+
11961208
obligation.goal = self.normalize_trait_goal(obligation.goal);
11971209
let goal = obligation.goal;
11981210
let flags = collect_flags(db, goal);

‎crates/hir/src/core/lower/event.rs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,23 @@ fn create_topic0_const<'db>(
364364
)));
365365
tuple_elems.push(body_ctxt.push_expr(close_paren, origin.clone()));
366366

367+
// The `AsBytes` tuple impls in `core` stop at arity 16, so nest the
368+
// elements into chunks when the signature is longer. Byte concatenation is
369+
// associative, so the nesting doesn't change the hashed bytes.
370+
const MAX_TUPLE_ARITY: usize = 16;
371+
while tuple_elems.len() > MAX_TUPLE_ARITY {
372+
tuple_elems = tuple_elems
373+
.chunks(MAX_TUPLE_ARITY)
374+
.map(|chunk| {
375+
if let [single] = chunk {
376+
*single
377+
} else {
378+
body_ctxt.push_expr(Expr::Tuple(chunk.to_vec()), origin.clone())
379+
}
380+
})
381+
.collect();
382+
}
383+
367384
// Build the tuple expression and wrap in keccak call
368385
let tuple_expr = Expr::Tuple(tuple_elems);
369386
let tuple_id = body_ctxt.push_expr(tuple_expr, origin.clone());

‎crates/hir/test_files/desugar/event_many_fields.fe‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
use std::evm::Address
2+
3+
// 13 total fields -> 28 signature-tuple elements, which exceeds the
4+
// `AsBytes` tuple arity limit (16), so the TOPIC0 keccak tuple is nested.
5+
#[event]
6+
pub struct ManyFields {
7+
#[indexed]
8+
pub who: Address,
9+
pub f0: u256,
10+
pub f1: u256,
11+
pub f2: u256,
12+
pub f3: u256,
13+
pub f4: u256,
14+
pub f5: u256,
15+
pub f6: u256,
16+
pub f7: u256,
17+
pub f8: u256,
18+
pub f9: u256,
19+
pub f10: u256,
20+
pub f11: u256,
21+
}

‎crates/hir/test_files/desugar/event_many_fields.snap‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
---
2+
source: crates/hir/tests/desugar.rs
3+
expression: output
4+
input_file: test_files/desugar/event_many_fields.fe
5+
---
6+
use core::prelude::*
7+
use std::prelude::*
8+
use std::evm::Address
9+
10+
pub struct ManyFields {
11+
pub who: Address,
12+
pub f0: u256,
13+
pub f1: u256,
14+
pub f2: u256,
15+
pub f3: u256,
16+
pub f4: u256,
17+
pub f5: u256,
18+
pub f6: u256,
19+
pub f7: u256,
20+
pub f8: u256,
21+
pub f9: u256,
22+
pub f10: u256,
23+
pub f11: u256,
24+
}
25+
26+
impl std::evm::Event for ManyFields {
27+
const TOPIC0: u256 = core::keccak((("ManyFields", "(", Address::SOL_TYPE, ",", u256::SOL_TYPE, ",", u256::SOL_TYPE, ",", u256::SOL_TYPE, ",", u256::SOL_TYPE, ",", u256::SOL_TYPE, ",", u256::SOL_TYPE, ","), (u256::SOL_TYPE, ",", u256::SOL_TYPE, ",", u256::SOL_TYPE, ",", u256::SOL_TYPE, ",", u256::SOL_TYPE, ",", u256::SOL_TYPE, ")")))
28+
29+
#[inline(always)]
30+
fn emit<L: std::evm::Log>(own self, _ log: mut L) {
31+
let (data_ptr, data_len) = std::evm::encode_event_payload((self.f0, self.f1, self.f2, self.f3, self.f4, self.f5, self.f6, self.f7, self.f8, self.f9, self.f10, self.f11))
32+
log.log2(offset: data_ptr, len: data_len, topic0: Self::TOPIC0, topic1: self.who.as_topic())
33+
}
34+
}

‎crates/mir/src/runtime/lower/body.rs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4969,7 +4969,13 @@ impl<'db> RmirEmitter<'db> {
49694969
.semantic(self.db)
49704970
.expect("runtime const reification requires a semantic instance");
49714971
reify_runtime_const_for_ty(self.db, semantic, expected_ty, value).unwrap_or_else(|| {
4972-
panic!("semantic const should reify for runtime lowering: {value:?}")
4972+
panic!(
4973+
"semantic const should reify for runtime lowering: `{}` (expected type `{}`). \
4974+
This is a compiler bug: the const failed to evaluate but no diagnostic was \
4975+
reported during type checking.",
4976+
value.pretty_print(self.db),
4977+
expected_ty.pretty_print(self.db),
4978+
)
49734979
})
49744980
}
49754981

‎crates/uitest/fixtures/ty_check/event_too_many_data_fields.fe‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Events are encoded through the `EventAbiEncode` tuple impls, which stop at
2+
// arity 16. An event with 17 non-indexed fields must produce a trait-bound
3+
// diagnostic pointing at the event definition instead of an ICE.
4+
use std::evm::Log
5+
6+
#[event]
7+
struct TooWide {
8+
f0: u256,
9+
f1: u256,
10+
f2: u256,
11+
f3: u256,
12+
f4: u256,
13+
f5: u256,
14+
f6: u256,
15+
f7: u256,
16+
f8: u256,
17+
f9: u256,
18+
f10: u256,
19+
f11: u256,
20+
f12: u256,
21+
f13: u256,
22+
f14: u256,
23+
f15: u256,
24+
f16: u256,
25+
}

‎crates/uitest/fixtures/ty_check/event_too_many_data_fields.snap‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
source: crates/uitest/tests/ty_check.rs
3+
expression: diags
4+
input_file: fixtures/ty_check/event_too_many_data_fields.fe
5+
---
6+
error[6-0003]: trait bound is not satisfied
7+
┌─ event_too_many_data_fields.fe:6:1
8+
│
9+
6 │ ╭ #[event]
10+
7 │ │ struct TooWide {
11+
8 │ │ f0: u256,
12+
9 │ │ f1: u256,
13+
· │
14+
24 │ │ f16: u256,
15+
25 │ │ }
16+
│ ╰─^ `(u256, u256, u256, u256, u256, u256, u256, u256, u256, u256, u256, u256, u256, u256, u256, u256, u256)` doesn't implement `EventAbiEncode<Sol>`
17+
│
18+
┌─ src/evm/effects.fe:438:14
19+
│
20+
438 │ where T: EventAbiEncode<Sol>
21+
│ ------------------- required by this bound on `encode_event_payload`

‎crates/uitest/fixtures/ty_check/string_literal_trait_bound_not_sat.fe‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Unsatisfied trait bounds on goals that mention string literal types used to
2+
// be silently discharged: string literals carry a string-sorted inference
3+
// variable until the `String<N>` fallback is applied, and the final
4+
// obligation pass treated such goals as "not concrete" and skipped the
5+
// diagnostic. The error then surfaced as an ICE during MIR lowering.
6+
use core::AsBytes
7+
8+
fn takes_asbytes<T: AsBytes>(_ x: T) -> u256 {
9+
0
10+
}
11+
12+
fn tuple_with_unsatisfied_elem() -> u256 {
13+
takes_asbytes(("a", true))
14+
}
15+
16+
fn tuple_arity_above_asbytes_impls() -> u256 {
17+
core::keccak(("a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o", "p", "q", "r"))
18+
}

‎crates/uitest/fixtures/ty_check/string_literal_trait_bound_not_sat.snap‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
source: crates/uitest/tests/ty_check.rs
3+
expression: diags
4+
input_file: fixtures/ty_check/string_literal_trait_bound_not_sat.fe
5+
---
6+
error[6-0003]: trait bound is not satisfied
7+
┌─ string_literal_trait_bound_not_sat.fe:13:5
8+
│
9+
8 │ fn takes_asbytes<T: AsBytes>(_ x: T) -> u256 {
10+
│ ------- required by this bound on `takes_asbytes`
11+
·
12+
13 │ takes_asbytes(("a", true))
13+
│ ^^^^^^^^^^^^^
14+
│ │
15+
│ `(String<1>, bool)` doesn't implement `AsBytes`
16+
│ trait bound `bool: AsBytes` is not satisfied
17+
18+
error[6-0003]: trait bound is not satisfied
19+
┌─ string_literal_trait_bound_not_sat.fe:17:5
20+
│
21+
17 │ core::keccak(("a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o", "p", "q", "r"))
22+
│ ^^^^^^^^^^^^ `(String<1>, String<1>, String<1>, String<1>, String<1>, String<1>, String<1>, String<1>, String<1>, String<1>, String<1>, String<1>, String<1>, String<1>, String<1>, String<1>, String<1>, String<1>)` doesn't implement `AsBytes`
23+
│
24+
┌─ src/bytes.fe:443:24
25+
│
26+
443 │ pub const fn keccak<T: AsBytes>(_ x: T) -> u256 {
27+
│ ------- required by this bound on `keccak`

0 commit comments

Comments
 (0)