Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the neverAccessedVariables lint and the fixpoint solver #16

Merged
merged 10 commits into from
Jun 28, 2024
175 changes: 175 additions & 0 deletions src/detectors/builtin/neverAccessedVariables.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
import { ASTStatement, ASTRef } from "@tact-lang/compiler/dist/grammar/ast";
import { Solver } from "../../internals/solver";
import { Detector } from "../detector";
import { JoinSemilattice } from "../../internals/lattice";
import { MistiContext } from "../../internals/context";
import { CompilationUnit, Node, CFG } from "../../internals/ir";
import { createError, MistiTactError, Severity } from "../../internals/errors";
import { forEachExpression } from "../../internals/tactASTUtil";

type FieldName = string;
type ConstantName = string;

interface VariableState {
declared: Set<[string, ASTRef]>;
used: Set<string>;
}

/**
* A powerset lattice that keeps state of local varialbes within control flow.
*/
class VariableUsageLattice implements JoinSemilattice<VariableState> {
bottom(): VariableState {
return { declared: new Set(), used: new Set() };
}

join(a: VariableState, b: VariableState): VariableState {
const joinedDeclared = new Set([...a.declared, ...b.declared]);
const joinedUsed = new Set([...a.used, ...b.used]);
return { declared: joinedDeclared, used: joinedUsed };
}

leq(a: VariableState, b: VariableState): boolean {
return (
[...a.declared].every((x) => b.declared.has(x)) &&
[...a.used].every((x) => b.used.has(x))
);
}
}

