Skip to content

Commit 2ae271f

Browse files
dstallenbergdstallenberg
authored
fix: instrumentation and CFG generation (#149)
Both the instrumentation and the control flow graph had several mistakes related to missing traces or nodes. Additionally, the branch distance has been improved.
1 parent e252703 commit 2ae271f

22 files changed

+1954
-334
lines changed

‎libraries/analysis-javascript/lib/cfg/ControlFlowGraphVisitor.ts‎

Lines changed: 71 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -426,13 +426,15 @@ export class ControlFlowGraphVisitor extends AbstractSyntaxTreeVisitor {
426426
`Entering IfStatement at ${this._getNodeId(path)}`
427427
);
428428

429-
const branchNode = this._createNode(path); // or path.get("test") ??
430-
// TODO test
431-
429+
const branchNode = this._createNode(path);
432430
this._connectToParents(branchNode);
431+
this._currentParents = [branchNode.id];
432+
433+
const testNode = this._createNode(path.get("test"));
434+
this._connectToParents(testNode);
435+
this._currentParents = [testNode.id];
433436

434437
// consequent
435-
this._currentParents = [branchNode.id];
436438
this._edgeType = EdgeType.CONDITIONAL_TRUE;
437439
let sizeBefore = this._nodesList.length;
438440
path.get("consequent").visit();
@@ -446,7 +448,7 @@ export class ControlFlowGraphVisitor extends AbstractSyntaxTreeVisitor {
446448
const consequentNodes = this._currentParents;
447449

448450
// alternate
449-
this._currentParents = [branchNode.id];
451+
this._currentParents = [testNode.id];
450452
this._edgeType = EdgeType.CONDITIONAL_FALSE;
451453

452454
sizeBefore = this._nodesList.length;
@@ -481,6 +483,10 @@ export class ControlFlowGraphVisitor extends AbstractSyntaxTreeVisitor {
481483
`Entering DoWhileStatement at ${this._getNodeId(path)}`
482484
);
483485

486+
const doWhileNode = this._createNode(path);
487+
this._connectToParents(doWhileNode);
488+
this._currentParents = [doWhileNode.id];
489+
484490
this._breakNodesStack.push(new Set());
485491
this._continueNodesStack.push(new Set());
486492

@@ -499,7 +505,7 @@ export class ControlFlowGraphVisitor extends AbstractSyntaxTreeVisitor {
499505
}
500506

501507
// loop
502-
const loopNode = this._createNode(path); // or path.get("test") ??
508+
const loopNode = this._createNode(path.get("test")); // or path.get("test") ??
503509
// TODO test
504510

505511
this._connectToParents(loopNode);
@@ -547,11 +553,15 @@ export class ControlFlowGraphVisitor extends AbstractSyntaxTreeVisitor {
547553
`Entering WhileStatement at ${this._getNodeId(path)}`
548554
);
549555

556+
const whileNode = this._createNode(path);
557+
this._connectToParents(whileNode);
558+
this._currentParents = [whileNode.id];
559+
550560
this._breakNodesStack.push(new Set());
551561
this._continueNodesStack.push(new Set());
552562

553563
// loop
554-
const loopNode = this._createNode(path); // or path.get("test") ??
564+
const loopNode = this._createNode(path.get("test")); // or path.get("test") ??
555565
// TODO test
556566

557567
this._connectToParents(loopNode);
@@ -605,6 +615,10 @@ export class ControlFlowGraphVisitor extends AbstractSyntaxTreeVisitor {
605615
`Entering ForStatement at ${this._getNodeId(path)}`
606616
);
607617

618+
const forNode = this._createNode(path);
619+
this._connectToParents(forNode);
620+
this._currentParents = [forNode.id];
621+
608622
this._breakNodesStack.push(new Set());
609623
this._continueNodesStack.push(new Set());
610624

@@ -630,8 +644,7 @@ export class ControlFlowGraphVisitor extends AbstractSyntaxTreeVisitor {
630644
);
631645
}
632646

633-
// TODO test
634-
const testNode = this._createNode(path); // or path.get("test") ??
647+
const testNode = this._createNode(path.get("test"));
635648
this._connectToParents(testNode);
636649
this._currentParents = [testNode.id];
637650

@@ -701,6 +714,10 @@ export class ControlFlowGraphVisitor extends AbstractSyntaxTreeVisitor {
701714
`Entering ForInStatement at ${this._getNodeId(path)}`
702715
);
703716

717+
const forInNode = this._createNode(path);
718+
this._connectToParents(forInNode);
719+
this._currentParents = [forInNode.id];
720+
704721
this._breakNodesStack.push(new Set());
705722
this._continueNodesStack.push(new Set());
706723

@@ -720,8 +737,8 @@ export class ControlFlowGraphVisitor extends AbstractSyntaxTreeVisitor {
720737
// left
721738
path.get("left").visit();
722739

723-
// TODO test
724-
const testNode = this._createNode(path); // or path.get("test") ??
740+
// test does not exist so we create placeholder?
741+
const testNode = this._createPlaceholderNode(path.get("left")); // stupid hack but we cannot have the placeholder twice
725742
this._connectToParents(testNode);
726743
this._currentParents = [testNode.id];
727744

@@ -776,6 +793,10 @@ export class ControlFlowGraphVisitor extends AbstractSyntaxTreeVisitor {
776793
`Entering ForOfStatement at ${this._getNodeId(path)}`
777794
);
778795

796+
const forOfNode = this._createNode(path);
797+
this._connectToParents(forOfNode);
798+
this._currentParents = [forOfNode.id];
799+
779800
this._breakNodesStack.push(new Set());
780801
this._continueNodesStack.push(new Set());
781802

@@ -795,8 +816,8 @@ export class ControlFlowGraphVisitor extends AbstractSyntaxTreeVisitor {
795816
// left
796817
path.get("left").visit();
797818

798-
// TODO test
799-
const testNode = this._createNode(path); // or path.get("test") ??
819+
// test does not exist so we create placeholder?
820+
const testNode = this._createPlaceholderNode(path.get("left")); // stupid hack but we cannot have the placeholder twice
800821
this._connectToParents(testNode);
801822
this._currentParents = [testNode.id];
802823

@@ -853,42 +874,54 @@ export class ControlFlowGraphVisitor extends AbstractSyntaxTreeVisitor {
853874

854875
this._breakNodesStack.push(new Set());
855876

856-
// TODO test
857-
const testNode = this._createNode(path); // or path.get("test") ??
877+
const switchNode = this._createNode(path);
878+
this._connectToParents(switchNode);
879+
this._currentParents = [switchNode.id];
880+
881+
const testNode = this._createNode(path.get("discriminant"));
858882
this._connectToParents(testNode);
859883
this._currentParents = [testNode.id];
860884

861885
for (const caseNode of path.get("cases")) {
862-
// TODO test
863-
const caseTestNode = this._createNode(caseNode); // or path.get("test") ??
864-
this._connectToParents(caseTestNode);
865-
this._currentParents = [caseTestNode.id];
866-
867886
if (caseNode.has("test")) {
868-
// case
869-
this._edgeType = EdgeType.CONDITIONAL_TRUE;
870-
}
887+
// test
888+
const caseTestNode = this._createNode(caseNode.get("test"));
889+
this._connectToParents(caseTestNode);
890+
this._currentParents = [caseTestNode.id];
871891

872-
// consequent
873-
if (caseNode.get("consequent").length === 0) {
874-
// empty body
875-
// create placeholder node
876-
const placeholderNode = this._createPlaceholderNode(caseNode);
877-
this._connectToParents(placeholderNode);
878-
this._currentParents = [placeholderNode.id];
879-
} else {
880-
for (const consequentNode of caseNode.get("consequent")) {
881-
consequentNode.visit();
892+
// consequent
893+
this._edgeType = EdgeType.CONDITIONAL_TRUE;
894+
const consequentNode = this._createNode(caseNode);
895+
this._connectToParents(consequentNode);
896+
this._currentParents = [consequentNode.id];
897+
898+
if (caseNode.get("consequent").length > 0) {
899+
for (const consequentNode of caseNode.get("consequent")) {
900+
consequentNode.visit();
901+
}
882902
}
883-
}
884903

885-
if (caseNode.has("test")) {
886-
// case
904+
const trueParents = this._currentParents; // if there is a break these should be empty
905+
// alternate
906+
// placeholder
887907
this._edgeType = EdgeType.CONDITIONAL_FALSE;
888-
this._currentParents = [caseTestNode.id, ...this._currentParents];
908+
this._currentParents = [caseTestNode.id];
909+
const alternateNode = this._createPlaceholderNode(caseNode);
910+
this._connectToParents(alternateNode);
911+
this._currentParents = [alternateNode.id, ...trueParents]; // normal + fall through case
889912
} else {
890913
// default
891-
this._currentParents = [...this._currentParents];
914+
if (caseNode.get("consequent").length === 0) {
915+
// empty body
916+
// create placeholder node
917+
const placeholderNode = this._createPlaceholderNode(caseNode);
918+
this._connectToParents(placeholderNode);
919+
this._currentParents = [placeholderNode.id];
920+
} else {
921+
for (const consequentNode of caseNode.get("consequent")) {
922+
consequentNode.visit();
923+
}
924+
}
892925
}
893926
}
894927

‎libraries/analysis-javascript/lib/type/resolving/TypeModel.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ export class TypeModel {
5555
this._scoreHasChangedMap = new Map();
5656

5757
this._objectTypeDescription = new Map();
58+
59+
this.addId("anon"); // should be removed at some point
5860
}
5961

6062
getObjectDescription(element: string): ObjectType {
@@ -343,9 +345,17 @@ export class TypeModel {
343345

344346
const probabilityMap = new Map<string, number>();
345347

348+
if (id === "anon") {
349+
return probabilityMap;
350+
}
351+
346352
const typeScoreMap = this._elementTypeScoreMap.get(id);
347353
const relationMap = this._relationScoreMap.get(id);
348354

355+
if (typeScoreMap === undefined) {
356+
throw new Error(`Cannot get typescoreMap of ${id}`);
357+
}
358+
349359
if (!relationPairsVisited) {
350360
relationPairsVisited = new Map();
351361
// this._scoreHasChangedMap.set(element, false);

‎libraries/instrumentation-javascript/lib/instrumentation/VisitState.ts‎

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -358,15 +358,14 @@ export class VisitState {
358358
}
359359

360360
getBranchMetaTracker(
361-
branchName,
362-
testAsAst,
361+
branchName: string,
363362
testAsCode: string,
364363
variables: string[]
365364
) {
366365
const T = this.types;
367366

368367
const metaTracker = T.callExpression(T.identifier(this.metaVarName), [
369-
T.numericLiteral(branchName),
368+
T.stringLiteral(`${branchName}`),
370369
T.objectExpression([
371370
T.objectProperty(
372371
T.stringLiteral("condition_ast"),
@@ -380,23 +379,12 @@ export class VisitState {
380379
T.stringLiteral("variables"),
381380
T.ObjectExpression([
382381
...variables
383-
.filter((v, i, a) => a.indexOf(v) === i)
384-
.map((v) => {
385-
if (v.includes(".")) {
386-
const split = v.split(".");
387-
388-
return T.objectProperty(
389-
T.stringLiteral(v),
390-
T.optionalMemberExpression(
391-
T.identifier(split[0]),
392-
T.identifier(split[1]),
393-
false,
394-
true
395-
)
396-
);
397-
} else {
398-
return T.objectProperty(T.stringLiteral(v), T.identifier(v));
399-
}
382+
.filter((v, i, a) => a.indexOf(v) === i) // remove duplicates
383+
.map(([source, identifier]) => {
384+
return T.objectProperty(
385+
T.stringLiteral(source),
386+
T.identifier(identifier)
387+
);
400388
}),
401389
])
402390
),

0 commit comments

Comments
 (0)