From 7bcc7e421d32cdbd69889b71748d8025dd45bd2c Mon Sep 17 00:00:00 2001 From: Peter O'Hearn Date: Thu, 5 Jan 2017 14:18:05 -0800 Subject: [PATCH] [threadsafety] don't warn on methods from classes annotated ThreadConfined Reviewed By: sblackshear Differential Revision: D4384861 fbshipit-source-id: 9b7c999 --- .../infer/annotation/ThreadConfined.java | 21 +++++++++++++++++++ infer/src/checkers/ThreadSafety.ml | 13 +++++++++++- infer/src/checkers/annotations.ml | 6 ++++-- infer/src/checkers/annotations.mli | 1 + .../java/threadsafety/Annotations.java | 17 +++++++++++++++ 5 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 infer/annotations/com/facebook/infer/annotation/ThreadConfined.java diff --git a/infer/annotations/com/facebook/infer/annotation/ThreadConfined.java b/infer/annotations/com/facebook/infer/annotation/ThreadConfined.java new file mode 100644 index 000000000..fe1ae2103 --- /dev/null +++ b/infer/annotations/com/facebook/infer/annotation/ThreadConfined.java @@ -0,0 +1,21 @@ +/* + * Copyright (c) 2004 - 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; + + +@Target(ElementType.TYPE) +@Retention(RetentionPolicy.CLASS) +public @interface ThreadConfined { +} diff --git a/infer/src/checkers/ThreadSafety.ml b/infer/src/checkers/ThreadSafety.ml index d94ca940d..bbab6a01c 100644 --- a/infer/src/checkers/ThreadSafety.ml +++ b/infer/src/checkers/ThreadSafety.ml @@ -245,6 +245,16 @@ 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 + + (* 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 = @@ -268,7 +278,8 @@ let should_analyze_proc pdesc tenv = not (FbThreadSafety.is_logging_method pn) && not (is_call_to_builder_class_method pn) && not (is_call_to_immutable_collection_method tenv pn) && - not (runs_on_ui_thread pdesc) + not (runs_on_ui_thread pdesc) && + not (is_thread_confined_method tenv pn) (* 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/src/checkers/annotations.ml b/infer/src/checkers/annotations.ml index 7eef7eaad..d534b862d 100644 --- a/infer/src/checkers/annotations.ml +++ b/infer/src/checkers/annotations.ml @@ -116,12 +116,11 @@ let guarded_by = "GuardedBy" let thread_safe = "ThreadSafe" let not_thread_safe = "NotThreadSafe" let ui_thread = "UiThread" +let thread_confined = "ThreadConfined" let ia_is_not_thread_safe ia = ia_ends_with ia not_thread_safe -(* we don't want it to just end with ThreadSafe, because this falls - foul of the @NotThreadSafe annotation *) let ia_is_thread_safe ia = ia_ends_with ia thread_safe @@ -218,6 +217,9 @@ let ia_is_guarded_by ia = let ia_is_ui_thread ia = ia_ends_with ia ui_thread +let ia_is_thread_confined ia = + ia_ends_with ia thread_confined + type annotation = | Nullable | Present diff --git a/infer/src/checkers/annotations.mli b/infer/src/checkers/annotations.mli index fe43be7d2..74c3f7cea 100644 --- a/infer/src/checkers/annotations.mli +++ b/infer/src/checkers/annotations.mli @@ -97,6 +97,7 @@ 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_ui_thread : Annot.Item.t -> bool +val ia_is_thread_confined : Annot.Item.t -> bool val ia_iter : (Annot.t -> unit) -> Annot.Item.t -> unit diff --git a/infer/tests/codetoanalyze/java/threadsafety/Annotations.java b/infer/tests/codetoanalyze/java/threadsafety/Annotations.java index 4beccf7b7..afd7662c0 100644 --- a/infer/tests/codetoanalyze/java/threadsafety/Annotations.java +++ b/infer/tests/codetoanalyze/java/threadsafety/Annotations.java @@ -11,6 +11,8 @@ package codetoanalyze.java.checkers; import android.support.annotation.UiThread; +import com.facebook.infer.annotation.ThreadConfined; + /** tests for classes and method annotations that are meaningful w.r.t thread-safety */ @ThreadSafe @@ -35,4 +37,19 @@ class Annotations { this.f = new Object(); } + Confined con; + + public void confinedCaller(){ + con.foo(); + } + + @ThreadConfined + class Confined { + Integer x; + + void foo(){ + x = 22; + } + } + }