Added support for localising variables through which correlated reads

flow. This somewhat compensates for the lack of SSA form for closure
variables under the new lexical scheme.

git-svn-id: https://wala.svn.sourceforge.net/svnroot/wala/trunk@4505 f5eafffb-2e1d-0410-98e4-8ec43c5233c4
This commit is contained in:
msridhar1 2012-02-17 20:25:22 +00:00
parent 5106842f30
commit a3d0b45eef
10 changed files with 241 additions and 29 deletions

View File

@ -227,6 +227,7 @@ public abstract class TestCorrelatedPairExtraction {
}
// another example with "this"
// fails since variables from enclosing functions are no longer in SSA form, hence no correlation is found
@Test
public void test9() {
testRewriter("function defglobals(globals) {\n" +
@ -413,7 +414,7 @@ public abstract class TestCorrelatedPairExtraction {
}
// fails since the assignment to "value" in the extracted version gets a (spurious) reference error CFG edge
@Test @Ignore
@Test
public void test18() {
testRewriter("function addMethods(source) {\n" +
" var properties = Object.keys(source);\n" +
@ -426,13 +427,55 @@ public abstract class TestCorrelatedPairExtraction {
"function addMethods(source) {\n" +
" var properties = Object.keys(source);\n" +
" for (var i = 0, length = properties.length; i < length; i++) {\n" +
" var property = properties[i], value; (function _forin_body_0(property, thi$) { value = source[property];\n" +
" thi$.prototype[property] = value; })(property, this);\n" +
" var property = properties[i], value; value = (function _forin_body_0(property, thi$) { var value; value = source[property];\n" +
" thi$.prototype[property] = value; return value; })(property, this);\n" +
" }\n" +
" return this;\n" +
"}");
}
// slight variation of test18
@Test
public void test18_b() {
testRewriter("function addMethods(source) {\n" +
" var properties = Object.keys(source);\n" +
" for (var i = 0, length = properties.length; i < length; i++) {\n" +
" var property = properties[i], foo = 23, value = source[property];\n" +
" this.prototype[property] = value;\n" +
" }\n" +
" return this;\n" +
"}",
"function addMethods(source) {\n" +
" var properties = Object.keys(source);\n" +
" for (var i = 0, length = properties.length; i < length; i++) {\n" +
" var property = properties[i], foo = 23, value; value = (function _forin_body_0(property, thi$) { var value; value = source[property];\n" +
" thi$.prototype[property] = value; return value; })(property, this);\n" +
" }\n" +
" return this;\n" +
"}");
}
// fails since the assignment to "value" in the extracted version gets a (spurious) reference error CFG edge
@Test
public void test18_c() {
testRewriter("function addMethods(source) {\n" +
" var properties = Object.keys(source);\n" +
" for (var i = 0, length = properties.length; i < length; i++) {\n" +
" var property = properties[i], foo = 23, value = source[property], bar = 42;\n" +
" this.prototype[property] = value;\n" +
" }\n" +
" return this;\n" +
"}",
"function addMethods(source) {\n" +
" var properties = Object.keys(source);\n" +
" for (var i = 0, length = properties.length; i < length; i++) {\n" +
" var property = properties[i], foo = 23, value, bar; value = (function _forin_body_0(property, thi$) { var value; value = source[property], bar = 42;\n" +
" thi$.prototype[property] = value; return value; })(property, this);\n" +
" }\n" +
" return this;\n" +
"}");
}
@Test
public void test19() {
testRewriter("function extend(dest, src) {\n" +
@ -447,6 +490,7 @@ public abstract class TestCorrelatedPairExtraction {
"function write(p, v) { this[p] = v; }");
}
// fails due to a missing LOCAL_SCOPE node
@Test
public void test20() {
testRewriter("function every(object, fn, bind) {\n" +
@ -502,4 +546,27 @@ public abstract class TestCorrelatedPairExtraction {
" return results;\n" +
"}");
}
// variant of test1
@Test
public void test23() {
testRewriter("function extend(dest, src) {\n" +
" var s;\n" +
" for(var p in src) {\n" +
" s = src[p];\n" +
" dest[p] = s;\n" +
" }\n" +
"}",
"function extend(dest, src) {\n" +
" var s;\n" +
" for(var p in src) {\n" +
" s = (function _forin_body_0(p) {\n" +
" var s;" +
" s = src[p];\n" +
" dest[p] = s;\n" +
" return s;" +
" })(p);\n" +
" }\n" +
"}");
}
}

View File

@ -11,26 +11,38 @@
package com.ibm.wala.cast.js.ipa.callgraph.correlations;
import java.util.HashSet;
import java.util.Set;
import com.ibm.wala.cast.tree.CAstSourcePositionMap.Position;
/**
* A correlation exists between a dynamic property read r and a dynamic property write w such that
* the value read in r may flow into w, and r and w are guaranteed to access a property of the same name.
*
* We additionally track the set of local variables the value read in r may flow through before reaching
* w. These will be candidates for localisation when extracting the correlation into a closure.
*
* @author mschaefer
*
*/
public abstract class Correlation {
public abstract class Correlation {
private final String indexName;
private final Set<String> flownThroughLocals;
protected Correlation(String indexName) {
protected Correlation(String indexName, Set<String> flownThroughLocals) {
this.indexName = indexName;
this.flownThroughLocals = new HashSet<String>(flownThroughLocals);
}
public String getIndexName() {
return indexName;
}
public Set<String> getFlownThroughLocals() {
return flownThroughLocals;
}
public abstract Position getStartPosition(SSASourcePositionMap positions);
public abstract Position getEndPosition(SSASourcePositionMap positions);
public abstract String pp(SSASourcePositionMap positions);

View File

@ -18,6 +18,7 @@ import java.net.URL;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
@ -57,6 +58,8 @@ import com.ibm.wala.util.collections.Iterator2Iterable;
import com.ibm.wala.util.collections.ObjectArrayMapping;
import com.ibm.wala.util.collections.Pair;
import com.ibm.wala.util.intset.BitVectorIntSet;
import com.ibm.wala.util.intset.IntIterator;
import com.ibm.wala.util.intset.IntSet;
import com.ibm.wala.util.intset.MutableIntSet;
import com.ibm.wala.util.intset.OrdinalSetMapping;
import com.ibm.wala.util.io.FileProvider;
@ -138,7 +141,7 @@ public class CorrelationFinder {
if(TRACK_ESCAPES) {
for(int j=0;j<inst2.getNumberOfUses();++j) {
if(inst2.getUse(j) == index) {
summary.addCorrelation(new EscapeCorrelation(get, (SSAAbstractInvokeInstruction)inst2, indexName));
summary.addCorrelation(new EscapeCorrelation(get, (SSAAbstractInvokeInstruction)inst2, indexName, getSourceLevelNames(astMethod, reached)));
break;
}
}
@ -149,7 +152,7 @@ public class CorrelationFinder {
// now find property writes with the same index whose RHS is in 'reached'
for(AbstractReflectivePut put : puts)
if(put.getMemberRef() == index && reached.contains(put.getValue()))
summary.addCorrelation(new ReadWriteCorrelation(get, put, indexName));
summary.addCorrelation(new ReadWriteCorrelation(get, put, indexName, getSourceLevelNames(astMethod, reached)));
}
return summary;
@ -169,6 +172,16 @@ public class CorrelationFinder {
}
return indexName;
}
private Set<String> getSourceLevelNames(AstMethod astMethod, IntSet vs) {
Set<String> res = new HashSet<String>();
for(IntIterator iter=vs.intIterator();iter.hasNext();) {
String name = getSourceLevelName(astMethod, iter.next());
if(name != null)
res.add(name);
}
return res;
}
// checks whether the given SSA variable must always be assigned a numeric value
private boolean mustBeNumeric(IR ir, DefUse du, int v) {

View File

@ -11,6 +11,8 @@
package com.ibm.wala.cast.js.ipa.callgraph.correlations;
import java.util.Set;
import com.ibm.wala.cast.ir.ssa.AbstractReflectiveGet;
import com.ibm.wala.cast.tree.CAstSourcePositionMap.Position;
import com.ibm.wala.ssa.SSAAbstractInvokeInstruction;
@ -28,8 +30,9 @@ public class EscapeCorrelation extends Correlation {
private final AbstractReflectiveGet get;
private final SSAAbstractInvokeInstruction invoke;
public EscapeCorrelation(AbstractReflectiveGet get, SSAAbstractInvokeInstruction invoke, String indexName) {
super(indexName);
public EscapeCorrelation(AbstractReflectiveGet get, SSAAbstractInvokeInstruction invoke,
String indexName, Set<String> flownThroughLocals) {
super(indexName, flownThroughLocals);
this.get = get;
this.invoke = invoke;
}

View File

@ -11,6 +11,8 @@
package com.ibm.wala.cast.js.ipa.callgraph.correlations;
import java.util.Set;
import com.ibm.wala.cast.ir.ssa.AbstractReflectiveGet;
import com.ibm.wala.cast.ir.ssa.AbstractReflectivePut;
import com.ibm.wala.cast.tree.CAstSourcePositionMap.Position;
@ -26,8 +28,9 @@ public class ReadWriteCorrelation extends Correlation {
private final AbstractReflectiveGet get;
private final AbstractReflectivePut put;
public ReadWriteCorrelation(AbstractReflectiveGet get, AbstractReflectivePut put, String indexName) {
super(indexName);
public ReadWriteCorrelation(AbstractReflectiveGet get, AbstractReflectivePut put,
String indexName, Set<String> flownThroughLocals) {
super(indexName, flownThroughLocals);
this.get = get;
this.put = put;
}

View File

@ -17,6 +17,7 @@ import static com.ibm.wala.cast.tree.CAstNode.BLOCK_EXPR;
import static com.ibm.wala.cast.tree.CAstNode.BLOCK_STMT;
import static com.ibm.wala.cast.tree.CAstNode.CALL;
import static com.ibm.wala.cast.tree.CAstNode.CONSTANT;
import static com.ibm.wala.cast.tree.CAstNode.DECL_STMT;
import static com.ibm.wala.cast.tree.CAstNode.EMPTY;
import static com.ibm.wala.cast.tree.CAstNode.FUNCTION_EXPR;
import static com.ibm.wala.cast.tree.CAstNode.FUNCTION_STMT;
@ -36,6 +37,7 @@ import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Set;
import com.ibm.wala.cast.js.types.JavaScriptTypes;
import com.ibm.wala.cast.tree.CAst;
@ -45,8 +47,11 @@ import com.ibm.wala.cast.tree.CAstNode;
import com.ibm.wala.cast.tree.CAstNodeTypeMap;
import com.ibm.wala.cast.tree.CAstSourcePositionMap;
import com.ibm.wala.cast.tree.impl.CAstBasicRewriter.NoKey;
import com.ibm.wala.cast.tree.impl.CAstControlFlowRecorder;
import com.ibm.wala.cast.tree.impl.CAstOperator;
import com.ibm.wala.cast.tree.impl.CAstSymbolImpl;
import com.ibm.wala.util.collections.HashMapFactory;
import com.ibm.wala.util.collections.HashSetFactory;
import com.ibm.wala.util.collections.Pair;
import com.ibm.wala.util.debug.UnimplementedError;
@ -104,8 +109,16 @@ import com.ibm.wala.util.debug.UnimplementedError;
* <p>Local variable declarations inside the extracted code have to be hoisted to the enclosing function;
* otherwise they would become local variables of the extracted function instead.</p>
* <p>This is already taken care of by the translation from Rhino's AST to CAst.</p>
* <p>Optionally, the policy can request that one local variable of the surrounding function be turned into
* a local variable of the extracted closure. The rewriter checks that this is possible: the code to extract
* must not contain function calls or <code>new</code> expressions, and it must not contain <code>break</code>,
* <code>continue</code>, or <code>return</code> statements. The former requirement prevents a called function
* from observing a different value of the local variable than before. The latter requirement is necessary
* because the final value of the localised variable needs to be returned and assigned to its counterpart in
* the surrounding function; since non-local jumps are encoded by special return values (see next item),
* this would no longer be possible.</p>
* </li>
* <li><b><code>break</code>,<code>continue</code>,<code>return</code></b>:
* <li><b><code>break</code>, <code>continue</code>, <code>return</code></b>:
* <p>A <code>break</code> or <code>continue</code> statement within the extracted loop body that refers
* to the loop itself or an enclosing loop would become invalid in the transformed code. A <code>return</code>
* statement would no longer return from the enclosing function, but instead from the extracted function.</p>
@ -169,6 +182,8 @@ public class ClosureExtractor extends CAstRewriterExt {
private LinkedList<ExtractionPolicy> policies = new LinkedList<ExtractionPolicy>();
private final ExtractionPolicyFactory policyFactory;
private static final boolean LOCALISE = true;
// names for extracted functions are built from this string with a number appended
private static final String EXTRACTED_FUN_BASENAME = "_forin_body_";
@ -303,7 +318,7 @@ public class ClosureExtractor extends CAstRewriterExt {
private CAstNode copyReturn(CAstNode root, CAstControlFlowMap cfg, NodePos context, Map<Pair<CAstNode, NoKey>, CAstNode> nodeMap) {
ExtractionPos epos = ExtractionPos.getEnclosingExtractionPos(context);
if(epos == null)
if(epos == null || isSynthetic(root))
return copyNode(root, cfg, context, nodeMap);
// add a return to every enclosing extracted function body
@ -424,11 +439,19 @@ public class ClosureExtractor extends CAstRewriterExt {
if(tler.getEndInner() != -1)
throw new UnimplementedError("Two-level extraction not fully implemented.");
int i;
if(start.getKind() == CAstNode.BLOCK_EXPR) {
if(start.getKind() == CAstNode.BLOCK_STMT) {
CAstNode[] before = new CAstNode[tler.getStartInner()];
for(i=0;i<tler.getStartInner();++i)
prologue.add(copyNodes(start.getChild(i), cfg, context, nodeMap));
for(;i<start.getChildCount();++i)
fun_body_stmts.add(start.getChild(i));
before[i] = copyNodes(start.getChild(i), cfg, context, nodeMap);
prologue.add(Ast.makeNode(BLOCK_STMT, before));
if(i+1 == start.getChildCount()) {
fun_body_stmts.add(addSpuriousExnFlow(start.getChild(i), cfg));
} else {
CAstNode[] after = new CAstNode[start.getChildCount()-i];
for(int j=0;j+i<start.getChildCount();++j)
after[j] = addSpuriousExnFlow(start.getChild(j+i), cfg);
fun_body_stmts.add(Ast.makeNode(BLOCK_EXPR, after));
}
for(i=context.getStart()+1;i<context.getEnd();++i)
fun_body_stmts.add(root.getChild(i));
} else if(start.getKind() == CAstNode.LOCAL_SCOPE) {
@ -453,6 +476,38 @@ public class ClosureExtractor extends CAstRewriterExt {
}
}
}
List<String> locals = context.getRegion().getLocals();
String theLocal = null;
if(LOCALISE && locals.size() == 1 && noJumpsAndNoCalls(fun_body_stmts)) {
// the variable can be localised, remember its name
theLocal = locals.get(0);
// append "return <theLocal>;" to the end of the function body
CAstNode retLocal = Ast.makeNode(RETURN, addExnFlow(makeVarRef(theLocal), JavaScriptTypes.ReferenceError, entity, context));
markSynthetic(retLocal);
// insert as last stmt if fun_body_stmts is a single block, otherwise append
if(fun_body_stmts.size() == 1 && fun_body_stmts.get(0).getKind() == BLOCK_STMT) {
CAstNode[] stmts = new CAstNode[fun_body_stmts.get(0).getChildCount()+1];
for(int i=0;i<stmts.length-1;++i)
stmts[i] = fun_body_stmts.get(0).getChild(i);
stmts[stmts.length-1] = retLocal;
fun_body_stmts.set(0, Ast.makeNode(BLOCK_STMT, stmts));
} else {
fun_body_stmts.add(retLocal);
}
// prepend declaration "var <theLocal>;"
CAstNode theLocalDecl = Ast.makeNode(DECL_STMT, Ast.makeConstant(new CAstSymbolImpl(theLocal)),
addExnFlow(makeVarRef("$$undefined"), JavaScriptTypes.ReferenceError, entity, context));
if(fun_body_stmts.size() > 1) {
CAstNode newBlock = Ast.makeNode(BLOCK_STMT, fun_body_stmts.toArray(new CAstNode[0]));
fun_body_stmts.clear();
fun_body_stmts.add(newBlock);
}
fun_body_stmts.add(0, Ast.makeNode(BLOCK_STMT, theLocalDecl));
}
CAstNode fun_body = Ast.makeNode(BLOCK_STMT, fun_body_stmts.toArray(new CAstNode[0]));
/*
@ -551,6 +606,9 @@ public class ClosureExtractor extends CAstRewriterExt {
Ast.makeNode(LOCAL_SCOPE, wrapIn(BLOCK_STMT, fixup == null ? Ast.makeNode(EMPTY) : fixup)));
stmts.add(Ast.makeNode(BLOCK_STMT, decl, fixup));
} else if(theLocal != null) {
// assign final value of the localised variable back
stmts.add(Ast.makeNode(CAstNode.ASSIGN, addExnFlow(makeVarRef(theLocal), JavaScriptTypes.ReferenceError, entity, context), call));
} else {
stmts.add(call);
}
@ -570,6 +628,19 @@ public class ClosureExtractor extends CAstRewriterExt {
return stmts;
}
private CAstNode addSpuriousExnFlow(CAstNode node, CAstControlFlowMap cfg) {
CAstControlFlowRecorder flow = (CAstControlFlowRecorder)cfg;
if(node.getKind() == ASSIGN) {
if(node.getChild(0).getKind() == VAR) {
CAstNode var = node.getChild(0);
if(!flow.isMapped(var))
flow.map(var, var);
flow.add(var, CAstControlFlowMap.EXCEPTION_TO_EXIT, JavaScriptTypes.ReferenceError);
}
}
return node;
}
private CAstNode createReturnFixup(ExtractionPos context, CAstEntity entity) {
return Ast.makeNode(IF_STMT,
@ -672,4 +743,35 @@ public class ClosureExtractor extends CAstRewriterExt {
return true;
return false;
}
private final Set<CAstNode> synthetic = HashSetFactory.make();
private void markSynthetic(CAstNode node) {
this.synthetic.add(node);
}
private boolean isSynthetic(CAstNode node) {
return synthetic.contains(node);
}
private boolean noJumpsAndNoCalls(Collection<CAstNode> nodes) {
for(CAstNode node : nodes)
if(!noJumpsAndNoCalls(node))
return false;
return true;
}
private boolean noJumpsAndNoCalls(CAstNode node) {
switch(node.getKind()) {
case CAstNode.BREAK:
case CAstNode.CONTINUE:
case CAstNode.GOTO:
case CAstNode.RETURN:
case CAstNode.CALL:
case CAstNode.NEW:
return false;
}
for(int i=0;i<node.getChildCount();++i)
if(!noJumpsAndNoCalls(node.getChild(i)))
return false;
return true;
}
}

View File

@ -125,7 +125,9 @@ public class CorrelatedPairExtractionPolicy extends ExtractionPolicy {
endNode = endNodes.iterator().next();
}
Pair<CAstNode, ? extends ExtractionRegion> region_info = findClosestContainingBlock(entity, startNode, endNode, corr.getIndexName());
List<String> locals = corr.getFlownThroughLocals().size() == 1 ? Collections.singletonList(corr.getFlownThroughLocals().iterator().next())
: Collections.<String>emptyList();
Pair<CAstNode, ? extends ExtractionRegion> region_info = findClosestContainingBlock(entity, startNode, endNode, corr.getIndexName(), locals);
if(region_info == null) {
if(DEBUG)
System.err.println("Couldn't find enclosing block for correlation " + corr.pp(correlations.getPositions()));
@ -174,7 +176,7 @@ public class CorrelatedPairExtractionPolicy extends ExtractionPolicy {
}
}
private Pair<CAstNode, ? extends ExtractionRegion> findClosestContainingBlock(CAstEntity entity, ChildPos startNode, ChildPos endNode, String parmName) {
private Pair<CAstNode, ? extends ExtractionRegion> findClosestContainingBlock(CAstEntity entity, ChildPos startNode, ChildPos endNode, String parmName, List<String> locals) {
ChildPos pos = startNode;
CAstNode block = null;
int start = -1, end = 0;
@ -208,17 +210,17 @@ public class CorrelatedPairExtractionPolicy extends ExtractionPolicy {
}
// special hack to handle "var p = ..., x = y[p];", where startNode = "y[p]"
if(block.getChild(0).getKind() == CAstNode.BLOCK_EXPR && start == 0) {
if(block.getChild(0).getKind() == CAstNode.BLOCK_STMT && start == 0) {
for(start_inner=0;start_inner<block.getChild(0).getChildCount();++start_inner)
if(NodePos.inSubtree(startNode.getChild(), block.getChild(0).getChild(start_inner)))
return Pair.make(block, new TwoLevelExtractionRegion(start, end, start_inner, end_inner, Collections.singletonList(parmName)));
return Pair.make(block, new TwoLevelExtractionRegion(start, end, start_inner, end_inner, Collections.singletonList(parmName), locals));
}
// special hack to handle the case where we're extracting the body of a local scope
else if(block.getChild(start).getKind() == CAstNode.LOCAL_SCOPE && end == start+1) {
return Pair.make(block, new TwoLevelExtractionRegion(start, end, 0, -1, Collections.singletonList(parmName)));
return Pair.make(block, new TwoLevelExtractionRegion(start, end, 0, -1, Collections.singletonList(parmName), locals));
}
return Pair.make(block, new ExtractionRegion(start, end, Collections.singletonList(parmName)));
return Pair.make(block, new ExtractionRegion(start, end, Collections.singletonList(parmName), locals));
}
private static int getCoveringChildIndex(CAstNode parent, int start, CAstNode child) {

View File

@ -21,13 +21,19 @@ import java.util.List;
*/
public class ExtractionRegion {
private int start, end;
// parameters for the extracted method
private final List<String> parameters;
// variables that should be made local to the extracted method if possible
private final List<String> locals;
public ExtractionRegion(int start, int end, List<String> parameters) {
public ExtractionRegion(int start, int end, List<String> parameters, List<String> locals) {
super();
this.start = start;
this.end = end;
this.parameters = parameters;
this.locals = locals;
}
public int getStart() {
@ -49,4 +55,8 @@ public class ExtractionRegion {
public void setEnd(int end) {
this.end = end;
}
public List<String> getLocals() {
return locals;
}
}

View File

@ -73,9 +73,9 @@ public class ForInBodyExtractionPolicy extends ExtractionPolicy {
new SubtreeOfKind(LABEL_STMT)).matches(node)) {
List<String> parms = Collections.singletonList((String)loopVar.getLastMatch());
if(node.getChild(1).getKind() == LOCAL_SCOPE) {
return Collections.<ExtractionRegion>singletonList(new TwoLevelExtractionRegion(1, 2, 0, -1, parms));
return Collections.<ExtractionRegion>singletonList(new TwoLevelExtractionRegion(1, 2, 0, -1, parms, Collections.<String>emptyList()));
} else {
return Collections.singletonList(new ExtractionRegion(1, 2, parms));
return Collections.singletonList(new ExtractionRegion(1, 2, parms, Collections.<String>emptyList()));
}
}
@ -94,7 +94,7 @@ public class ForInBodyExtractionPolicy extends ExtractionPolicy {
new SubtreeOfKind(EACH_ELEMENT_GET)),
new AnyNode()).matches(node)) {
List<String> parms = Collections.singletonList((String)loopVar.getLastMatch());
return Collections.singletonList(new ExtractionRegion(1, 2, parms));
return Collections.singletonList(new ExtractionRegion(1, 2, parms, Collections.<String>emptyList()));
}
return null;
}

View File

@ -16,8 +16,8 @@ import java.util.List;
public class TwoLevelExtractionRegion extends ExtractionRegion {
private final int start_inner, end_inner;
public TwoLevelExtractionRegion(int start, int end, int start_inner, int end_inner, List<String> parameters) {
super(start, end, parameters);
public TwoLevelExtractionRegion(int start, int end, int start_inner, int end_inner, List<String> parameters, List<String> locals) {
super(start, end, parameters, locals);
this.start_inner = start_inner;
this.end_inner = end_inner;
}