/**
* A detector that identifies write-only or unused variables, fields and constants.
*
* These variables are either assigned but never used in any meaningful computation,
* or they are declared and never used at all, which may indicate redundant code
* or an incomplete implementation of the intended logic.
*/
export class NeverAccessedVariables extends Detector {
check(_ctx: MistiContext, cu: CompilationUnit): MistiTactError[] {
return [
...this.checkFields(cu),
...this.checkConstants(cu),
...this.checkVariables(cu),
];
}

checkFields(cu: CompilationUnit): MistiTactError[] {
const definedFields = this.collectDefinedFields(cu);
const usedFields = this.collectUsedFields(cu);
return Array.from(
new Set(
[...definedFields].filter(([name, _ref]) => !usedFields.has(name)),
),
).map(([name, ref]) =>
createError(`Field ${name} is never used`, Severity.MEDIUM, ref),
);
}

collectDefinedFields(cu: CompilationUnit): Set<[FieldName, ASTRef]> {
return Array.from(cu.ast.getContracts()).reduce((acc, contract) => {
contract.declarations.forEach((decl) => {
if (decl.kind === "def_field") {
acc.add([decl.name, decl.ref]);
}
});
return acc;
}, new Set<[FieldName, ASTRef]>());
}

collectUsedFields(cu: CompilationUnit): Set<FieldName> {
return Array.from(cu.ast.getFunctions()).reduce((acc, fun) => {
forEachExpression(fun, (expr) => {
if (expr.kind === "op_field" && expr.src.kind === "id") {
acc.add(expr.src.value);
}
});
return acc;
}, new Set<FieldName>());
}

checkConstants(cu: CompilationUnit): MistiTactError[] {
const definedConstants = this.collectDefinedConstants(cu);
const usedConstants = this.collectUsedNames(cu);
return Array.from(
new Set(
[...definedConstants].filter(
([name, _ref]) => !usedConstants.has(name),
),
),
).map(([name, ref]) =>
createError(`Constant ${name} is never used`, Severity.MEDIUM, ref),
);
}

collectDefinedConstants(cu: CompilationUnit): Set<[ConstantName, ASTRef]> {
return Array.from(cu.ast.getConstants(false)).reduce((acc, constant) => {
acc.add([constant.name, constant.ref]);
return acc;
}, new Set<[ConstantName, ASTRef]>());
}

/**
* Collects all the identifiers using withing all the statements.
*/
collectUsedNames(cu: CompilationUnit): Set<ConstantName> {
return Array.from(cu.ast.getStatements()).reduce((acc, stmt) => {
forEachExpression(stmt, (expr) => {
if (expr.kind === "id") {
acc.add(expr.value);
}
});
return acc;
}, new Set<FieldName>());
}

/**
* Checks never accessed local variables in all the functions leveraging the
* monotonic framework and the fixpoint dataflow solver.
*/
checkVariables(cu: CompilationUnit): MistiTactError[] {
const errors: MistiTactError[] = [];
cu.forEachCFG(cu.ast, (cfg: CFG, _: Node, stmt: ASTStatement) => {
if (cfg.origin === "stdlib") {
return;
}
const lattice = new VariableUsageLattice();
const transfer = (_node: Node, inState: VariableState) => {
const outState = { ...inState };
if (stmt.kind === "statement_let") {
outState.declared.add([stmt.name, stmt.ref]);
} else {
forEachExpression(stmt, (expr) => {
if (expr.kind === "id") {
outState.used.add(expr.value);
}
});
}
return outState;
};
const solver = new Solver(cfg, transfer, lattice);
const results = solver.findFixpoint();

const declaredVariables = new Map<string, ASTRef>();
const usedVariables = new Set<string>();
results.getStates().forEach((state) => {
state.declared.forEach(([name, ref]) =>
declaredVariables.set(name, ref),
);
state.used.forEach((name) => usedVariables.add(name));
});
Array.from(declaredVariables.keys()).forEach((name) => {
if (!usedVariables.has(name)) {
errors.push(
createError(
`Variable ${name} is never accessed`,
Severity.MEDIUM,
declaredVariables.get(name)!,
),
);
}
});
});

return errors;
}
}
25 changes: 10 additions & 15 deletions src/detectors/builtin/readOnlyVariables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,13 @@ export class ReadOnlyVariables extends Detector {
* @param ctx The Souffle program to which the facts are added.
*/
addConstraints(cu: CompilationUnit, ctx: Context<ASTRef>) {
const addUses = (funName: string, node: ASTStatement | ASTExpression) => {
forEachExpression(node, (expr: ASTExpression) => {
if (expr.kind === "id") {
ctx.addFact("varUse", Fact.from([expr.value, funName], expr.ref));
}
});
};
cu.forEachCFG(cu.ast, (cfg: CFG, _: Node, stmt: ASTStatement) => {
if (cfg.origin === "stdlib") {
return;
Expand All @@ -114,30 +121,18 @@ export class ReadOnlyVariables extends Detector {
switch (stmt.kind) {
case "statement_let":
ctx.addFact("varDecl", Fact.from([stmt.name, funName], stmt.ref));
forEachExpression(stmt.expression, (expr: ASTExpression) => {
if (expr.kind === "id") {
ctx.addFact("varUse", Fact.from([expr.value, funName], expr.ref));
}
});
addUses(funName, stmt.expression);
break;
case "statement_assign":
case "statement_augmentedassign":
ctx.addFact(
"varAssign",
Fact.from([stmt.path[0].name, funName], stmt.ref),
);
forEachExpression(stmt.expression, (expr: ASTExpression) => {
if (expr.kind === "id") {
ctx.addFact("varUse", Fact.from([expr.value, funName], expr.ref));
}
});
addUses(funName, stmt.expression);
break;
default:
forEachExpression(stmt, (expr: ASTExpression) => {
if (expr.kind === "id") {
ctx.addFact("varUse", Fact.from([expr.value, funName], stmt.ref));
}
});
addUses(funName, stmt);
break;
}
});
Expand Down
4 changes: 4 additions & 0 deletions src/detectors/detector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ const BuiltInDetectors: Record<string, () => Promise<Detector>> = {
import("./builtin/readOnlyVariables").then(
(module) => new module.ReadOnlyVariables(),
),
NeverAccessedVariables: () =>
import("./builtin/neverAccessedVariables").then(
(module) => new module.NeverAccessedVariables(),
),
ZeroAddress: () =>
import("./builtin/zeroAddress").then((module) => new module.ZeroAddress()),
};
Expand Down
1 change: 1 addition & 0 deletions src/internals/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const ConfigSchema = z.object({
/** Built-in detectors enabled by default, if no user configuration is provided. */
export const BUILTIN_DETECTORS: DetectorConfig[] = [
{ className: "ReadOnlyVariables" },
{ className: "NeverAccessedVariables" },
{ className: "ZeroAddress" },
];

Expand Down
Loading
Loading