Skip to content

Commit

Permalink
Allow for multiple method parameters to be @MustCallAlias in Resource…
Browse files Browse the repository at this point in the history
… Leak Checker; fixes typetools#4785 (typetools#4808)
  • Loading branch information
Nargeshdb authored and wmdietl committed Aug 3, 2021
1 parent 609c83f commit d9c3167
Show file tree
Hide file tree
Showing 2 changed files with 153 additions and 54 deletions.
Expand Up @@ -239,20 +239,21 @@ private void handleInvocation(Set<Obligation> obligations, Node node) {

/**
* If node is an invocation of a this or super constructor that has a MustCallAlias return type
* and a MustCallAlias parameter, check if any variable in the current set of obligations is being
* passed to the other constructor. If so, remove it from the obligations.
* and some MustCallAlias parameter(s), check if any variable in the current set of obligations is
* being passed to the other constructor. If so, remove it from the obligations.
*
* @param obligations current obligations
* @param node a super or this constructor invocation
*/
private void handleThisOrSuperConstructorMustCallAlias(Set<Obligation> obligations, Node node) {
Node mustCallAliasArgument = getMustCallAliasArgumentNode(node);
List<Node> mustCallAliasArguments = getMustCallAliasArgumentNodes(node);
// If the MustCallAlias argument is also in the set of obligations, then remove it -- its
// obligation has been fulfilled by being passed on to the MustCallAlias constructor
// (because a this/super constructor call can only occur in the body of another
// constructor).
if (mustCallAliasArgument instanceof LocalVariableNode) {
removeObligationsContainingVar(obligations, (LocalVariableNode) mustCallAliasArgument);
// obligation has been fulfilled by being passed on to the MustCallAlias constructor (because a
// this/super constructor call can only occur in the body of another constructor).
for (Node mustCallAliasArgument : mustCallAliasArguments) {
if (mustCallAliasArgument instanceof LocalVariableNode) {
removeObligationsContainingVar(obligations, (LocalVariableNode) mustCallAliasArgument);
}
}
}

Expand Down Expand Up @@ -409,8 +410,8 @@ private boolean representSame(JavaExpression target, @Nullable JavaExpression en
/**
* Given a node representing a method or constructor call, checks that if the result of the call
* has a non-empty {@code @MustCall} type, then the result is pseudo-assigned to some location
* that can take ownership of the result. Searches for the set of same resources in {@code
* obligations} and adds the new resource alias to it if one exists. Otherwise creates a new set.
* that can take ownership of the result. Searches for the set(s) of same resources in {@code
* obligations} and adds the new resource alias to it if any exists. Otherwise creates a new set.
*
* @param obligations the currently-tracked obligations. This is always side-effected: an
* obligation is either modified (by removing it from the obligation set and adding a new one)
Expand All @@ -429,40 +430,45 @@ private void trackInvocationResult(Set<Obligation> obligations, Node node) {
return;
}

// `mustCallAlias` is the argument passed in the MustCallAlias position, if any exists,
// otherwise null.
Node mustCallAlias = getMustCallAliasArgumentNode(node);
// If mustCallAlias is null and call returns @This, set mustCallAlias to the receiver.
if (mustCallAlias == null
&& node instanceof MethodInvocationNode
// mustCallAliases is a (possibly-empty) list of arguments passed in a MustCallAlias position
List<Node> mustCallAliases = getMustCallAliasArgumentNodes(node);
// If call returns @This, add the receiver to mustCallAliases.
if (node instanceof MethodInvocationNode
&& typeFactory.returnsThis((MethodInvocationTree) tree)) {
mustCallAlias =
removeCastsAndGetTmpVarIfPresent(((MethodInvocationNode) node).getTarget().getReceiver());
mustCallAliases.add(
removeCastsAndGetTmpVarIfPresent(
((MethodInvocationNode) node).getTarget().getReceiver()));
}

ResourceAlias tmpVarAsResourceAlias = new ResourceAlias(new LocalVariable(tmpVar), tree);
if (mustCallAlias instanceof FieldAccessNode) {
// Do not track the call result if the MustCallAlias argument is a field. Handling of
// @Owning fields is a completely separate check, and there is never a need to track an
// alias of a non-@Owning field, as by definition such a field does not have
// obligations!
} else if (mustCallAlias instanceof LocalVariableNode) {
Obligation obligationContainingMustCallAlias =
getObligationForVar(obligations, (LocalVariableNode) mustCallAlias);
// If mustCallAlias is a local variable already being tracked, add tmpVarAsResourceAlias
// to the set containing mustCallAlias.
if (obligationContainingMustCallAlias != null) {
Set<ResourceAlias> newResourceAliasSet =
FluentIterable.from(obligationContainingMustCallAlias.resourceAliases)
.append(tmpVarAsResourceAlias)
.toSet();
obligations.remove(obligationContainingMustCallAlias);
obligations.add(new Obligation(newResourceAliasSet));
}
} else {
// If mustCallAlias is neither a field nor a local already in the set of obligations,
// add it to a new set.
if (mustCallAliases.isEmpty()) {
// If mustCallAliases is an empty List, add tmpVarAsResourceAlias to a new set.
obligations.add(new Obligation(ImmutableSet.of(tmpVarAsResourceAlias)));
} else {
for (Node mustCallAlias : mustCallAliases) {
if (mustCallAlias instanceof FieldAccessNode) {
// Do not track the call result if the MustCallAlias argument is a field. Handling of
// @Owning fields is a completely separate check, and there is never a need to track an
// alias of a non-@Owning field, as by definition such a field does not have obligations!
} else {
if (!(mustCallAlias instanceof LocalVariableNode)) {
throw new TypeSystemError(
"unexpected node type for mustCallAlias: " + mustCallAlias.getClass());
}
Obligation obligationContainingMustCallAlias =
getObligationForVar(obligations, (LocalVariableNode) mustCallAlias);
// If mustCallAlias is a local variable already being tracked, add tmpVarAsResourceAlias
// to the set containing mustCallAlias.
if (obligationContainingMustCallAlias != null) {
Set<ResourceAlias> newResourceAliasSet =
FluentIterable.from(obligationContainingMustCallAlias.resourceAliases)
.append(tmpVarAsResourceAlias)
.toSet();
obligations.remove(obligationContainingMustCallAlias);
obligations.add(new Obligation(newResourceAliasSet));
}
}
}
}
}

Expand Down Expand Up @@ -516,9 +522,13 @@ private boolean shouldTrackInvocationResult(Set<Obligation> obligations, Node no
* field or a definitely non-owning pointer
*/
private boolean returnTypeIsMustCallAliasWithUntrackable(MethodInvocationNode node) {
Node mustCallAliasArgument = getMustCallAliasArgumentNode(node);
return mustCallAliasArgument instanceof FieldAccessNode
|| mustCallAliasArgument instanceof ThisNode;
List<Node> mustCallAliasArguments = getMustCallAliasArgumentNodes(node);
for (Node mustCallAliasArg : mustCallAliasArguments) {
if (!(mustCallAliasArg instanceof FieldAccessNode || mustCallAliasArg instanceof ThisNode)) {
return false;
}
}
return !mustCallAliasArguments.isEmpty();
}

/**
Expand Down Expand Up @@ -991,38 +1001,38 @@ private String receiverAsString(FieldAccessNode fieldAccessNode) {
}

/**
* Finds the argument passed in the {@code @MustCallAlias} position for a call.
* Finds the arguments passed in the {@code @MustCallAlias} positions for a call.
*
* @param callNode callNode representing the call; must be {@link MethodInvocationNode} or {@link
* ObjectCreationNode}
* @return if {@code callNode} invokes a method with a {@code @MustCallAlias} annotation on some
* formal parameter (or the receiver), returns the result of calling {@link
* #removeCastsAndGetTmpVarIfPresent(Node)} on the argument passed in that position.
* Otherwise, returns {@code null}.
* formal parameter(s) (or the receiver), returns the result of calling {@link
* #removeCastsAndGetTmpVarIfPresent(Node)} on the argument(s) passed in corresponding
* position(s). Otherwise, returns an empty List.
*/
private @Nullable Node getMustCallAliasArgumentNode(Node callNode) {
private List<Node> getMustCallAliasArgumentNodes(Node callNode) {
Preconditions.checkArgument(
callNode instanceof MethodInvocationNode || callNode instanceof ObjectCreationNode);
List<Node> result = new ArrayList<>();
if (!typeFactory.hasMustCallAlias(callNode.getTree())) {
return null;
return result;
}

Node result = null;
List<Node> args = getArgumentsOfInvocation(callNode);
List<? extends VariableElement> parameters = getParametersOfInvocation(callNode);
for (int i = 0; i < args.size(); i++) {
if (typeFactory.hasMustCallAlias(parameters.get(i))) {
result = args.get(i);
break;
result.add(removeCastsAndGetTmpVarIfPresent(args.get(i)));
}
}

// If none of the parameters were @MustCallAlias, it must be the receiver
if (result == null && callNode instanceof MethodInvocationNode) {
result = ((MethodInvocationNode) callNode).getTarget().getReceiver();
if (result.isEmpty() && callNode instanceof MethodInvocationNode) {
result.add(
removeCastsAndGetTmpVarIfPresent(
((MethodInvocationNode) callNode).getTarget().getReceiver()));
}

result = removeCastsAndGetTmpVarIfPresent(result);
return result;
}

Expand Down
@@ -0,0 +1,89 @@
import java.io.*;
import java.net.*;
import org.checkerframework.checker.calledmethods.qual.*;
import org.checkerframework.checker.mustcall.qual.*;
import org.checkerframework.common.returnsreceiver.qual.*;

class MultipleMethodParamsMustCallAliasTest {

void testMultiMethodParamsCorrect1(@Owning InputStream in1, @Owning InputStream in2)
throws IOException {

ReplicaInputStreams r = new ReplicaInputStreams(in1, in2);

r.close();
}

void testMultiMethodParamsCorrect2(@Owning InputStream in1, @Owning InputStream in2)
throws IOException {

ReplicaInputStreams r = new ReplicaInputStreams(in1, in2);

try {
in1.close();
} catch (IOException e) {
} finally {
in2.close();
}
}

// It's a FP, see: https://github.com/typetools/checker-framework/issues/4843
// :: error: required.method.not.called
void testMultiMethodParamsCorrect3(@Owning InputStream in1, @Owning InputStream in2)
throws IOException {

ReplicaInputStreams r = new ReplicaInputStreams(in1, in2);

try {
in1.close();
} finally {
in2.close();
}
}

// :: error: required.method.not.called
void testMultiMethodParamsWrong1(@Owning InputStream in1, @Owning InputStream in2)
throws IOException {

ReplicaInputStreams r = new ReplicaInputStreams(in1, in2);

in1.close();
}

// :: error: required.method.not.called
void testMultiMethodParamsWrong2(@Owning InputStream in1, @Owning InputStream in2)
throws IOException {

ReplicaInputStreams r = new ReplicaInputStreams(in1, in2);

in2.close();
}

// :: error: required.method.not.called
void testMultiMethodParamsWrong3(@Owning InputStream in1) throws IOException {
// :: error: required.method.not.called
Socket socket = new Socket("address", 12);
ReplicaInputStreams r = new ReplicaInputStreams(in1, socket.getInputStream());
}

class ReplicaInputStreams implements Closeable {

private final @Owning InputStream in1;
private final @Owning InputStream in2;

public @MustCallAlias ReplicaInputStreams(
@MustCallAlias InputStream i1, @MustCallAlias InputStream i2) {
this.in1 = i1;
this.in2 = i2;
}

@Override
@EnsuresCalledMethods(
value = {"this.in1", "this.in2"},
methods = {"close"})
public void close() throws IOException {
in1.close();
in2.close();
}
}
}

0 comments on commit d9c3167

Please sign in to comment.