Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changelog/lint-defaulted-loop-counters.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
forge: patch
forge-lint: patch
---

Avoid uninitialized-local warnings for unsigned counters that implicitly start at zero in conventional for-loop headers.
6 changes: 6 additions & 0 deletions crates/lint/docs/uninitialized-local.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ Flags local variables that are declared without an initializer and then read bef

Reports any local variable of `VarKind::Statement` (i.e., a variable declared inside a function body, not a parameter or state variable) whose first use is a read and which has never been explicitly assigned prior to that read on at least one execution path.

Unsigned counters declared in a `for` initializer may intentionally start at zero, as in
`for (uint256 i; i < n; ++i)`. The lint exempts these counters when the condition compares
them against an upper bound and the header update uses `++i` or `i++`. Other uninitialized
locals read by the condition or body still produce warnings. Counters declared outside the
header or incremented inside the body retain their existing diagnostics.

## Why is this bad?

Reading an uninitialized variable means the code silently depends on a language-level zero-default rather than an explicit value chosen by the developer. Common consequences include:
Expand Down
42 changes: 40 additions & 2 deletions crates/lint/src/sol/med/uninitialized_local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@ use crate::{
},
};
use solar::{
ast::ElementaryType,
interface::{Span, data_structures::Never},
sema::{
Gcx, Hir,
hir::{
Expr, ExprKind, Function, LoopSource, Res, Stmt, StmtKind, TypeKind, VarKind,
VariableId, Visit,
BinOpKind, Block, Expr, ExprKind, Function, LoopSource, Res, Stmt, StmtKind, TypeKind,
UnOpKind, VarKind, VariableId, Visit,
},
},
};
Expand Down Expand Up @@ -64,6 +65,36 @@ impl Checker<'_> {
}
}

/// Recognizes an unsigned counter whose implicit zero is intentional in a conventional `for`
/// header. Matching the lowered wrapper's span keeps declarations outside the header distinct.
fn defaulted_counter_loop<'hir>(
hir: &Hir<'hir>,
block: &'hir Block<'hir>,
) -> Option<&'hir Stmt<'hir>> {
if let [Stmt { kind: StmtKind::DeclSingle(vid), .. }, loop_stmt] = block.stmts
&& let StmtKind::Loop(body, LoopSource::ForWithUpdate) = &loop_stmt.kind
&& block.span == loop_stmt.span
&& hir.variable(*vid).initializer.is_none()
&& matches!(hir.variable(*vid).ty.kind, TypeKind::Elementary(ElementaryType::UInt(_)))
&& let [Stmt { kind: StmtKind::If(condition, then, Some(else_)), .. }] = body.stmts
&& matches!(else_.kind, StmtKind::Break)
&& let ExprKind::Binary(left, op, right) = &condition.peel_parens().kind
&& ((matches!(op.kind, BinOpKind::Lt | BinOpKind::Le) && left.as_variable() == Some(*vid))
|| (matches!(op.kind, BinOpKind::Gt | BinOpKind::Ge)
&& right.as_variable() == Some(*vid)))
&& let StmtKind::Block(inner) = &then.kind
&& inner.span == body.span
&& let [_, Stmt { kind: StmtKind::Expr(update), .. }] = inner.stmts
&& let ExprKind::Unary(op, target) = &update.peel_parens().kind
&& matches!(op.kind, UnOpKind::PreInc | UnOpKind::PostInc)
&& target.as_variable() == Some(*vid)
{
Some(loop_stmt)
} else {
None
}
}

impl<'hir> Visit<'hir> for Checker<'hir> {
type BreakValue = Never;

Expand All @@ -73,6 +104,13 @@ impl<'hir> Visit<'hir> for Checker<'hir> {

