Skip to content

Commit

Permalink
Enhance resource leak analysis with additional annotation(s) (eclipse…
Browse files Browse the repository at this point in the history
…-jdt#1716)

fixes eclipse-jdt#1530

Main feature work:
* Define @owning and @NotOwning in org.eclipse.jdt.annotation
* Evaluate @owning on method (return)
* Analysis of method body vis-a-vis its return type w/ or w/o @owning
  * New warning: returning a resource from a method lacking @owning
* Read and evaluate @Owning/@NotOwning from .class files
* add support for custom close method (@Owning-annotated method receiver)
* include @owning fields:
  + tag from source & binary
  + in close() methods (canonical & custom):
    - complain on unclosed @owning fields (current class & supers)
    - evaluate super.close() as closing super's @owning fields
    - don't suggest to use t-w-r for fields
  + implicitly use syntactic analysis for fields to understand pattern:
      if (this.resource != null) this.resource.close();
* new problems for insufficient resource management:
  + field not @owning
  + class with @owning field not implementing AutoCloseable
  + class with @owning field does not implement close()
  + new irritant InsufficientResourceManagement to control severity
* problem API in JavaCore
* Support @NotOwning on subtypes of AutoCloseable to imply resource-free
* detect unsafe redefinitions
* detect custom wrapper resources from pattern of annotations
* recognize first param of WrapperResource-ctor as implicitly @owning

Improvement of flow analysis:
* Improve analysis of t-w-r & try-finally
* better separate analysis modes w/ vs w/o annotations
  + new problem ids: MandatoryCloseNotShown(AtExit)
* more complete sync between outer-inner resources
* improve analysis on return statement as last thing in a method
* more complete integration of SwitchExpression
* consider equivalence between different FTV for the same local
  + look up the block hierarchy for nearest relevant FTV
  Result: more errors unclosed-at-exit re resource from outer scope

Implementation improvements:
* unify how we handle passing an argument to an invocation
* unify detection of risk
  + check null status of tracker binding and original local
  + respect/ignore UNREACHABLE per context:
    - riskyNullStatusAt: ignore
    - UnconditionalFlowInfo.isPotentially(Non)Null: respect
* unify marking of nested tracking variables (in & out!)
* more precise marking at various events (closed vs. passed to owned)
* better connection between finally and try blocks
  + these are analysed in "wrong" order: finally then try
  + remember precise BlockScope where an FTV was created
  + remember contexts where an FTV was modified within a block
    - use this to re-use an FTV from finally for the beginning of try
    - however, create a fresh FTV in all other relevant re-assignments
  + transfer finallyInfo across re-assign for effect of close in finally
* track FTV#owningState for more specific problem messages (future)
* try harder to leverage last meaningful flowInfo at method end
  (see BlockScope.isLastInMethod() and its callers)
* don't report "never closed" if indeed close occurred on some branch
* avoid raw types in FTV & BlockScope

Tests:
* new ResourceLeakAnnotatedTest to repeat existing tests:
  + switch mode to annotation-based resource leak analysis
  + selective overrides where different results are expected
  + move/add new tests here
+ AbstractCompilerTest needs to search super class of
  ResourceLeakAnnotatedTests for inherited tests.
  • Loading branch information
stephan-herrmann authored Jan 11, 2024
1 parent a7a1673 commit 4989c7f
Show file tree
Hide file tree
Showing 60 changed files with 3,431 additions and 456 deletions.
2 changes: 1 addition & 1 deletion org.eclipse.jdt.annotation/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Bundle-ManifestVersion: 2
Bundle-Name: %bundleName
Bundle-Localization: bundle
Bundle-SymbolicName: org.eclipse.jdt.annotation
Bundle-Version: 2.2.900.qualifier
Bundle-Version: 2.3.0.qualifier
Export-Package: org.eclipse.jdt.annotation
Bundle-RequiredExecutionEnvironment: JavaSE-1.8
Bundle-Vendor: %providerName
Expand Down
2 changes: 1 addition & 1 deletion org.eclipse.jdt.annotation/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
<version>4.31.0-SNAPSHOT</version>
</parent>
<artifactId>org.eclipse.jdt.annotation</artifactId>
<version>2.2.900-SNAPSHOT</version>
<version>2.3.0-SNAPSHOT</version>
<packaging>eclipse-plugin</packaging>

<build>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*******************************************************************************
* Copyright (c) 2024 GK Software SE and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Stephan Herrmann - initial API and implementation
*******************************************************************************/
package org.eclipse.jdt.annotation;

import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

/**
* This annotation allows to specify absence of responsibility for certain objects as they are passed from one method to another.
* <p>This annotation is the direct inverse of {@link Owning} and can be used in all locations, where that annotation is used.
* {@code @NotOwning} can be used do specify a shared responsibility rather than the clear separation between sources
* and sinks of resources given by {@link Owning}.
* </p>
* <p>
* In particular the annotation {@code @NotOwning} makes explicit that absence of {@code @Owning} is by intention.
* </p>
* <p>Additionally, placing {@code @NotOwning} on a class that implements {@link AutoCloseable} specifies that instances of this
* class do <em>not</em> hold any gc-resistant resources and therefore calling {@link AutoCloseable#close()} is not necessary.
* </p>
*
* @since 2.3
*/
@Retention(RetentionPolicy.CLASS)
@Documented
@Target({ ElementType.TYPE, ElementType.PARAMETER, ElementType.METHOD, ElementType.FIELD })
public @interface NotOwning {
// no details
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*******************************************************************************
* Copyright (c) 2024 GK Software SE and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Stephan Herrmann - initial API and implementation
*******************************************************************************/
package org.eclipse.jdt.annotation;

import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

/**
* This annotation allows to specify responsibility for certain objects as they are passed from one method to another.
* <p>
* Based on this annotation, flow oriented analysis tools can be made more precise without the need to perform
* full-system analysis, as the {@code @Owning} annotation describes a contract, consisting of two sides that can be
* checked independently.
* </p>
* <p>
* The Eclipse Compiler for Java&trade; (version 3.37 and greater) interprets this annotation when placed on a value of
* type {@link AutoCloseable} or any subtype. This annotation is interpreted specifically in these locations:
* </p>
* <dl>
* <dt>Method parameter ({@link ElementType#PARAMETER}):</dt>
* <dd>Here {@code @Owning} denotes that the responsibility to close a resource passed into this parameter will lie with the
* receiving method, on this particular flow.
* </dd>
* <dt>Method ({@link ElementType#METHOD}) - as a way to refer to the method return value:</dt>
* <dd>Here {@code @Owning} denotes that responsibility to close a resource received from a call to this method will lie
* with the receiving code.
* </dd>
* <dt>Field ({@link ElementType#FIELD})</dt>
* <dd>This annotation marks that the lifecycle of a resource stored in the field is tied to the lifecycle of the
* enclosing object. In order to allow for precise analysis in this situation, it is recommended that the enclosing
* class of such a field implements {@link AutoCloseable}. In that case, it will be the responsibility of the class's
* {@code close()} implementation to also close all resources stored in {@code @Owning} fields.
* <dt>Method receiver (via {@link ElementType#TYPE_USE})</dt>
* <dd>Annotating a method receiver (explicit {@code this} parameter) as owning signals that the method is responsible
* for closing all contained resources. The method will be treated as a "custom close method".<br/>
* The annotation is not evaluated in any other TYPE_USE locations.
* </dl>
* <p><strong>Responsibility</strong> to close a resource may <strong>initially arise</strong> from these situations:</p>
* <ul>
* <li>Instantiating a class that is a subtype of {@link AutoCloseable}.</li>
* <li>Receiving a method argument via a parameter of type {@link AutoCloseable} (or subtype) that is marked as {@code @Owning}.</li>
* <li>Receiving a result from a call to method that is marked as {@code @Owning} and returns {@link AutoCloseable} (or a subtype).
* <ul><li>If the {@code @Owning} annotation is absent in this situation, the receiving method is <em>potentially responsible</em>.</li></ul>
* </li>
* <li>Within the {@code close()} method of a class implementing {@link AutoCloseable}, and within each "custom close method",
* each resource field annotated as {@code @Owning} must be closed.</li>
* </ul>
* <p><strong>Responsibility</strong> to close a resource may be <strong>fulfilled</strong> by ensuring any of these measures on every possible path:</p>
* <ul>
* <li>Invoking {@link AutoCloseable#close()} or a "custom close method".</li>
* <li>Passing the resource into a method where the receiving parameter has type {@link AutoCloseable} (or subtype)
* and is marked as {@code @Owning}.</li>
* <li>Returning the resource to the caller, provided that the current method is marked as {@code @Owning} and has a declared return type
* of {@link AutoCloseable} (or a subtype).</li>
* <li>Assigning the resource to a field annotated as {@code @Owning}.
* <li>Within the {@code close()} method of a class implementing {@link AutoCloseable} and within a "custom close method"
* closing the resource held by a field tagged as {@code @Owning} can happen either directly, or for inherited fields
* by invoking {@code super.close()}, or the super version of a "custom close method".
* </ul>
*
* @since 2.3
*/
@Retention(RetentionPolicy.CLASS)
@Documented
@Target({ ElementType.PARAMETER, ElementType.METHOD, ElementType.FIELD, ElementType.TYPE_USE })
public @interface Owning {
// no details
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2023 IBM Corporation and others.
* Copyright (c) 2000, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -2048,6 +2048,23 @@ public interface IProblem {
/** @since 3.14 */
int DuplicateResource = Internal + 1251;

/** @since 3.37 */
int ShouldMarkMethodAsOwning = Internal + 1260;
/** @since 3.37 */
int MandatoryCloseNotShown = Internal + 1261;
/** @since 3.37 */
int MandatoryCloseNotShownAtExit = Internal + 1262;
/** @since 3.37 */
int NotOwningResourceField = Internal + 1263;
/** @since 3.37 */
int OwningFieldInNonResourceClass = Internal + 1264;
/** @since 3.37 */
int OwningFieldShouldImplementClose = Internal + 1265;
/** @since 3.37 */
int OverrideReducingParamterOwning = Internal + 1266;
/** @since 3.37 */
int OverrideAddingReturnOwning = Internal + 1267;

// terminally
/** @since 3.14 */
int UsingTerminallyDeprecatedType = TypeRelated + 1400;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2023 IBM Corporation and others.
* Copyright (c) 2000, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -32,9 +32,16 @@

import java.util.List;

import static org.eclipse.jdt.internal.compiler.lookup.MethodBinding.PARAM_NONNULL;
import static org.eclipse.jdt.internal.compiler.lookup.MethodBinding.PARAM_NULLABLE;
import static org.eclipse.jdt.internal.compiler.lookup.MethodBinding.PARAM_NULLITY;
import static org.eclipse.jdt.internal.compiler.lookup.MethodBinding.PARAM_OWNING;
import static org.eclipse.jdt.internal.compiler.lookup.MethodBinding.PARAM_NOTOWNING;

import org.eclipse.jdt.core.compiler.*;
import org.eclipse.jdt.internal.compiler.*;
import org.eclipse.jdt.internal.compiler.ast.TypeReference.AnnotationPosition;
import org.eclipse.jdt.internal.compiler.flow.FlowContext;
import org.eclipse.jdt.internal.compiler.flow.FlowInfo;
import org.eclipse.jdt.internal.compiler.impl.*;
import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
Expand Down Expand Up @@ -113,16 +120,28 @@ static void createArgumentBindings(Argument[] arguments, MethodBinding binding,
for (int i = 0, length = arguments.length; i < length; i++) {
Argument argument = arguments[i];
binding.parameters[i] = argument.createBinding(scope, binding.parameters[i]);
long argumentTagBits = argument.binding.tagBits;
if ((argumentTagBits & TagBits.AnnotationOwning) != 0) {
if (binding.parameterFlowBits == null) {
binding.parameterFlowBits = new byte[arguments.length];
}
binding.parameterFlowBits[i] |= PARAM_OWNING;
} else if ((argumentTagBits & TagBits.AnnotationNotOwning) != 0) {
if (binding.parameterFlowBits == null) {
binding.parameterFlowBits = new byte[arguments.length];
}
binding.parameterFlowBits[i] |= PARAM_NOTOWNING;
}
if (useTypeAnnotations)
continue; // no business with SE7 null annotations in the 1.8 case.
// createBinding() has resolved annotations, now transfer nullness info from the argument to the method:
long argTypeTagBits = (argument.binding.tagBits & TagBits.AnnotationNullMASK);
long argTypeTagBits = (argumentTagBits & TagBits.AnnotationNullMASK);
if (argTypeTagBits != 0) {
if (binding.parameterNonNullness == null) {
binding.parameterNonNullness = new Boolean[arguments.length];
if (binding.parameterFlowBits == null) {
binding.parameterFlowBits = new byte[arguments.length];
binding.tagBits |= TagBits.IsNullnessKnown;
}
binding.parameterNonNullness[i] = Boolean.valueOf(argTypeTagBits == TagBits.AnnotationNonNull);
binding.parameterFlowBits[i] = MethodBinding.flowBitFromAnnotationTagBit(argTypeTagBits);
}
}
}
Expand Down Expand Up @@ -213,38 +232,54 @@ public void bindThrownExceptions() {
}

/**
* Feed null information from argument annotations into the analysis and mark arguments as assigned.
* Feed information from certain argument annotations into the analysis and mark arguments as assigned.
* Annotations evaluated here are (if enabled):
* <ul>
* <li>NonNull - for null analysis
* <li>Nullable - for null analysis
* <li>Owning - for resource leak analysis
* <li>NotOwning - for resource leak analysis
* </ul>
*/
static void analyseArguments(LookupEnvironment environment, FlowInfo flowInfo, Argument[] methodArguments, MethodBinding methodBinding) {
static void analyseArguments(LookupEnvironment environment, FlowInfo flowInfo, FlowContext flowContext, Argument[] methodArguments, MethodBinding methodBinding) {
if (methodArguments != null) {
boolean usesNullTypeAnnotations = environment.usesNullTypeAnnotations();
boolean usesOwningAnnotations = environment.usesOwningAnnotations();

int length = Math.min(methodBinding.parameters.length, methodArguments.length);
for (int i = 0; i < length; i++) {
TypeBinding parameterBinding = methodBinding.parameters[i];
LocalVariableBinding local = methodArguments[i].binding;
if (usesNullTypeAnnotations) {
// leverage null type annotations:
long tagBits = methodBinding.parameters[i].tagBits & TagBits.AnnotationNullMASK;
long tagBits = parameterBinding.tagBits & TagBits.AnnotationNullMASK;
if (tagBits == TagBits.AnnotationNonNull)
flowInfo.markAsDefinitelyNonNull(methodArguments[i].binding);
flowInfo.markAsDefinitelyNonNull(local);
else if (tagBits == TagBits.AnnotationNullable)
flowInfo.markPotentiallyNullBit(methodArguments[i].binding);
else if (methodBinding.parameters[i].isFreeTypeVariable())
flowInfo.markNullStatus(methodArguments[i].binding, FlowInfo.FREE_TYPEVARIABLE);
flowInfo.markPotentiallyNullBit(local);
else if (parameterBinding.isFreeTypeVariable())
flowInfo.markNullStatus(local, FlowInfo.FREE_TYPEVARIABLE);
} else {
if (methodBinding.parameterNonNullness != null) {
if (methodBinding.parameterFlowBits != null) {
// leverage null-info from parameter annotations:
Boolean nonNullNess = methodBinding.parameterNonNullness[i];
if (nonNullNess != null) {
if (nonNullNess.booleanValue())
flowInfo.markAsDefinitelyNonNull(methodArguments[i].binding);
else
flowInfo.markPotentiallyNullBit(methodArguments[i].binding);
}
int nullity = methodBinding.parameterFlowBits[i] & PARAM_NULLITY;
if (nullity == PARAM_NONNULL)
flowInfo.markAsDefinitelyNonNull(local);
else if (nullity == PARAM_NULLABLE)
flowInfo.markPotentiallyNullBit(local);
}
}
if (!flowInfo.hasNullInfoFor(methodArguments[i].binding))
flowInfo.markNullStatus(methodArguments[i].binding, FlowInfo.UNKNOWN); // ensure nullstatus is initialized
if (!flowInfo.hasNullInfoFor(local))
flowInfo.markNullStatus(local, FlowInfo.UNKNOWN); // ensure nullstatus is initialized
// tag parameters as being set:
flowInfo.markAsDefinitelyAssigned(methodArguments[i].binding);
flowInfo.markAsDefinitelyAssigned(local);

if (usesOwningAnnotations && local.type.hasTypeBit(TypeIds.BitAutoCloseable|TypeIds.BitCloseable)) {
long owningTagBits = local.tagBits & TagBits.AnnotationOwningMASK;
int initialNullStatus = (local.tagBits & TagBits.AnnotationOwning) !=0 ? FlowInfo.NULL : FlowInfo.NON_NULL; // defaulting to not-owning
local.closeTracker = new FakedTrackingVariable(local, methodArguments[i], flowInfo, flowContext, initialNullStatus, usesOwningAnnotations);
local.closeTracker.owningState = FakedTrackingVariable.owningStateFromTagBits(owningTagBits, FakedTrackingVariable.NOT_OWNED_PER_DEFAULT);
}
}
}
}
Expand Down Expand Up @@ -635,6 +670,15 @@ public void resolveReceiver() {
if (this.receiver.type.hasNullTypeAnnotation(AnnotationPosition.ANY)) {
this.scope.problemReporter().nullAnnotationUnsupportedLocation(this.receiver.type);
}
if (this.scope.compilerOptions().isAnnotationBasedResourceAnalysisEnabled && this.receiver.type.resolvedType != null) {
for (AnnotationBinding annotationBinding : this.receiver.type.resolvedType.getTypeAnnotations()) {
ReferenceBinding annotationType = annotationBinding.getAnnotationType();
if (annotationType != null && annotationType.hasTypeBit(TypeIds.BitOwningAnnotation)) {
this.binding.extendedTagBits = ExtendedTagBits.IsClosingMethod;
break;
}
}
}
}
public void resolveJavadoc() {

Expand Down Expand Up @@ -699,14 +743,15 @@ void validateNullAnnotations(boolean useTypeAnnotations) {
if (this.binding == null) return;
// null annotations on parameters?
if (!useTypeAnnotations) {
if (this.binding.parameterNonNullness != null) {
if (this.binding.parameterFlowBits != null) {
int length = this.binding.parameters.length;
for (int i=0; i<length; i++) {
if (this.binding.parameterNonNullness[i] != null) {
long nullAnnotationTagBit = this.binding.parameterNonNullness[i].booleanValue()
byte nullity = this.binding.parameterFlowBits[i];
if (nullity != 0) {
long nullAnnotationTagBit = nullity == PARAM_NONNULL
? TagBits.AnnotationNonNull : TagBits.AnnotationNullable;
if (!this.scope.validateNullAnnotation(nullAnnotationTagBit, this.arguments[i].type, this.arguments[i].annotations))
this.binding.parameterNonNullness[i] = null;
this.binding.parameterFlowBits[i] &= ~PARAM_NULLITY;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,15 +109,16 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl
&& this.resolvedType instanceof ReferenceBinding
&& ((ReferenceBinding)this.resolvedType).hasTypeBit(TypeIds.BitWrapperCloseable);
for (int i = 0, count = this.arguments.length; i < count; i++) {
Expression argument = this.arguments[i];
flowInfo =
this.arguments[i]
argument
.analyseCode(currentScope, flowContext, flowInfo)
.unconditionalInits();
// if argument is an AutoCloseable insert info that it *may* be closed (by the target method, i.e.)
if (analyseResources && !hasResourceWrapperType) { // allocation of wrapped closeables is analyzed specially
flowInfo = FakedTrackingVariable.markPassedToOutside(currentScope, this.arguments[i], flowInfo, flowContext, false);
flowInfo = handleResourcePassedToInvocation(currentScope, this.binding, argument, i, flowContext, flowInfo);
}
this.arguments[i].checkNPEbyUnboxing(currentScope, flowContext, flowInfo);
argument.checkNPEbyUnboxing(currentScope, flowContext, flowInfo);
}
analyseArguments(currentScope, flowContext, flowInfo, this.binding, this.arguments);
}
Expand All @@ -139,7 +140,7 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl

// after having analysed exceptions above start tracking newly allocated resource:
if (currentScope.compilerOptions().analyseResourceLeaks && FakedTrackingVariable.isAnyCloseable(this.resolvedType))
FakedTrackingVariable.analyseCloseableAllocation(currentScope, flowInfo, this);
FakedTrackingVariable.analyseCloseableAllocation(currentScope, flowInfo, flowContext, this);

ReferenceBinding declaringClass = this.binding.declaringClass;
MethodScope methodScope = currentScope.methodScope();
Expand Down
Loading

0 comments on commit 4989c7f

Please sign in to comment.