diff --git a/infer/annotations/com/facebook/infer/annotation/AssumeThreadSafe.java b/infer/annotations/com/facebook/infer/annotation/ThreadSafe.java similarity index 60% rename from infer/annotations/com/facebook/infer/annotation/AssumeThreadSafe.java rename to infer/annotations/com/facebook/infer/annotation/ThreadSafe.java index e2f8ca64b..b4acd6b9f 100644 --- a/infer/annotations/com/facebook/infer/annotation/AssumeThreadSafe.java +++ b/infer/annotations/com/facebook/infer/annotation/ThreadSafe.java @@ -14,11 +14,14 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; -/** assume that a method is thread-safe without actually checking it (as opposed to @ThreadSafe, - which does check). useful for suppressing warnings involving benign races */ +/** + * Similar to @ThreadSafe annotation from javax.concurrent.annotation, but can be applied to + * methods. In addition, you can ask Infer to assume thread-safety rather than checking it by using + * @ThreadSafe(enableChecks = false). + */ -@Target({ ElementType.METHOD }) +@Target({ ElementType.METHOD, ElementType.TYPE }) @Retention(RetentionPolicy.CLASS) -public @interface AssumeThreadSafe { - String because(); /** describe why the thread-safety assumption is reasonable */ +public @interface ThreadSafe { + boolean enableChecks() default true; } diff --git a/infer/annotations/com/facebook/infer/annotation/ThreadSafeMethod.java b/infer/annotations/com/facebook/infer/annotation/ThreadSafeMethod.java deleted file mode 100644 index 99e29355e..000000000 --- a/infer/annotations/com/facebook/infer/annotation/ThreadSafeMethod.java +++ /dev/null @@ -1,22 +0,0 @@ -/* - * Copyright (c) 2017 - present Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - */ - -package com.facebook.infer.annotation; - -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -/** like javax.annotation.concurrent.ThreadSafe, but for individual methods rather than classes */ - -@Target(ElementType.METHOD) -@Retention(RetentionPolicy.CLASS) -public @interface ThreadSafeMethod { -} diff --git a/infer/src/checkers/ThreadSafety.ml b/infer/src/checkers/ThreadSafety.ml index 3d771f5e8..f4f4d6e98 100644 --- a/infer/src/checkers/ThreadSafety.ml +++ b/infer/src/checkers/ThreadSafety.ml @@ -669,8 +669,29 @@ let runs_on_ui_thread proc_desc = Annotations.ia_is_on_unbind annot || Annotations.ia_is_on_unmount annot) -let is_assumed_thread_safe pdesc = - Annotations.pdesc_return_annot_ends_with pdesc Annotations.assume_thread_safe + +(* returns true if the annotation is @ThreadSafe or @ThreadSafe(enableChecks = true) *) +let is_thread_safe item_annot = + let f (annot, _) = + Annotations.annot_ends_with annot Annotations.thread_safe && + match annot.Annot.parameters with + | ["false"] -> false + | _ -> true in + List.exists ~f item_annot + +(* returns true if the annotation is @ThreadSafe(enableChecks = false) *) +let is_assumed_thread_safe item_annot = + let f (annot, _) = + Annotations.annot_ends_with annot Annotations.thread_safe && + match annot.Annot.parameters with + | ["false"] -> true + | _ -> false in + List.exists ~f item_annot + +let pdesc_is_assumed_thread_safe pdesc tenv = + is_assumed_thread_safe (Annotations.pdesc_get_return_annot pdesc) || + PatternMatch.check_current_class_attributes + is_assumed_thread_safe tenv (Procdesc.get_proc_name pdesc) (* return true if we should compute a summary for the procedure. if this returns false, we won't analyze the procedure or report any warnings on it *) @@ -684,7 +705,7 @@ let should_analyze_proc pdesc tenv = not (is_call_to_immutable_collection_method tenv pn) && not (runs_on_ui_thread pdesc) && not (is_thread_confined_method tenv pdesc) && - not (is_assumed_thread_safe pdesc) + not (pdesc_is_assumed_thread_safe pdesc tenv) (* return true if we should report on unprotected accesses during the procedure *) let should_report_on_proc (_, _, proc_name, proc_desc) = @@ -762,8 +783,9 @@ let get_current_class_and_threadsafe_superclasses tenv pname = match pname with | Procname.Java java_pname -> let current_class = Procname.java_get_class_type_name java_pname in - let thread_safe_annotated_classes = PatternMatch.find_superclasses_with_attributes - Annotations.ia_is_thread_safe tenv current_class + let thread_safe_annotated_classes = + PatternMatch.find_superclasses_with_attributes + is_thread_safe tenv current_class in Some (current_class,thread_safe_annotated_classes) | _ -> None (*shouldn't happen*) @@ -981,7 +1003,7 @@ let process_results_table file_env tab = Annotations.pname_has_return_annot pn ~attrs_of_pname:Specs.proc_resolve_attributes - Annotations.ia_is_thread_safe_method) + is_thread_safe) tenv (Procdesc.get_proc_name pdesc) in let should_report ((_, tenv, _, pdesc) as proc_env) = diff --git a/infer/src/checkers/annotations.ml b/infer/src/checkers/annotations.ml index 46fa642fc..99e9c0d8d 100644 --- a/infer/src/checkers/annotations.ml +++ b/infer/src/checkers/annotations.ml @@ -15,7 +15,6 @@ module L = Logging (** Annotations. *) let any_thread = "AnyThread" -let assume_thread_safe = "AssumeThreadSafe" let bind = "Bind" let bind_view = "BindView" let bind_array = "BindArray" @@ -56,7 +55,6 @@ let suppress_lint = "SuppressLint" let suppress_view_nullability = "SuppressViewNullability" let thread_confined = "ThreadConfined" let thread_safe = "ThreadSafe" -let thread_safe_method = "ThreadSafeMethod" let true_on_null = "TrueOnNull" let ui_thread = "UiThread" let verify_annotation = "com.facebook.infer.annotation.Verify" @@ -122,15 +120,6 @@ let struct_typ_has_annot (struct_typ : StructTyp.t) predicate = let ia_is_not_thread_safe ia = ia_ends_with ia not_thread_safe -let ia_is_thread_safe ia = - ia_ends_with ia thread_safe - -let ia_is_thread_safe_method ia = - ia_ends_with ia thread_safe_method - -let ia_is_assume_thread_safe ia = - ia_ends_with ia assume_thread_safe - let ia_is_nullable ia = ia_ends_with ia nullable diff --git a/infer/src/checkers/annotations.mli b/infer/src/checkers/annotations.mli index 692b45e02..9f077f05f 100644 --- a/infer/src/checkers/annotations.mli +++ b/infer/src/checkers/annotations.mli @@ -12,7 +12,6 @@ open! IStd (** Annotations. *) val any_thread : string -val assume_thread_safe : string val expensive : string val no_allocation : string val nullable : string @@ -26,7 +25,6 @@ val strict : string val suppress_lint : string val thread_confined : string val thread_safe : string -val thread_safe_method : string val ui_thread : string val visibleForTesting : string @@ -77,9 +75,6 @@ val ia_is_integrity_source : Annot.Item.t -> bool val ia_is_integrity_sink : Annot.Item.t -> bool val ia_is_guarded_by : Annot.Item.t -> bool val ia_is_not_thread_safe : Annot.Item.t -> bool -val ia_is_thread_safe : Annot.Item.t -> bool -val ia_is_thread_safe_method : Annot.Item.t -> bool -val ia_is_assume_thread_safe : Annot.Item.t -> bool val ia_is_ui_thread : Annot.Item.t -> bool val ia_is_thread_confined : Annot.Item.t -> bool val ia_is_returns_ownership : Annot.Item.t -> bool diff --git a/infer/tests/codetoanalyze/java/threadsafety/Annotations.java b/infer/tests/codetoanalyze/java/threadsafety/Annotations.java index 50364a901..b940b26d8 100644 --- a/infer/tests/codetoanalyze/java/threadsafety/Annotations.java +++ b/infer/tests/codetoanalyze/java/threadsafety/Annotations.java @@ -14,11 +14,9 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; -import javax.annotation.concurrent.ThreadSafe; - import android.support.annotation.UiThread; -import com.facebook.infer.annotation.AssumeThreadSafe; +import com.facebook.infer.annotation.ThreadSafe; import com.facebook.infer.annotation.Functional; import com.facebook.infer.annotation.ThreadConfined; import com.facebook.infer.annotation.ReturnsOwnership; @@ -56,6 +54,16 @@ interface Interface { @ReturnsOwnership Obj returnsOwnershipMethod(); } +@ThreadSafe(enableChecks = false) +class AssumedThreadSafe { + + Object field; + + public void writeOk() { + this.field = new Object(); + } +} + @ThreadSafe class Annotations implements Interface { Object f; @@ -138,7 +146,7 @@ class Annotations implements Interface { this.f = new Object(); } - @AssumeThreadSafe(because = "it's a test") + @ThreadSafe(enableChecks = false) public void assumeThreadSafeOk() { this.f = new Object(); } @@ -293,4 +301,8 @@ class Annotations implements Interface { owned.f = new Object(); } + public void writeToAssumedThreadSafeClassOk(AssumedThreadSafe c) { + c.writeOk(); + } + } diff --git a/infer/tests/codetoanalyze/java/threadsafety/ThreadSafeExample.java b/infer/tests/codetoanalyze/java/threadsafety/ThreadSafeExample.java index f0b83f604..eb6a454a9 100644 --- a/infer/tests/codetoanalyze/java/threadsafety/ThreadSafeExample.java +++ b/infer/tests/codetoanalyze/java/threadsafety/ThreadSafeExample.java @@ -10,11 +10,10 @@ package codetoanalyze.java.checkers; import javax.annotation.concurrent.NotThreadSafe; -import javax.annotation.concurrent.ThreadSafe; -import com.google.common.annotations.VisibleForTesting; +import com.facebook.infer.annotation.ThreadSafe; -import com.facebook.infer.annotation.ThreadSafeMethod; +import com.google.common.annotations.VisibleForTesting; @ThreadSafe public class ThreadSafeExample{ @@ -161,12 +160,12 @@ class NonThreadSafeClass { Object field; - @ThreadSafeMethod + @ThreadSafe public void threadSafeMethod() { this.field = new Object(); // should warn } - @ThreadSafeMethod + @ThreadSafe public void safeMethod() { } @@ -175,12 +174,12 @@ class NonThreadSafeClass { class NonThreadSafeSubclass extends NonThreadSafeClass { @Override - // overrides method annotated with @ThreadSafeMethod, should warn + // overrides method annotated with @ThreadSafe, should warn public void safeMethod() { this.field = new Object(); } - // won't report this now, but should in the future. if a method annotated with @ThreadSafeMethod + // won't report this now, but should in the future. if a method annotated with @ThreadSafe // in class C touches field f, then all other accesses to f in C must also be thread-safe public void FN_touchesSameFieldAsThreadSafeMethod() { this.field = new Object();