fix(callgraph): Incorrect processing of StateWrite for cells (#279)

Closes #278

CHANGELOG.md

@@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `UnboundMap`: False positive: Issue [#262](https://github.com/nowarp/misti/issues/262)
 - Internal Errors Printed to `stderr` Instead of JSON Output: Issue [#263](https://github.com/nowarp/misti/issues/263)
 - `CellBounds`: Infinite recursion: PR [#272](https://github.com/nowarp/misti/pull/272)
+- Callgraph: Incorrect processing of `Effect.StateWrite` for cells: PR [#279](https://github.com/nowarp/misti/pull/279)
 
 ## [0.6.2] - 2024-12-25
 

src/detectors/builtin/ensurePrgSeed.ts

@@ -1,8 +1,8 @@
 import { CompilationUnit } from "../../internals/ir";
 import {
   forEachExpression,
-  PRG_INIT_NAMES,
-  PRG_NATIVE_USE_NAMES,
+  PRG_INIT_FUNCTIONS,
+  PRG_NATIVE_USE_FUNCTIONS,
 } from "../../internals/tact";
 import { MistiTactWarning, Severity } from "../../internals/warnings";
 import { AstDetector } from "../detector";
@@ -46,10 +46,10 @@ export class EnsurePrgSeed extends AstDetector {
       (acc, node) => {
         forEachExpression(node, (expr) => {
           if (expr.kind === "static_call") {
-            if (PRG_INIT_NAMES.has(idText(expr.function))) {
+            if (PRG_INIT_FUNCTIONS.has(idText(expr.function))) {
               acc.hasInitializer = true;
             }
-            if (PRG_NATIVE_USE_NAMES.has(idText(expr.function))) {
+            if (PRG_NATIVE_USE_FUNCTIONS.has(idText(expr.function))) {
               acc.uses.push(expr);
             }
           }
@@ -70,7 +70,7 @@ export class EnsurePrgSeed extends AstDetector {
           `PRG seed should be initialized before using ${idText(use.function)}`,
           use.loc,
           {
-            suggestion: `Use ${Array.from(PRG_INIT_NAMES)
+            suggestion: `Use ${Array.from(PRG_INIT_FUNCTIONS)
               .map((name) => "`" + name + "`")
               .join(
                 ", ",

src/internals/ir/callGraph.ts

@@ -3,16 +3,19 @@ import { AstStore } from "./astStore";
 import { IdxGenerator } from "./indices";
 import { MistiContext } from "../../";
 import { Logger } from "../../internals/logger";
-import { findInExpressions, forEachExpression } from "../tact/iterators";
 import {
-  DATETIME_NAMES,
   isSelfAccess,
   isSendCall,
   isSelf,
-  PRG_INIT_NAMES,
-  PRG_NATIVE_USE_NAMES,
-  PRG_SAFE_USE_NAMES,
-} from "../tact/util";
+  DATETIME_FUNCTIONS,
+  PRG_INIT_FUNCTIONS,
+  PRG_NATIVE_USE_FUNCTIONS,
+  PRG_SAFE_USE_FUNCTIONS,
+  MAP_MUTATING_METHODS,
+  STRING_MUTATING_METHODS,
+  BUILDER_MUTATING_METHODS,
+} from "../tact";
+import { findInExpressions, forEachExpression } from "../tact/iterators";
 import {
   AstFunctionDef,
   AstReceiver,
@@ -413,14 +416,14 @@ export class CallGraph {
     if (!funcNode) return;
     if (expr.kind === "static_call") {
       const functionName = idText(expr.function);
-      if (DATETIME_NAMES.has(functionName))
+      if (DATETIME_FUNCTIONS.has(functionName))
         funcNode.addEffect(Effect.AccessDatetime);
       else if (
-        PRG_NATIVE_USE_NAMES.has(functionName) ||
-        PRG_SAFE_USE_NAMES.has(functionName)
+        PRG_NATIVE_USE_FUNCTIONS.has(functionName) ||
+        PRG_SAFE_USE_FUNCTIONS.has(functionName)
       )
         funcNode.addEffect(Effect.PrgUse);
-      else if (PRG_INIT_NAMES.has(functionName))
+      else if (PRG_INIT_FUNCTIONS.has(functionName))
         funcNode.addEffect(Effect.PrgSeedInit);
     }
     if (isSendCall(expr)) funcNode.addEffect(Effect.Send);
@@ -518,35 +521,16 @@ function isContractStateWrite(stmt: AstStatement): boolean {
   ) {
     return isSelfAccess(stmt.path);
   }
-
-  // https://docs.tact-lang.org/book/maps/
-  const MAP_MUTATING_OPERATIONS = new Set<string>(["set", "del", "replace"]);
-  // For slices, cells, builders:
-  // https://github.com/tact-lang/tact/blob/08133e8418f3c6dcb49229b45cfeb7dd261bbe1f/stdlib/std/cells.tact#L75
-  const CELL_MUTATING_OPERATIONS = new Set<string>([
-    "loadRef",
-    "loadBits",
-    "loadInt",
-    "loadUint",
-    "loadBool",
-    "loadBit",
-    "loadCoins",
-    "loadAddress",
-    "skipBits",
-  ]);
-  // Strings:
-  // https://github.com/tact-lang/tact/blob/08133e8418f3c6dcb49229b45cfeb7dd261bbe1f/stdlib/std/text.tact#L18
-  const STRING_MUTATING_OPERATIONS = new Set<string>(["append"]);
   return (
     null !==
     findInExpressions(
       stmt,
       (expr) =>
         expr.kind === "method_call" &&
         isSelf(expr.self) &&
-        (MAP_MUTATING_OPERATIONS.has(idText(expr.method)) ||
-          STRING_MUTATING_OPERATIONS.has(idText(expr.method)) ||
-          CELL_MUTATING_OPERATIONS.has(idText(expr.method))),
+        (MAP_MUTATING_METHODS.has(idText(expr.method)) ||
+          STRING_MUTATING_METHODS.has(idText(expr.method)) ||
+          BUILDER_MUTATING_METHODS.has(idText(expr.method))),
     )
   );
 }

src/internals/tact/stdlib.ts

@@ -1,8 +1,72 @@
+/**
+ * This module contains definitions from the Tact stdlib  and logic for
+ * accessing the stdlib in Misti.
+ *
+ * It should be reviewed before each Tact update to determine if updates are needed.
+ *
+ * @packageDocumentation
+ */
+
 import { MistiContext } from "../context";
 import { hasSubdirs } from "../util";
 import { SrcInfo } from "@tact-lang/compiler/dist/grammar/ast";
 import path from "path";
 
+/**
+ * Stdlib functions that access datetime functions.
+ */
+export const DATETIME_FUNCTIONS = new Set(["now", "timestamp"]);
+
+/**
+ * Stdlib functions that initialize PRG seed.
+ */
+export const PRG_INIT_FUNCTIONS = new Set([
+  "nativePrepareRandom",
+  "nativeRandomize",
+  "nativeRandomizeLt",
+]);
+
+/**
+ * Native stdlib functions that use PRG.
+ */
+export const PRG_NATIVE_USE_FUNCTIONS = new Set([
+  "nativeRandom",
+  "nativeRandomInterval",
+]);
+
+/**
+ * Safe Tact wrapper functions that use PRG.
+ */
+export const PRG_SAFE_USE_FUNCTIONS = new Set(["random", "randomInt"]);
+
+/**
+ * Map methods that mutate state.
+ * See: https://docs.tact-lang.org/book/maps/
+ */
+export const MAP_MUTATING_METHODS = new Set<string>(["set", "del", "replace"]);
+
+/**
+ * Builder methods mutating state.
+ * https://github.com/tact-lang/tact/blob/08133e8418f3c6dcb49229b45cfeb7dd261bbe1f/stdlib/std/cells.tact#L75
+ */
+export const BUILDER_MUTATING_METHODS = new Set<string>([
+  "storeRef",
+  "storeBits",
+  "storeInt",
+  "storeUint",
+  "storeBool",
+  "storeBit",
+  "storeCoins",
+  "storeAddress",
+  "skipBits",
+]);
+
+/**
+ * String mutating methods.
+ * https://github.com/tact-lang/tact/blob/08133e8418f3c6dcb49229b45cfeb7dd261bbe1f/stdlib/std/text.tact#L18
+ */
+export const STRING_MUTATING_METHODS = new Set<string>(["append"]);
+
 /**
  * A mandatory part of the file path to stdlib if using the default path.
  */

src/internals/tact/util.ts

@@ -1,6 +1,7 @@
 import { forEachExpression } from "./iterators";
 import { evalToType } from "../../internals/tact/";
 import { unreachable } from "../util";
+import { MAP_MUTATING_METHODS, BUILDER_MUTATING_METHODS } from "./stdlib";
 import { AstComparator } from "@tact-lang/compiler/dist/";
 import {
   AstExpression,
@@ -23,25 +24,6 @@ import { prettyPrint } from "@tact-lang/compiler/dist/prettyPrinter";
 import { Interval as RawInterval } from "ohm-js";
 import * as path from "path";
 
-/** Stdlib functions that access datetime functions. */
-export const DATETIME_NAMES = new Set(["now", "timestamp"]);
-
-/** Stdlib functions that initialize PRG seed. */
-export const PRG_INIT_NAMES = new Set([
-  "nativePrepareRandom",
-  "nativeRandomize",
-  "nativeRandomizeLt",
-]);
-
-/** Native stdlib functions that use PRG. */
-export const PRG_NATIVE_USE_NAMES = new Set([
-  "nativeRandom",
-  "nativeRandomInterval",
-]);
-
-/** Safe Tact wrapper functions that use PRG. */
-export const PRG_SAFE_USE_NAMES = new Set(["random", "randomInt"]);
-
 /**
  * Creates a concise string representation of `SrcInfo`.
  */
@@ -104,28 +86,13 @@ export function isSelfAccess(expr: AstExpression): boolean {
  * @returns True iff `call` is a stdlib method mutating its receiver.
  */
 export function isStdlibMutationMethod(call: AstMethodCall): boolean {
-  // https://docs.tact-lang.org/book/maps
-  const mapMutationOperations = ["set", "del"];
-  // See: stdlib/std/cells.tact
-  const builderMutationOperations = [
-    "storeInt",
-    "storeUint",
-    "storeBool",
-    "storeBit",
-    "storeCoins",
-    "storeRef",
-    "storeSlice",
-    "storeBuilder",
-    "storeAddress",
-    "storeMaybeRef",
-  ];
   const methodName = idText(call.method);
   return (
     // Filter out contract calls e.g.: `self.set(/*...*/)`
     !isSelf(call.self) &&
     // TODO: This should be rewritten when we have types in AST
-    (mapMutationOperations.includes(methodName) ||
-      builderMutationOperations.includes(methodName))
+    (MAP_MUTATING_METHODS.has(methodName) ||
+      BUILDER_MUTATING_METHODS.has(methodName))
   );
 }
 

test/all/syntax.expected.callgraph.dot

@@ -17,37 +17,40 @@ digraph "CallGraph" {
 [Send,StateRead]"];
     node_28 [label="fun funcWithStateRead()
 [StateRead]"];
-    node_29 [label="fun funcWithStateWrite()
+    node_29 [label="fun funcWithStateWrite1()
 [StateWrite]"];
-    node_30 [label="fun funcWithMultipleEffects()
+    node_30 [label="fun funcWithStateWrite2(s: Slice): Address"];
+    node_31 [label="fun funcWithMultipleEffects()
 [AccessDatetime,PrgUse,PrgSeedInit]"];
-    node_31 [label="receive()"];
-    node_32 [label="dump"];
-    node_33 [label="emptyMap"];
-    node_34 [label="m::set"];
-    node_35 [label="s::loadRefEx"];
-    node_36 [label="getA"];
-    node_37 [label="sender"];
-    node_38 [label="newAddress"];
-    node_39 [label="now"];
-    node_40 [label="nativeRandomizeLt"];
-    node_41 [label="beginString"];
-    node_42 [label="a::append"];
-    node_17 -> node_32;
-    node_18 -> node_33;
-    node_19 -> node_32;
-    node_20 -> node_33;
+    node_32 [label="receive()"];
+    node_33 [label="dump"];
+    node_34 [label="emptyMap"];
+    node_35 [label="m::set"];
+    node_36 [label="s::loadRefEx"];
+    node_37 [label="getA"];
+    node_38 [label="sender"];
+    node_39 [label="newAddress"];
+    node_40 [label="s::loadAddress"];
+    node_41 [label="now"];
+    node_42 [label="nativeRandomizeLt"];
+    node_43 [label="beginString"];
+    node_44 [label="a::append"];
+    node_17 -> node_33;
+    node_18 -> node_34;
+    node_19 -> node_33;
     node_20 -> node_34;
-    node_20 -> node_34;
-    node_20 -> node_34;
-    node_21 -> node_35;
-    node_24 -> node_36;
-    node_26 -> node_37;
+    node_20 -> node_35;
+    node_20 -> node_35;
+    node_20 -> node_35;
+    node_21 -> node_36;
+    node_24 -> node_37;
+    node_26 -> node_38;
     node_27 -> node_15;
-    node_29 -> node_38;
-    node_30 -> node_39;
-    node_30 -> node_12;
+    node_29 -> node_39;
     node_30 -> node_40;
     node_31 -> node_41;
+    node_31 -> node_12;
     node_31 -> node_42;
+    node_32 -> node_43;
+    node_32 -> node_44;
 }