From f4b1af6f913a987871a99b24fc68d7e32b43413c Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Wed, 18 Jan 2017 19:57:57 -0800 Subject: [PATCH] [thread-safety] allow @ThreadConfined annotation on methods Reviewed By: peterogithub Differential Revision: D4433240 fbshipit-source-id: 9fb646b --- .../infer/annotation/ThreadConfined.java | 2 +- infer/src/checkers/ThreadSafety.ml | 21 ++++++++++--------- .../java/threadsafety/Annotations.java | 5 +++++ 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/infer/annotations/com/facebook/infer/annotation/ThreadConfined.java b/infer/annotations/com/facebook/infer/annotation/ThreadConfined.java index 60424da1e..b7353864f 100644 --- a/infer/annotations/com/facebook/infer/annotation/ThreadConfined.java +++ b/infer/annotations/com/facebook/infer/annotation/ThreadConfined.java @@ -15,7 +15,7 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; -@Target({ ElementType.TYPE, ElementType.FIELD }) +@Target({ ElementType.TYPE, ElementType.FIELD, ElementType.METHOD }) @Retention(RetentionPolicy.CLASS) public @interface ThreadConfined { } diff --git a/infer/src/checkers/ThreadSafety.ml b/infer/src/checkers/ThreadSafety.ml index 6037c0f8e..30bb9c8bb 100644 --- a/infer/src/checkers/ThreadSafety.ml +++ b/infer/src/checkers/ThreadSafety.ml @@ -291,20 +291,21 @@ let is_initializer tenv proc_name = Procname.is_class_initializer proc_name || FbThreadSafety.is_custom_init tenv proc_name -(* Methods in @ThreadConfined classes are assumed to all run on the same thread. - For the moment we won't warn on accesses resulting from use of such methods at all. - In future we should account for races between these methods and methods from completely - different classes that don't necessarily run on the same thread as the confined object. -*) -let is_thread_confined_method tenv pname = - PatternMatch.check_current_class_attributes - Annotations.ia_is_thread_confined tenv pname - let is_annotated f_annot pdesc = let annotated_signature = Annotations.get_annotated_signature (Procdesc.get_attributes pdesc) in let ret_annotation, _ = annotated_signature.Annotations.ret in f_annot ret_annotation +(* Methods in @ThreadConfined classes and methods annotated with @ThreadConfied are assumed to all + run on the same thread. For the moment we won't warn on accesses resulting from use of such + methods at all. In future we should account for races between these methods and methods from + completely different classes that don't necessarily run on the same thread as the confined + object. *) +let is_thread_confined_method tenv pdesc = + is_annotated Annotations.ia_is_thread_confined pdesc || + PatternMatch.check_current_class_attributes + Annotations.ia_is_thread_confined tenv (Procdesc.get_proc_name pdesc) + (* we don't want to warn on methods that run on the UI thread because they should always be single-threaded *) let runs_on_ui_thread proc_desc = @@ -325,7 +326,7 @@ let should_analyze_proc pdesc tenv = not (is_call_to_builder_class_method pn) && not (is_call_to_immutable_collection_method tenv pn) && not (runs_on_ui_thread pdesc) && - not (is_thread_confined_method tenv pn) + not (is_thread_confined_method tenv pdesc) (* return true if we should report on unprotected accesses during the procedure *) let should_report_on_proc (_, tenv, proc_name, proc_desc) = diff --git a/infer/tests/codetoanalyze/java/threadsafety/Annotations.java b/infer/tests/codetoanalyze/java/threadsafety/Annotations.java index df57957e8..28003bb08 100644 --- a/infer/tests/codetoanalyze/java/threadsafety/Annotations.java +++ b/infer/tests/codetoanalyze/java/threadsafety/Annotations.java @@ -83,4 +83,9 @@ class Annotations { this.encapsulatedField.fld = new Object(); } + @ThreadConfined + public void threadConfinedMethodOk() { + this.f = new Object(); + } + }