Skip to content

Commit f9c8d7c

Browse files
authored
fix(slang): use declared type for state variable storage operations (#358)
State variable loads, stores, increments, and compound assignments hardcoded ui256 instead of resolving the declared type from Slang's `StateVariableDefinition::get_type()`. This caused incorrect overflow behavior for smaller types — e.g. `uint8 x = 255; x++` would not revert in checked mode because the checked add operated at 256-bit width. The fix threads the resolved element type through `emit_storage_load` and `AssignmentTarget::Storage`, matching how local variables already work.
1 parent 5001835 commit f9c8d7c

8 files changed

Lines changed: 50 additions & 34 deletions

File tree

solx-slang/src/ast/contract/function/expression/arithmetic.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -192,12 +192,12 @@ impl<'state, 'context, 'block> ExpressionEmitter<'state, 'context, 'block> {
192192
.storage_layout
193193
.get(&state_variable.node_id())
194194
.ok_or_else(|| anyhow::anyhow!("unregistered state variable: {name}"))?;
195-
let old = self.emit_storage_load(slot, block)?;
196-
// TODO: use declared element type once state variable types are tracked.
197-
// Currently hardcodes ui256, so `uint8 x = 255; x++` won't revert
198-
// in checked mode because overflow is checked at 256-bit width.
199-
let ui256 = self.state.builder.types.ui256;
200-
let one = self.state.builder.emit_sol_constant(1, ui256, block);
195+
let element_type = TypeConversion::resolve_state_variable_type(
196+
&state_variable,
197+
&self.state.builder,
198+
)?;
199+
let old = self.emit_storage_load(slot, element_type, block)?;
200+
let one = self.state.builder.emit_sol_constant(1, element_type, block);
201201
let new_value = block
202202
.append_operation(operator.emit_sol_binary_operation(
203203
self.checked,

solx-slang/src/ast/contract/function/expression/assignment.rs

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ use crate::ast::contract::function::expression::operator::Operator;
1919
enum AssignmentTarget<'context, 'block> {
2020
/// Local variable — alloca'd pointer and its declared element type.
2121
Local(Value<'context, 'block>, Type<'context>),
22-
/// State variable — storage slot.
23-
Storage(u64),
22+
/// State variable — storage slot and declared element type.
23+
Storage(u64, Type<'context>),
2424
}
2525

2626
impl<'state, 'context, 'block> ExpressionEmitter<'state, 'context, 'block> {
@@ -42,7 +42,11 @@ impl<'state, 'context, 'block> ExpressionEmitter<'state, 'context, 'block> {
4242
.storage_layout
4343
.get(&state_variable.node_id())
4444
.ok_or_else(|| anyhow::anyhow!("unregistered state variable: {name}"))?;
45-
AssignmentTarget::Storage(*slot)
45+
let element_type = TypeConversion::resolve_state_variable_type(
46+
&state_variable,
47+
&self.state.builder,
48+
)?;
49+
AssignmentTarget::Storage(*slot, element_type)
4650
}
4751
Some(Definition::Variable(_) | Definition::Parameter(_)) => {
4852
let (pointer, element_type) = self
@@ -69,9 +73,9 @@ impl<'state, 'context, 'block> ExpressionEmitter<'state, 'context, 'block> {
6973
.emit_sol_load(pointer, element_type, &block)?;
7074
(old, element_type)
7175
}
72-
AssignmentTarget::Storage(slot) => {
73-
let old = self.emit_storage_load(slot, &block)?;
74-
(old, self.state.builder.types.ui256)
76+
AssignmentTarget::Storage(slot, element_type) => {
77+
let old = self.emit_storage_load(slot, element_type, &block)?;
78+
(old, element_type)
7579
}
7680
};
7781
let (rhs, block) = self.emit_value(&right, block)?;
@@ -112,11 +116,14 @@ impl<'state, 'context, 'block> ExpressionEmitter<'state, 'context, 'block> {
112116
.emit_sol_store(stored_value, pointer, &block);
113117
stored_value
114118
}
115-
AssignmentTarget::Storage(slot) => {
116-
let ui256 = self.state.builder.types.ui256;
117-
let stored_value = self.state.builder.emit_sol_cast(value, ui256, &block);
119+
AssignmentTarget::Storage(slot, element_type) => {
120+
let stored_value = TypeConversion::from_target_type(
121+
element_type,
122+
&self.state.builder,
123+
)
124+
.emit(value, &self.state.builder, &block);
118125
self.emit_storage_store(slot, stored_value, &block);
119-
value
126+
stored_value
120127
}
121128
};
122129
Ok((result, block))

solx-slang/src/ast/contract/function/expression/call/type_conversion.rs

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use melior::ir::Type;
66
use melior::ir::ValueLike;
77
use melior::ir::r#type::IntegerType;
8+
use slang_solidity::backend::ir::ast::StateVariableDefinition;
89
use slang_solidity::backend::ir::ast::Type as SlangType;
910

1011
/// Classification of Solidity type conversions.
@@ -24,20 +25,19 @@ impl<'context> TypeConversion<'context> {
2425
/// Maps a Slang semantic type to an MLIR type.
2526
pub fn resolve_slang_type(
2627
slang_type: &SlangType,
27-
context: &'context melior::Context,
2828
builder: &solx_mlir::Builder<'context>,
2929
) -> Type<'context> {
3030
match slang_type {
3131
SlangType::Integer(integer_type) => {
3232
let bits = integer_type.bits();
3333
if integer_type.signed() {
34-
Type::from(IntegerType::signed(context, bits))
34+
Type::from(IntegerType::signed(builder.context, bits))
3535
} else {
36-
Type::from(IntegerType::unsigned(context, bits))
36+
Type::from(IntegerType::unsigned(builder.context, bits))
3737
}
3838
}
3939
SlangType::Boolean(_) => Type::from(IntegerType::new(
40-
context,
40+
builder.context,
4141
solx_utils::BIT_LENGTH_BOOLEAN as u32,
4242
)),
4343
SlangType::Address(_) => builder.types.sol_address,
@@ -60,6 +60,18 @@ impl<'context> TypeConversion<'context> {
6060
}
6161
}
6262

63+
/// Resolves the declared Solidity type of a state variable to an MLIR type.
64+
pub fn resolve_state_variable_type(
65+
state_variable: &StateVariableDefinition,
66+
builder: &solx_mlir::Builder<'context>,
67+
) -> anyhow::Result<Type<'context>> {
68+
let name = state_variable.name().name();
69+
let slang_type = state_variable
70+
.get_type()
71+
.ok_or_else(|| anyhow::anyhow!("unresolved type for state variable: {name}"))?;
72+
Ok(Self::resolve_slang_type(&slang_type, builder))
73+
}
74+
6375
/// Emits the conversion, returning the cast value.
6476
pub fn emit<'block>(
6577
self,

solx-slang/src/ast/contract/function/expression/mod.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,11 @@ impl<'state, 'context, 'block> ExpressionEmitter<'state, 'context, 'block> {
146146
.ok_or_else(|| {
147147
anyhow::anyhow!("unregistered state variable: {name}")
148148
})?;
149-
let value = self.emit_storage_load(*slot, &block)?;
149+
let element_type = TypeConversion::resolve_state_variable_type(
150+
&state_variable,
151+
&self.state.builder,
152+
)?;
153+
let value = self.emit_storage_load(*slot, element_type, &block)?;
150154
Ok((Some(value), block))
151155
}
152156
Some(Definition::Variable(_) | Definition::Parameter(_)) => {
@@ -340,7 +344,6 @@ impl<'state, 'context, 'block> ExpressionEmitter<'state, 'context, 'block> {
340344
let slang_type = self.semantic.get_type_from_node_id(node_id)?;
341345
Some(TypeConversion::resolve_slang_type(
342346
&slang_type,
343-
self.state.builder.context,
344347
&self.state.builder,
345348
))
346349
}

solx-slang/src/ast/contract/function/expression/storage.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
//!
44
55
use melior::ir::BlockRef;
6+
use melior::ir::Type;
67
use melior::ir::Value;
78

89
use crate::ast::contract::function::expression::ExpressionEmitter;
@@ -12,11 +13,13 @@ impl<'state, 'context, 'block> ExpressionEmitter<'state, 'context, 'block> {
1213
pub fn emit_storage_load(
1314
&self,
1415
slot: u64,
16+
element_type: Type<'context>,
1517
block: &BlockRef<'context, 'block>,
1618
) -> anyhow::Result<Value<'context, 'block>> {
1719
let pointer = self.emit_storage_addr_of(slot, block);
18-
let ui256 = self.state.builder.types.ui256;
19-
self.state.builder.emit_sol_load(pointer, ui256, block)
20+
self.state
21+
.builder
22+
.emit_sol_load(pointer, element_type, block)
2023
}
2124

2225
/// Emits a storage store via `sol.addr_of` + `sol.store`.

solx-slang/src/ast/contract/function/mod.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ impl<'state, 'context> FunctionEmitter<'state, 'context> {
7979
.map(|param| {
8080
TypeConversion::resolve_slang_type(
8181
&param.get_type().expect("parameter type binding resolved"),
82-
self.state.builder.context,
8382
&self.state.builder,
8483
)
8584
})
@@ -92,7 +91,6 @@ impl<'state, 'context> FunctionEmitter<'state, 'context> {
9291
.map(|param| {
9392
TypeConversion::resolve_slang_type(
9493
&param.get_type().expect("return type binding resolved"),
95-
self.state.builder.context,
9694
&self.state.builder,
9795
)
9896
})

solx-slang/src/ast/contract/function/statement/mod.rs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -180,13 +180,7 @@ impl<'state, 'context, 'block> StatementEmitter<'state, 'context, 'block> {
180180
let name = declaration.name().name();
181181
let declared_type = declaration
182182
.get_type()
183-
.map(|slang_type| {
184-
TypeConversion::resolve_slang_type(
185-
&slang_type,
186-
self.state.builder.context,
187-
&self.state.builder,
188-
)
189-
})
183+
.map(|slang_type| TypeConversion::resolve_slang_type(&slang_type, &self.state.builder))
190184
.unwrap_or_else(|| self.state.builder.types.ui256);
191185

192186
let emitter = ExpressionEmitter::new(

solx-slang/src/ast/contract/mod.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,6 @@ impl<'state, 'context> ContractEmitter<'state, 'context> {
116116
.map(|param| {
117117
TypeConversion::resolve_slang_type(
118118
&param.get_type().expect("return type binding resolved"),
119-
self.state.builder.context,
120119
&self.state.builder,
121120
)
122121
})

0 commit comments

Comments
 (0)