From 82b03c49726bf3eee36b94f3020b7c1a66c113b1 Mon Sep 17 00:00:00 2001 From: Byakuren Hijiri Date: Fri, 26 Jul 2024 07:52:25 +0000 Subject: [PATCH] fix(neverAccessedVariables): False positive on contract constants These are not supported, so we have to remove the logic introduced here after #22 is implemented. --- .../builtin/neverAccessedVariables.ts | 11 ++- src/internals/ir.ts | 22 ++++- src/internals/tactIRBuilder.ts | 3 + test/contracts/never-accessed-4.cfg.json | 25 ++++++ test/contracts/never-accessed-4.expected.out | 1 + test/contracts/never-accessed-4.tact | 13 +++ test/contracts/readonly-1.cfg.json | 88 +++++++++---------- test/contracts/readonly-2.cfg.json | 32 +++---- test/contracts/unbound-loop-1.cfg.json | 36 ++++---- test/contracts/unbound-loop-2.cfg.json | 36 ++++---- test/contracts/unbound-loop-3.cfg.json | 58 ++++++------ test/contracts/unbound-loop-4.cfg.json | 58 ++++++------ test/contracts/zero-address.cfg.json | 6 +- 13 files changed, 226 insertions(+), 163 deletions(-) create mode 100644 test/contracts/never-accessed-4.cfg.json create mode 100644 test/contracts/never-accessed-4.expected.out create mode 100644 test/contracts/never-accessed-4.tact diff --git a/src/detectors/builtin/neverAccessedVariables.ts b/src/detectors/builtin/neverAccessedVariables.ts index 9b2bda2a..a7f67c0e 100644 --- a/src/detectors/builtin/neverAccessedVariables.ts +++ b/src/detectors/builtin/neverAccessedVariables.ts @@ -179,10 +179,13 @@ export class NeverAccessedVariables extends Detector { } collectDefinedConstants(cu: CompilationUnit): Set<[ConstantName, ASTRef]> { - return Array.from(cu.ast.getConstants()).reduce((acc, constant) => { - acc.add([constant.name, constant.ref]); - return acc; - }, new Set<[ConstantName, ASTRef]>()); + return Array.from(cu.ast.getConstants({ includeContract: false })).reduce( + (acc, constant) => { + acc.add([constant.name, constant.ref]); + return acc; + }, + new Set<[ConstantName, ASTRef]>(), + ); } /** diff --git a/src/internals/ir.ts b/src/internals/ir.ts index 6692e229..46cbb650 100644 --- a/src/internals/ir.ts +++ b/src/internals/ir.ts @@ -25,6 +25,7 @@ export class TactASTStore { /** * Constructs a TactASTStore with mappings to all major AST components. * @param stdlibIds Identifiers of AST elements defined in stdlib. + * @param contractConstants Identifiers of constants defined within contracts (XXX: remove after #22 is fixed) * @param programEntries Identifiers of AST elements defined on the top-level. * @param functions Functions and methods including user-defined and special methods. * @param constants Constants defined across the compilation unit. @@ -37,6 +38,7 @@ export class TactASTStore { */ constructor( private stdlibIds = new Set(), + private contractConstants = new Set(), private programEntries: Set, private functions: Map, private constants: Map, @@ -110,11 +112,27 @@ export class TactASTStore { * and contract constants. * @param params Additional parameters: * - includeStdlib: If true, includes constants defined in stdlib. + * - includeContract: If true, includes contstants defined within a contract. + * XXX: This won't compile since Tact 1.4.1, remove it after #22 is merged */ getConstants( - params: Partial<{ includeStdlib: boolean }> = {}, + params: Partial<{ includeStdlib: boolean; includeContract: boolean }> = {}, ): IterableIterator { - return this.getItems(this.constants, params); + const { includeContract = false } = params; + const constants = this.getItems(this.constants, params); + function* filterIterator( + iterator: IterableIterator, + condition: (item: ASTConstant) => boolean, + ): IterableIterator { + for (const item of iterator) { + if (condition(item)) { + yield item; + } + } + } + return includeContract + ? constants + : filterIterator(constants, (c) => !this.contractConstants.has(c.id)); } getContracts(): IterableIterator { diff --git a/src/internals/tactIRBuilder.ts b/src/internals/tactIRBuilder.ts index 6fa1a197..70cff75d 100644 --- a/src/internals/tactIRBuilder.ts +++ b/src/internals/tactIRBuilder.ts @@ -103,6 +103,7 @@ function hasSubdirs(filePath: string, subdirs: string[]): boolean { export class ASTMapper { private programEntries = new Set(); private stdlibIds = new Set(); + private contractConstants = new Set(); private functions = new Map< number, ASTFunction | ASTReceive | ASTInitFunction @@ -149,6 +150,7 @@ export class ASTMapper { public getASTStore(): TactASTStore { return new TactASTStore( this.stdlibIds, + this.contractConstants, this.programEntries, this.functions, this.constants, @@ -194,6 +196,7 @@ export class ASTMapper { break; case "def_constant": this.constants.set(decl.id, decl); + this.contractConstants.add(decl.id); break; default: throw new Error(`Unsupported contract declaration: ${decl}`); diff --git a/test/contracts/never-accessed-4.cfg.json b/test/contracts/never-accessed-4.cfg.json new file mode 100644 index 00000000..dc09198d --- /dev/null +++ b/test/contracts/never-accessed-4.cfg.json @@ -0,0 +1,25 @@ +{ + "projectName": "never-accessed-4", + "functions": [], + "contracts": [ + { + "name": "ConstantFieldTest", + "methods": [ + { + "name": "ConstantFieldTest.test", + "cfg": { + "nodes": [ + { + "id": 1566, + "stmtID": 21139, + "srcEdges": [], + "dstEdges": [] + } + ], + "edges": [] + } + } + ] + } + ] +} \ No newline at end of file diff --git a/test/contracts/never-accessed-4.expected.out b/test/contracts/never-accessed-4.expected.out new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/test/contracts/never-accessed-4.expected.out @@ -0,0 +1 @@ + diff --git a/test/contracts/never-accessed-4.tact b/test/contracts/never-accessed-4.tact new file mode 100644 index 00000000..f70d351a --- /dev/null +++ b/test/contracts/never-accessed-4.tact @@ -0,0 +1,13 @@ +// NOTE: This example won't compile after updating Tact to 1.4.1 +contract ConstantFieldTest { + const val: Int = ton("0.01"); + fun test() { + send(SendParameters{ + to: sender(), + bounce: true, + value: self.val, + mode: SendRemainingValue + SendIgnoreErrors + }); + } +} + diff --git a/test/contracts/readonly-1.cfg.json b/test/contracts/readonly-1.cfg.json index b218ff67..dcb90167 100644 --- a/test/contracts/readonly-1.cfg.json +++ b/test/contracts/readonly-1.cfg.json @@ -6,101 +6,101 @@ "cfg": { "nodes": [ { - "id": 1566, - "stmtID": 21125, + "id": 1717, + "stmtID": 23340, "srcEdges": [], "dstEdges": [ - 1568 + 1719 ] }, { - "id": 1567, - "stmtID": 21128, + "id": 1718, + "stmtID": 23343, "srcEdges": [ - 1568 + 1719 ], "dstEdges": [ - 1570 + 1721 ] }, { - "id": 1569, - "stmtID": 21131, + "id": 1720, + "stmtID": 23346, "srcEdges": [ - 1570 + 1721 ], "dstEdges": [ - 1572 + 1723 ] }, { - "id": 1571, - "stmtID": 21138, + "id": 1722, + "stmtID": 23353, "srcEdges": [ - 1572 + 1723 ], "dstEdges": [ - 1574, - 1578 + 1725, + 1729 ] }, { - "id": 1573, - "stmtID": 21137, + "id": 1724, + "stmtID": 23352, "srcEdges": [ - 1574 + 1725 ], "dstEdges": [ - 1576 + 1727 ] }, { - "id": 1575, - "stmtID": 21140, + "id": 1726, + "stmtID": 23355, "srcEdges": [ - 1576 + 1727 ], "dstEdges": [] }, { - "id": 1577, - "stmtID": 21140, + "id": 1728, + "stmtID": 23355, "srcEdges": [ - 1578 + 1729 ], "dstEdges": [] } ], "edges": [ { - "id": 1568, - "src": 1566, - "dst": 1567 + "id": 1719, + "src": 1717, + "dst": 1718 }, { - "id": 1570, - "src": 1567, - "dst": 1569 + "id": 1721, + "src": 1718, + "dst": 1720 }, { - "id": 1572, - "src": 1569, - "dst": 1571 + "id": 1723, + "src": 1720, + "dst": 1722 }, { - "id": 1574, - "src": 1571, - "dst": 1573 + "id": 1725, + "src": 1722, + "dst": 1724 }, { - "id": 1576, - "src": 1573, - "dst": 1575 + "id": 1727, + "src": 1724, + "dst": 1726 }, { - "id": 1578, - "src": 1571, - "dst": 1577 + "id": 1729, + "src": 1722, + "dst": 1728 } ] } diff --git a/test/contracts/readonly-2.cfg.json b/test/contracts/readonly-2.cfg.json index f096359b..87ebb28d 100644 --- a/test/contracts/readonly-2.cfg.json +++ b/test/contracts/readonly-2.cfg.json @@ -6,42 +6,42 @@ "cfg": { "nodes": [ { - "id": 1728, - "stmtID": 23209, + "id": 1879, + "stmtID": 25424, "srcEdges": [], "dstEdges": [ - 1730 + 1881 ] }, { - "id": 1729, - "stmtID": 23212, + "id": 1880, + "stmtID": 25427, "srcEdges": [ - 1730 + 1881 ], "dstEdges": [ - 1732 + 1883 ] }, { - "id": 1731, - "stmtID": 23214, + "id": 1882, + "stmtID": 25429, "srcEdges": [ - 1732 + 1883 ], "dstEdges": [] } ], "edges": [ { - "id": 1730, - "src": 1728, - "dst": 1729 + "id": 1881, + "src": 1879, + "dst": 1880 }, { - "id": 1732, - "src": 1729, - "dst": 1731 + "id": 1883, + "src": 1880, + "dst": 1882 } ] } diff --git a/test/contracts/unbound-loop-1.cfg.json b/test/contracts/unbound-loop-1.cfg.json index db6e0781..6cf0e27e 100644 --- a/test/contracts/unbound-loop-1.cfg.json +++ b/test/contracts/unbound-loop-1.cfg.json @@ -6,48 +6,48 @@ "cfg": { "nodes": [ { - "id": 1882, - "stmtID": 25284, + "id": 2033, + "stmtID": 27499, "srcEdges": [], "dstEdges": [ - 1884 + 2035 ] }, { - "id": 1883, - "stmtID": 25288, + "id": 2034, + "stmtID": 27503, "srcEdges": [ - 1884 + 2035 ], "dstEdges": [ - 1886 + 2037 ] }, { - "id": 1885, - "stmtID": 25290, + "id": 2036, + "stmtID": 27505, "srcEdges": [ - 1886 + 2037 ], "dstEdges": [] }, { - "id": 1887, - "stmtID": 25290, + "id": 2038, + "stmtID": 27505, "srcEdges": [], "dstEdges": [] } ], "edges": [ { - "id": 1884, - "src": 1882, - "dst": 1883 + "id": 2035, + "src": 2033, + "dst": 2034 }, { - "id": 1886, - "src": 1883, - "dst": 1885 + "id": 2037, + "src": 2034, + "dst": 2036 } ] } diff --git a/test/contracts/unbound-loop-2.cfg.json b/test/contracts/unbound-loop-2.cfg.json index f1e44848..6afc9dbb 100644 --- a/test/contracts/unbound-loop-2.cfg.json +++ b/test/contracts/unbound-loop-2.cfg.json @@ -6,48 +6,48 @@ "cfg": { "nodes": [ { - "id": 2037, - "stmtID": 27360, + "id": 2188, + "stmtID": 29575, "srcEdges": [], "dstEdges": [ - 2039 + 2190 ] }, { - "id": 2038, - "stmtID": 27364, + "id": 2189, + "stmtID": 29579, "srcEdges": [ - 2039 + 2190 ], "dstEdges": [ - 2041 + 2192 ] }, { - "id": 2040, - "stmtID": 27366, + "id": 2191, + "stmtID": 29581, "srcEdges": [ - 2041 + 2192 ], "dstEdges": [] }, { - "id": 2042, - "stmtID": 27366, + "id": 2193, + "stmtID": 29581, "srcEdges": [], "dstEdges": [] } ], "edges": [ { - "id": 2039, - "src": 2037, - "dst": 2038 + "id": 2190, + "src": 2188, + "dst": 2189 }, { - "id": 2041, - "src": 2038, - "dst": 2040 + "id": 2192, + "src": 2189, + "dst": 2191 } ] } diff --git a/test/contracts/unbound-loop-3.cfg.json b/test/contracts/unbound-loop-3.cfg.json index 11235726..3d3fdf7b 100644 --- a/test/contracts/unbound-loop-3.cfg.json +++ b/test/contracts/unbound-loop-3.cfg.json @@ -6,69 +6,69 @@ "cfg": { "nodes": [ { - "id": 2192, - "stmtID": 29439, + "id": 2343, + "stmtID": 31654, "srcEdges": [], "dstEdges": [ - 2194 + 2345 ] }, { - "id": 2193, - "stmtID": 29446, + "id": 2344, + "stmtID": 31661, "srcEdges": [ - 2194, - 2197 + 2345, + 2348 ], "dstEdges": [ - 2199 + 2350 ] }, { - "id": 2195, - "stmtID": 29445, + "id": 2346, + "stmtID": 31660, "srcEdges": [ - 2196 + 2347 ], "dstEdges": [ - 2197 + 2348 ] }, { - "id": 2198, - "stmtID": 29448, + "id": 2349, + "stmtID": 31663, "srcEdges": [ - 2199 + 2350 ], "dstEdges": [] }, { - "id": 2200, - "stmtID": 29448, + "id": 2351, + "stmtID": 31663, "srcEdges": [], "dstEdges": [] } ], "edges": [ { - "id": 2194, - "src": 2192, - "dst": 2193 + "id": 2345, + "src": 2343, + "dst": 2344 }, { - "id": 2196, - "src": 2193, - "dst": 2195 + "id": 2347, + "src": 2344, + "dst": 2346 }, { - "id": 2197, - "src": 2195, - "dst": 2193 + "id": 2348, + "src": 2346, + "dst": 2344 }, { - "id": 2199, - "src": 2193, - "dst": 2198 + "id": 2350, + "src": 2344, + "dst": 2349 } ] } diff --git a/test/contracts/unbound-loop-4.cfg.json b/test/contracts/unbound-loop-4.cfg.json index 03e2c1eb..25f51dff 100644 --- a/test/contracts/unbound-loop-4.cfg.json +++ b/test/contracts/unbound-loop-4.cfg.json @@ -13,69 +13,69 @@ "cfg": { "nodes": [ { - "id": 2351, - "stmtID": 31527, + "id": 2502, + "stmtID": 33742, "srcEdges": [], "dstEdges": [ - 2353 + 2504 ] }, { - "id": 2352, - "stmtID": 31534, + "id": 2503, + "stmtID": 33749, "srcEdges": [ - 2353, - 2356 + 2504, + 2507 ], "dstEdges": [ - 2358 + 2509 ] }, { - "id": 2354, - "stmtID": 31533, + "id": 2505, + "stmtID": 33748, "srcEdges": [ - 2355 + 2506 ], "dstEdges": [ - 2356 + 2507 ] }, { - "id": 2357, - "stmtID": 31536, + "id": 2508, + "stmtID": 33751, "srcEdges": [ - 2358 + 2509 ], "dstEdges": [] }, { - "id": 2359, - "stmtID": 31536, + "id": 2510, + "stmtID": 33751, "srcEdges": [], "dstEdges": [] } ], "edges": [ { - "id": 2353, - "src": 2351, - "dst": 2352 + "id": 2504, + "src": 2502, + "dst": 2503 }, { - "id": 2355, - "src": 2352, - "dst": 2354 + "id": 2506, + "src": 2503, + "dst": 2505 }, { - "id": 2356, - "src": 2354, - "dst": 2352 + "id": 2507, + "src": 2505, + "dst": 2503 }, { - "id": 2358, - "src": 2352, - "dst": 2357 + "id": 2509, + "src": 2503, + "dst": 2508 } ] } diff --git a/test/contracts/zero-address.cfg.json b/test/contracts/zero-address.cfg.json index f23ef26e..9aab4f63 100644 --- a/test/contracts/zero-address.cfg.json +++ b/test/contracts/zero-address.cfg.json @@ -6,12 +6,12 @@ "name": "SampleContract", "methods": [ { - "name": "SampleContract.init_33603", + "name": "SampleContract.init_35818", "cfg": { "nodes": [ { - "id": 2509, - "stmtID": 33601, + "id": 2660, + "stmtID": 35816, "srcEdges": [], "dstEdges": [] }