Skip to content

Commit

Permalink
Different approach for param analysis (#320)
Browse files Browse the repository at this point in the history 
* Different approach for param analysis

* Addressed comments
  • Loading branch information
@shubhamugare
shubhamugare committed Jun 17, 2019
1 parent 9e4b320 commit f1b0e3c
Show file tree
Hide file tree
Showing 2 changed files with 153 additions and 11 deletions.
60 changes: 54 additions & 6 deletions ...infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParams.java
View file
Expand Up @@ -16,6 +16,7 @@
package com.uber.nullaway.jarinfer;

import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.ibm.wala.cfg.ControlFlowGraph;
import com.ibm.wala.classLoader.IMethod;
Expand All @@ -33,8 +34,12 @@
import com.ibm.wala.util.graph.GraphUtil;
import com.ibm.wala.util.graph.dominators.Dominators;
import com.ibm.wala.util.graph.impl.GraphInverter;
import com.ibm.wala.util.graph.traverse.DFS;
import com.sun.istack.internal.NotNull;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;

/**
Expand All @@ -45,6 +50,7 @@
*/
public class DefinitelyDerefedParams {
private static final boolean DEBUG = false;
private boolean USE_EXTENDED_APPROACH = true;

private static void LOG(boolean cond, String tag, String msg) {
if (cond) System.out.println("[JI " + tag + "] " + msg);
Expand Down Expand Up @@ -98,15 +104,61 @@ private static void LOG(boolean cond, String tag, String msg) {
Set<Integer> analyze() {
// Get ExceptionPrunedCFG
LOG(DEBUG, "DEBUG", "@ " + method.getSignature());
Set<Integer> derefedParamList = new HashSet<Integer>();
prunedCFG = ExceptionPrunedCFG.make(cfg);
// In case the only control flows are exceptional, simply return.
if (prunedCFG.getNumberOfNodes() == 2
&& prunedCFG.containsNode(cfg.entry())
&& prunedCFG.containsNode(cfg.exit())
&& GraphUtil.countEdges(prunedCFG) == 0) {
return derefedParamList;
return new HashSet<>();
}

// Get number of params and value number of first param
int numParam = ir.getSymbolTable().getNumberOfParameters();
int firstParamIndex =
method.isStatic() ? 1 : 2; // 1-indexed; v1 is 'this' for non-static methods

Set<Integer> derefedParamList;
if (USE_EXTENDED_APPROACH) {
derefedParamList = computeDerefParamList(numParam, firstParamIndex);
} else {
derefedParamList = computeDerefParamListUsingPDom(numParam, firstParamIndex);
}
return derefedParamList;
}

@NotNull
private Set<Integer> computeDerefParamList(int numParam, int firstParamIndex) {
Set<Integer> derefedParamList = new HashSet<>();
Map<ISSABasicBlock, Set<Integer>> blockToDerefSetMap = new HashMap<>();

// find which params are treated as being non-null inside each basic block
prunedCFG.forEach(
basicBlock -> {
Set<Integer> derefParamSet = new HashSet<>();
blockToDerefSetMap.put(basicBlock, derefParamSet);
checkForUseOfParams(derefParamSet, numParam, firstParamIndex, basicBlock);
});

// For each param p, if no path exists from the entry block to the exit block which traverses
// only basic blocks which do not require p being non-null (e.g. by dereferencing it), then
// mark p as @NonNull
for (int i = firstParamIndex; i <= numParam; i++) {
final Integer param = i - 1;
if (!DFS.getReachableNodes(
prunedCFG,
ImmutableList.of(prunedCFG.entry()),
basicBlock -> !blockToDerefSetMap.get(basicBlock).contains(param))
.contains(prunedCFG.exit())) {
derefedParamList.add(param);
}
}
return derefedParamList;
}

@NotNull
private Set<Integer> computeDerefParamListUsingPDom(int numParam, int firstParamIndex) {
Set<Integer> derefedParamList = new HashSet<>();
// Get Dominator Tree
LOG(DEBUG, "DEBUG", "\tbuilding dominator tree...");
Graph<ISSABasicBlock> domTree = Dominators.make(prunedCFG, prunedCFG.entry()).dominatorTree();
Expand All @@ -120,10 +172,6 @@ Set<Integer> analyze() {
LOG(DEBUG, "DEBUG", "\tfinding dereferenced params...");
ArrayList<ISSABasicBlock> nodeQueue = new ArrayList<ISSABasicBlock>();
nodeQueue.add(prunedCFG.exit());
// Get number of params and value number of first param
int numParam = ir.getSymbolTable().getNumberOfParameters();
int firstParamIndex =
method.isStatic() ? 1 : 2; // 1-indexed; v1 is 'this' for non-static methods
LOG(DEBUG, "DEBUG", "param value numbers : " + firstParamIndex + " ... " + numParam);
while (!nodeQueue.isEmpty()) {
ISSABasicBlock node = nodeQueue.get(0);
Expand Down
104 changes: 99 additions & 5 deletions jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java
View file
Expand Up @@ -79,12 +79,11 @@ private void testTemplate(
Main.Result.OK,
compileResult);
DefinitelyDerefedParamsDriver driver = new DefinitelyDerefedParamsDriver();
Map<String, Set<Integer>> result =
driver.run(temporaryFolder.getRoot().getAbsolutePath(), "L" + pkg.replaceAll("\\.", "/"));
Assert.assertTrue(
testName + ": test failed!",
verify(
driver.run(
temporaryFolder.getRoot().getAbsolutePath(), "L" + pkg.replaceAll("\\.", "/")),
new HashMap<>(expected)));
testName + ": test failed! \n" + result + " does not match " + expected,
verify(result, new HashMap<>(expected)));
}

/**
Expand Down Expand Up @@ -275,6 +274,101 @@ public void toyNullTestAPI() throws Exception {
"}");
}

@Test
public void toyConditionalFlow() throws Exception {
testTemplate(
"toyNullTestAPI",
"toys",
"Foo",
ImmutableMap.of("toys.Foo:void test(String, String, String)", Sets.newHashSet(1, 2)),
"import com.google.common.base.Preconditions;",
"import java.util.Objects;",
"import org.junit.Assert;",
"class Foo {",
" private String foo;",
" public Foo(String str) {",
" if (str == null) str = \"foo\";",
" this.foo = str;",
" }",
" public void test(String s, String t, String u) {",
" if (s.length() >= 5) {",
" t.toString();",
" t = s;",
" } else {",
" Preconditions.checkNotNull(t);",
" u = t;",
" }",
" Objects.requireNonNull(u);",
" }",
"}");
}

@Test
public void toyConditionalFlow2() throws Exception {
testTemplate(
"toyNullTestAPI",
"toys",
"Foo",
ImmutableMap.of(
"toys.Foo:void test(Object, Object, Object, Object)", Sets.newHashSet(1, 4)),
"import com.google.common.base.Preconditions;",
"import java.util.Objects;",
"import org.junit.Assert;",
"class Foo {",
" private String foo;",
" public Foo(String str) {",
" if (str == null) str = \"foo\";",
" this.foo = str;",
" }",
" public void test(Object a, Object b, Object c, Object d) {",
" if (a != null) {",
" b.toString();",
" d.toString();",
" } else {",
" Preconditions.checkNotNull(c);",
" }",
" Objects.requireNonNull(a);",
" if (b != null) {",
" c.toString();",
" d.toString();",
" } else {",
" Preconditions.checkNotNull(b);",
" if (c != null) {",
" d.toString();",
" } else {",
" Preconditions.checkNotNull(d);",
" }",
" }",
" }",
"}");
}

@Test
public void toyReassigningTest() throws Exception {
testTemplate(
"toyNullTestAPI",
"toys",
"Foo",
ImmutableMap.of("toys.Foo:void test(String, String)", Sets.newHashSet(1)),
"import com.google.common.base.Preconditions;",
"import java.util.Objects;",
"import org.junit.Assert;",
"class Foo {",
" private String foo;",
" public Foo(String str) {",
" if (str == null) str = \"foo\";",
" this.foo = str;",
" }",
" public void test(String s, String t) {",
" Preconditions.checkNotNull(s);",
" if (t == null) {",
" t = s;",
" }",
" Objects.requireNonNull(t);",
" }",
"}");
}

@Test
public void toyJARAnnotatingClasses() throws Exception {
testAnnotationInJarTemplate(
Expand Down

0 comments on commit f1b0e3c

Please sign in to comment.