fn visit_stmt(&mut self, stmt: &'hir Stmt<'hir>) -> ControlFlow<Never> {
match &stmt.kind {
StmtKind::Block(block) => {
if let Some(loop_stmt) = defaulted_counter_loop(self.hir, block) {
// Skip only the counter's declaration; all reads in the loop still run
// through the ordinary checker, including reads of other locals.
return self.visit_stmt(loop_stmt);
}
}
StmtKind::DeclSingle(vid) => {
let v = self.hir.variable(*vid);
if v.kind == VarKind::Statement
Expand Down
99 changes: 99 additions & 0 deletions crates/lint/testdata/UninitializedLocalFor.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
//@compile-flags: --only-lint uninitialized-local

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

contract UninitializedLocalFor {
// Header counters deliberately start at zero, including reads in the loop body.
function counters(uint256 n) public pure returns (uint256 sum) {
for (uint256 i; i < n; ++i) {
sum += i;
}
for (uint8 j; j <= 10; j++) {
sum += j;
}
for (uint256 k; n > (k); (k)++) {
for (uint256 m; n >= m; ++m) {
sum += k + m;
}
}
}

// A declaration outside the header must not match the synthetic wrapper.
function outsideHeader(uint256 n) public pure {
{
uint256 i;
for (; i < n; ++i) {} //~WARN: local variable is read before being initialized
}
}

// Only the loop counter is exempt; an uninitialized bound is still a read.
function missingBound() public pure {
uint256 n;
for (uint256 i; i < n; ++i) {} //~WARN: local variable is read before being initialized
}

// The loop might run, so findings in its body must not be rolled back.
function bodyRead(uint256 n) public pure returns (uint256 sum) {
for (uint256 i; i < n; ++i) {
uint256 amount;
sum += amount; //~WARN: local variable is read before being initialized
}
}

// The loop might not run, so its writes do not initialize another local afterwards.
function zeroIterations(uint256 n) public pure returns (uint256) {
uint256 amount;
for (uint256 i; i < n; ++i) {
amount = i;
}
return amount; //~WARN: local variable is read before being initialized
}

// A header declaration alone does not make an unrelated local a counter.
function unrelatedHeader(uint256 n) public pure returns (uint256 sum) {
uint256 i = 0;
for (uint256 amount; i < n; ++i) {
sum += amount; //~WARN: local variable is read before being initialized
}
}

// Descending from an implicit zero is not the ascending-counter idiom.
function decrement(uint256 n) public pure {
for (uint256 i; i < n; --i) {} //~WARN: local variable is read before being initialized
}

// A condition that happens to read a local is not enough without its increment.
function differentUpdate(uint256 n) public pure {
for (uint256 i; i < n; --n) {} //~WARN: local variable is read before being initialized
}

// Loops without a header update keep their existing diagnostics.
function bodyUpdate(uint256 n) public pure {
for (uint256 i; i < n;) { //~WARN: local variable is read before being initialized
++i;
}
}

// A standalone compound read still relies on an unintended default.
function compoundRead() public pure returns (uint256) {
uint256 amount;
amount += 1; //~WARN: local variable is read before being initialized
return amount;
}

// Nested counter exemptions must not suppress another local's read.
function nestedRead(uint256 n) public pure returns (uint256 sum) {
for (uint256 i; i < n; ++i) {
for (uint256 j; j < n; ++j) {
uint256 amount;
sum += amount; //~WARN: local variable is read before being initialized
}
}
}

// The exception is limited to unsigned counters.
function signedCounter(int256 n) public pure {
for (int256 i; i < n; ++i) {} //~WARN: local variable is read before being initialized
}
}
88 changes: 88 additions & 0 deletions crates/lint/testdata/UninitializedLocalFor.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
warning[uninitialized-local]: local variable is read before being initialized
╭▸ ROOT/testdata/UninitializedLocalFor.sol:LL:CC
LL │ for (; i < n; ++i) {}
│ ━
╰ help: https://getfoundry.sh/forge/linting/uninitialized-local

warning[uninitialized-local]: local variable is read before being initialized
╭▸ ROOT/testdata/UninitializedLocalFor.sol:LL:CC
LL │ for (uint256 i; i < n; ++i) {}
│ ━
╰ help: https://getfoundry.sh/forge/linting/uninitialized-local

warning[uninitialized-local]: local variable is read before being initialized
╭▸ ROOT/testdata/UninitializedLocalFor.sol:LL:CC
LL │ sum += amount;
│ ━━━━━━
╰ help: https://getfoundry.sh/forge/linting/uninitialized-local

warning[uninitialized-local]: local variable is read before being initialized
╭▸ ROOT/testdata/UninitializedLocalFor.sol:LL:CC
LL │ return amount;
│ ━━━━━━
╰ help: https://getfoundry.sh/forge/linting/uninitialized-local

warning[uninitialized-local]: local variable is read before being initialized
╭▸ ROOT/testdata/UninitializedLocalFor.sol:LL:CC
LL │ sum += amount;
│ ━━━━━━
╰ help: https://getfoundry.sh/forge/linting/uninitialized-local

warning[uninitialized-local]: local variable is read before being initialized
╭▸ ROOT/testdata/UninitializedLocalFor.sol:LL:CC
LL │ for (uint256 i; i < n; --i) {}
│ ━
╰ help: https://getfoundry.sh/forge/linting/uninitialized-local

warning[uninitialized-local]: local variable is read before being initialized
╭▸ ROOT/testdata/UninitializedLocalFor.sol:LL:CC
LL │ for (uint256 i; i < n; --n) {}
│ ━
╰ help: https://getfoundry.sh/forge/linting/uninitialized-local

warning[uninitialized-local]: local variable is read before being initialized
╭▸ ROOT/testdata/UninitializedLocalFor.sol:LL:CC
LL │ for (uint256 i; i < n;) {
│ ━
╰ help: https://getfoundry.sh/forge/linting/uninitialized-local

warning[uninitialized-local]: local variable is read before being initialized
╭▸ ROOT/testdata/UninitializedLocalFor.sol:LL:CC
LL │ amount += 1;
│ ━━━━━━
╰ help: https://getfoundry.sh/forge/linting/uninitialized-local

warning[uninitialized-local]: local variable is read before being initialized
╭▸ ROOT/testdata/UninitializedLocalFor.sol:LL:CC
LL │ sum += amount;
│ ━━━━━━
╰ help: https://getfoundry.sh/forge/linting/uninitialized-local

warning[uninitialized-local]: local variable is read before being initialized
╭▸ ROOT/testdata/UninitializedLocalFor.sol:LL:CC
LL │ for (int256 i; i < n; ++i) {}
│ ━
╰ help: https://getfoundry.sh/forge/linting/uninitialized-local

Loading