diff --git a/CHANGELOG.md b/CHANGELOG.md index ffe70e1..12d68d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ Check [Keep a Changelog](http://keepachangelog.com/) for recommendations on how ## [Unreleased] +## [0.0.3] - 2024-09-07 + +- Learned Go's `fallthrough` keyword +- Learned an option to draw flat-switches (where all cases are direct descendants of the switch head) +- Added utilities for basic reachability testing + ## [0.0.2] - 2024-09-06 - Interactive demo website, use `bun demo` to run. diff --git a/package.json b/package.json index 6ba799f..3e64318 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,7 @@ "//": "START EXTENSION ATTRIBUTES", "publisher": "tamir-bahar", "name": "function-graph-overview", - "version": "0.0.2", + "version": "0.0.3", "description": "Function Graph Overview", "displayName": "Function Graph Overview", "icon": "./media/icon.png", @@ -91,4 +91,4 @@ "engines": { "vscode": "^1.86.0" } -} \ No newline at end of file +} diff --git a/src/control-flow/cfg.ts b/src/control-flow/cfg.ts index 54df243..b6f1b79 100644 --- a/src/control-flow/cfg.ts +++ b/src/control-flow/cfg.ts @@ -4,6 +4,7 @@ import Parser from "web-tree-sitter"; type Node = Parser.SyntaxNode; type NodeType = + | "MARKER_COMMENT" | "LOOP_HEAD" | "LOOP_EXIT" | "SELECT" @@ -35,6 +36,7 @@ interface GraphNode { type: NodeType; code: string; lines: number; + markers: string[]; } interface GraphEdge { @@ -57,15 +59,6 @@ interface BasicBlock { gotos?: Goto[]; } -interface SwitchlikeProps { - nodeType: NodeType; - mergeType: NodeType; - mergeCode: string; - caseName: string; - caseFieldName: string; - caseTypeName: NodeType; -} - export interface CFG { graph: MultiDirectedGraph; entry: string; @@ -122,16 +115,35 @@ export function mergeNodeAttrs(from: GraphNode, into: GraphNode): GraphNode { lines: from.lines + into.lines, }; } +interface Case { + conditionEntry: string | null; + conditionExit: string | null; + consequenceEntry: string | null; + consequenceExit: string | null; + alternativeExit: string; + hasFallthrough: boolean; + isDefault: boolean; +} + +interface BuilderOptions { + flatSwitch?: boolean; + markerPattern?: RegExp; +} export class CFGBuilder { private graph: MultiDirectedGraph; private entry: string; private nodeId: number; + private readonly flatSwitch: boolean; + private readonly markerPattern: RegExp | null; - constructor() { + constructor(options?: BuilderOptions) { this.graph = new MultiDirectedGraph(); this.nodeId = 0; this.entry = null; + + this.flatSwitch = options?.flatSwitch ?? false; + this.markerPattern = options?.markerPattern ?? null; } public buildCFG(functionNode: Node): CFG { @@ -156,10 +168,14 @@ export class CFGBuilder { private addNode(type: NodeType, code: string, lines: number = 1): string { const id = `node${this.nodeId++}`; - this.graph.addNode(id, { type, code, lines }); + this.graph.addNode(id, { type, code, lines, markers: [] }); return id; } + private addMarker(node: string, marker: string) { + this.graph.getNodeAttributes(node).markers.push(marker); + } + private addEdge( source: string, target: string, @@ -180,6 +196,38 @@ export class CFGBuilder { return child ? child.text : ""; } + private processStatements(statements: Node[]): BasicBlock { + const blockHandler = new BlockHandler(); + + // Ignore comments + const codeStatements = statements.filter((syntax) => { + if (syntax.type !== "comment") { + return true; + } + + return ( + this.markerPattern && Boolean(syntax.text.match(this.markerPattern)) + ); + }); + + if (codeStatements.length === 0) { + const emptyNode = this.addNode("EMPTY", "empty block"); + return { entry: emptyNode, exit: emptyNode }; + } + + let entry: string | null = null; + let previous: string | null = null; + for (const statement of codeStatements) { + const { entry: currentEntry, exit: currentExit } = blockHandler.update( + this.processBlock(statement), + ); + if (!entry) entry = currentEntry; + if (previous && currentEntry) this.addEdge(previous, currentEntry); + previous = currentExit; + } + return blockHandler.update({ entry, exit: previous }); + } + private processBlock(node: Node | null): BasicBlock { if (!node) return { entry: null, exit: null }; @@ -191,7 +239,9 @@ export class CFGBuilder { case "for_statement": return this.processForStatement(node); case "expression_switch_statement": - return this.processSwitchStatement(node); + case "type_switch_statement": + case "select_statement": + return this.processSwitchlike(node); case "return_statement": { const returnNode = this.addNode("RETURN", node.text); return { entry: returnNode, exit: null }; @@ -204,113 +254,141 @@ export class CFGBuilder { return this.processLabeledStatement(node); case "goto_statement": return this.processGotoStatement(node); - case "type_switch_statement": - return this.processTypeSwitchStatement(node); - case "select_statement": - return this.processSelectStatement(node); + case "comment": + return this.processComment(node); default: { const newNode = this.addNode("STATEMENT", node.text); return { entry: newNode, exit: newNode }; } } } + private processComment(commentSyntax: Parser.SyntaxNode): BasicBlock { + // We only ever ger here when marker comments are enabled, + // and only for marker comments as the rest are filtered out. + const commentNode = this.addNode("MARKER_COMMENT", commentSyntax.text); + if (this.markerPattern) { + const marker = commentSyntax.text.match(this.markerPattern)?.[1]; + if (marker) this.addMarker(commentNode, marker); + } + return { entry: commentNode, exit: commentNode }; + } - private processSwitchlike( - switchlikeSyntax: Parser.SyntaxNode, - props: SwitchlikeProps, - ): BasicBlock { - const { - nodeType, - mergeType, - mergeCode, - caseName, - caseTypeName, - caseFieldName, - } = props; + private buildSwitch( + cases: Case[], + mergeNode: string, + switchHeadNode: string, + ) { + let fallthrough: string | null = null; + let previous: string | null = null; + if (!this.flatSwitch && cases[0]?.conditionEntry) { + this.addEdge(switchHeadNode, cases[0].conditionEntry); + } + cases.forEach((thisCase) => { + if (this.flatSwitch) { + if (thisCase.consequenceEntry) { + this.addEdge(switchHeadNode, thisCase.consequenceEntry); + if (fallthrough) { + this.addEdge(fallthrough, thisCase.consequenceEntry); + } + } + } else { + if (fallthrough && thisCase.consequenceEntry) { + this.addEdge(fallthrough, thisCase.consequenceEntry); + } + if (previous && thisCase.conditionEntry) { + this.addEdge( + previous, + thisCase.conditionEntry, + "alternative" as EdgeType, + ); + } - const blockHandler = new BlockHandler(); - const valueNode = this.addNode( - nodeType, - this.getChildFieldText(switchlikeSyntax, "value"), - ); - const mergeNode = this.addNode(mergeType, mergeCode); + if (thisCase.consequenceEntry && thisCase.conditionExit) + this.addEdge( + thisCase.conditionExit, + thisCase.consequenceEntry, + "consequence", + ); - let previous = { node: valueNode, branchType: "regular" as EdgeType }; - switchlikeSyntax.namedChildren - .filter((child) => child.type === caseName) - .forEach((caseNode) => { - const caseType = this.getChildFieldText(caseNode, caseFieldName); - const caseConditionNode = this.addNode(caseTypeName, caseType); + // Update for next case + previous = thisCase.isDefault ? null : thisCase.alternativeExit; + } - const caseBlock = blockHandler.update( - this.processStatements(caseNode.namedChildren.slice(1)), + // Fallthrough is the same for both flat and non-flat layouts. + if (!thisCase.hasFallthrough && thisCase.consequenceExit) { + this.addEdge(thisCase.consequenceExit, mergeNode); + } + // Update for next case + fallthrough = thisCase.hasFallthrough ? thisCase.consequenceExit : null; + }); + // Connect the last node to the merge node. + // No need to handle `fallthrough` here as it is not allowed for the last case. + if (previous) { + this.addEdge(previous, mergeNode, "alternative"); + } + } + + private collectCases( + switchSyntax: Parser.SyntaxNode, + blockHandler: BlockHandler, + ): Case[] { + const cases: Case[] = []; + const caseTypes = [ + "default_case", + "communication_case", + "type_case", + "expression_case", + ]; + switchSyntax.namedChildren + .filter((child) => caseTypes.includes(child.type)) + .forEach((caseSyntax) => { + const isDefault = caseSyntax.type === "default_case"; + + const consequence = caseSyntax.namedChildren.slice(isDefault ? 0 : 1); + const hasFallthrough = consequence + .map((node) => node.type) + .includes("fallthrough_statement"); + + const conditionNode = this.addNode( + "CASE_CONDITION", + isDefault ? "default" : (caseSyntax.firstNamedChild?.text ?? ""), + ); + const consequenceNode = blockHandler.update( + this.processStatements(consequence), ); - if (caseBlock.entry) { - this.addEdge(caseConditionNode, caseBlock.entry, "consequence"); - } - if (caseBlock.exit) { - this.addEdge(caseBlock.exit, mergeNode); - } - this.addEdge(previous.node, caseConditionNode, previous.branchType); - previous = { - node: caseConditionNode, - branchType: "alternative", - }; + cases.push({ + conditionEntry: conditionNode, + conditionExit: conditionNode, + consequenceEntry: consequenceNode.entry, + consequenceExit: consequenceNode.exit, + alternativeExit: conditionNode, + hasFallthrough, + isDefault, + }); }); - const defaultCase = switchlikeSyntax.namedChildren.find( - (child) => child.type === "default_case", + return cases; + } + + private processSwitchlike(switchSyntax: Parser.SyntaxNode): BasicBlock { + const blockHandler = new BlockHandler(); + + const cases = this.collectCases(switchSyntax, blockHandler); + const headNode = this.addNode( + "SWITCH_CONDITION", + this.getChildFieldText(switchSyntax, "value"), ); - if (defaultCase !== undefined) { - const defaultBlock = blockHandler.update( - this.processStatements(defaultCase.namedChildren), - ); - this.addEdge(previous.node, defaultBlock.entry, previous.branchType); - this.addEdge(defaultBlock.entry, mergeNode); - } else { - this.addEdge(previous.node, mergeNode, previous.branchType); - } + const mergeNode: string = this.addNode("SWITCH_MERGE", ""); + this.buildSwitch(cases, mergeNode, headNode); blockHandler.forEachBreak((breakNode) => { this.addEdge(breakNode, mergeNode); }); - return blockHandler.update({ entry: valueNode, exit: mergeNode }); + return blockHandler.update({ entry: headNode, exit: mergeNode }); } - private processSelectStatement(selectSyntax: Parser.SyntaxNode): BasicBlock { - return this.processSwitchlike(selectSyntax, { - caseFieldName: "communication", - caseName: "communication_case", - caseTypeName: "COMMUNICATION_CASE", - mergeCode: "MERGE", - mergeType: "SELECT_MERGE", - nodeType: "SELECT", - }); - } - private processTypeSwitchStatement( - switchSyntax: Parser.SyntaxNode, - ): BasicBlock { - return this.processSwitchlike(switchSyntax, { - caseFieldName: "value", - caseName: "type_case", - caseTypeName: "TYPE_CASE", - mergeCode: "MERGE", - mergeType: "TYPE_SWITCH_MERGE", - nodeType: "TYPE_SWITCH_VALUE", - }); - } - private processSwitchStatement(switchSyntax: Node): BasicBlock { - return this.processSwitchlike(switchSyntax, { - caseFieldName: "value", - caseName: "expression_case", - caseTypeName: "CASE_CONDITION", - mergeCode: "MERGE", - mergeType: "SWITCH_MERGE", - nodeType: "SWITCH_CONDITION", - }); - } private processGotoStatement(gotoSyntax: Parser.SyntaxNode): BasicBlock { const name = gotoSyntax.firstNamedChild.text; const gotoNode = this.addNode("GOTO", name); @@ -345,32 +423,6 @@ export class CFGBuilder { return { entry: breakNode, exit: null, breaks: [breakNode] }; } - private processStatements(statements: Node[]): BasicBlock { - const blockHandler = new BlockHandler(); - - // Ignore comments - const codeStatements = statements.filter( - (syntax) => syntax.type !== "comment", - ); - - if (codeStatements.length === 0) { - const emptyNode = this.addNode("EMPTY", "empty block"); - return { entry: emptyNode, exit: emptyNode }; - } - - let entry: string | null = null; - let previous: string | null = null; - for (const statement of codeStatements) { - const { entry: currentEntry, exit: currentExit } = blockHandler.update( - this.processBlock(statement), - ); - if (!entry) entry = currentEntry; - if (previous && currentEntry) this.addEdge(previous, currentEntry); - previous = currentExit; - } - return blockHandler.update({ entry, exit: previous }); - } - private processIfStatement( ifNode: Node, mergeNode: string | null = null, diff --git a/src/frontend/src/assets/samples/aaSwitchTesting.go b/src/frontend/src/assets/samples/aaSwitchTesting.go new file mode 100644 index 0000000..92b7a40 --- /dev/null +++ b/src/frontend/src/assets/samples/aaSwitchTesting.go @@ -0,0 +1,10 @@ +func basicSwitchTest() { + switch x { + // Who cares? + case 1: + // CFG: a + fallthrough + case 2: + case 3: + } +} \ No newline at end of file diff --git a/src/frontend/src/assets/samples/typeSwitch.go b/src/frontend/src/assets/samples/typeSwitch.go new file mode 100644 index 0000000..3496fd8 --- /dev/null +++ b/src/frontend/src/assets/samples/typeSwitch.go @@ -0,0 +1,10 @@ +func typeSwitch(i interface{}) { + switch v := i.(type) { + case int: + fmt.Printf("Twice %v is %v\n", v, v*2) + case string: + fmt.Printf("%q is %v bytes long\n", v, len(v)) + default: + fmt.Printf("I don't know about type %T!\n", v) + } +} \ No newline at end of file diff --git a/src/frontend/src/lib/Graph.svelte b/src/frontend/src/lib/Graph.svelte index c973923..cea0751 100644 --- a/src/frontend/src/lib/Graph.svelte +++ b/src/frontend/src/lib/Graph.svelte @@ -15,6 +15,7 @@ export let verbose: boolean = false; export let simplify: boolean = true; export let trim: boolean = true; + export let flatSwitch: boolean = false; async function initialize() { parser = await initializeParser(); @@ -26,6 +27,7 @@ readonly simplify: boolean; readonly verbose: boolean; readonly trim: boolean; + readonly flatSwitch: boolean; } function renderCode(code: string, options: RenderOptions) { @@ -41,8 +43,8 @@ "
", ) .replaceAll(")", "
"); - - let builder = new CFGBuilder(); + console.log(functionNode.childForFieldName("name").text); + let builder = new CFGBuilder({ flatSwitch }); let cfg = builder.buildCFG(functionNode); if (!cfg) { return; @@ -70,7 +72,7 @@
{#await initialize() then} - {@html renderWrapper(code, { simplify, verbose, trim })} + {@html renderWrapper(code, { simplify, verbose, trim, flatSwitch })} {/await}
diff --git a/src/frontend/src/lib/SampleViewer.svelte b/src/frontend/src/lib/SampleViewer.svelte index 58b9cf8..e9d85e4 100644 --- a/src/frontend/src/lib/SampleViewer.svelte +++ b/src/frontend/src/lib/SampleViewer.svelte @@ -10,6 +10,7 @@ let simplify: boolean = true; let verbose: boolean = false; let trim: boolean = true; + let flatSwitch: boolean = false;
@@ -21,11 +22,14 @@ + + +
{#each Object.entries(goSamples) as [name, code] (name)}
{code}
- + {/each}
diff --git a/src/frontend/src/lib/SimpleGraph.svelte b/src/frontend/src/lib/SimpleGraph.svelte index 700f886..2b09eb5 100644 --- a/src/frontend/src/lib/SimpleGraph.svelte +++ b/src/frontend/src/lib/SimpleGraph.svelte @@ -42,7 +42,6 @@ "
", ) .replaceAll(")", "
"); - let builder = new CFGBuilder(); let cfg = builder.buildCFG(functionNode); if (!cfg) { diff --git a/src/test/nodecount.go b/src/test/nodecount.go index d149697..5e6b514 100644 --- a/src/test/nodecount.go +++ b/src/test/nodecount.go @@ -47,3 +47,30 @@ label: g() } } + +/* +nodes: 1, +reaches: [ + ["a", "b"] +] +*/ +func trivialReachability() { + // CFG: a + // CFG: b +} + +/* +reaches: [ + ["A", "B"] +] +*/ +func hasFallthrough() { + switch 1 { + case 1: + // CFG: A + fallthrough + case 2: + // CFG: B + "Include me!" + } +} diff --git a/src/test/nodecount.test.ts b/src/test/nodecount.test.ts index 3092597..676e763 100644 --- a/src/test/nodecount.test.ts +++ b/src/test/nodecount.test.ts @@ -4,6 +4,10 @@ import goSampleCode from "./nodecount.go" with { type: "text" }; import treeSitterGo from "../../parsers/tree-sitter-go.wasm?url"; import { CFGBuilder, type CFG } from "../control-flow/cfg"; import { simplifyCFG } from "../control-flow/graph-ops"; +import type { MultiDirectedGraph } from "graphology"; +import { bfsFromNode } from "graphology-traversal"; + +const markerPattern: RegExp = /CFG: (\w+)/; async function initializeParser() { await Parser.init(); @@ -17,7 +21,8 @@ const parser = await initializeParser(); const tree = parser.parse(goSampleCode); interface Requirements { - nodes: number; + nodes?: number; + reaches?: [string, string][]; } interface TestFunction { name: string; @@ -29,10 +34,8 @@ function parseComment(text: string): Requirements { const jsonContent = text .slice(2, -2) .trim() - .replaceAll(/^/gm, '"') - .replaceAll(/:/gm, '":') - .replaceAll(/$/gm, ",") - .replaceAll(/,$/gm, ""); + .replaceAll(/^(?=\w)/gm, '"') + .replaceAll(/:/gm, '":'); return JSON.parse(`{${jsonContent}}`); } @@ -81,15 +84,73 @@ function buildSimpleCFG(functionNode: Parser.SyntaxNode): CFG { return simplifyCFG(buildCFG(functionNode)); } +function buildMarkerCFG(functionNode: Parser.SyntaxNode): CFG { + const builder = new CFGBuilder({ markerPattern }); + return builder.buildCFG(functionNode); +} + +function pathExists( + graph: MultiDirectedGraph, + source: string, + target: string, +): boolean { + let foundTarget = false; + bfsFromNode(graph, source, (node) => { + foundTarget ||= node == target; + return foundTarget; + }); + return foundTarget; +} + +function getMarkerMap(cfg: CFG): Map { + const markerMap: Map = new Map(); + cfg.graph.forEachNode((node, { markers }) => { + markers?.forEach((marker) => markerMap.set(marker, node)); + }); + return markerMap; +} + const testFunctions = [...iterTestFunctions(tree)]; const testMap = new Map( testFunctions.map((testFunc) => [testFunc.name, testFunc]), ); const testNames = [...testMap.keys()]; -test.each(testNames)("Node count for %s", (name) => { + +function testsFor(reqName: string): string[] { + return testNames.filter((name) => { + const testFunc = testMap.get(name) as TestFunction; + return Object.hasOwn(testFunc.reqs, reqName); + }); +} + +test.each(testsFor("nodes"))("Node count for %s", (name) => { const testFunc = testMap.get(name) as TestFunction; expect(testFunc).toBeDefined(); - const cfg = buildSimpleCFG(testFunc.function); - expect(cfg.graph.order).toBe(testFunc.reqs.nodes); + if (testFunc.reqs.nodes) { + const cfg = buildSimpleCFG(testFunc.function); + expect(cfg.graph.order).toBe(testFunc.reqs.nodes); + } +}); + +test.each(testsFor("reaches"))("Reachability for %s", (name) => { + const testFunc = testMap.get(name) as TestFunction; + expect(testFunc).toBeDefined(); + + if (testFunc.reqs.reaches) { + const cfg = buildMarkerCFG(testFunc.function); + const markerMap = getMarkerMap(cfg); + const getNode = (marker: string) => { + const node = markerMap.get(marker); + if (node) { + return node; + } + throw new Error(`No node found for marker ${marker}`); + }; + testFunc.reqs.reaches.forEach(([source, target]) => + expect(pathExists(cfg.graph, getNode(source), getNode(target))).toBe( + true, + ), + ); + } });