diff --git a/infer/src/checkers/ThreadSafety.ml b/infer/src/checkers/ThreadSafety.ml index de7f6847d..b700ec4b6 100644 --- a/infer/src/checkers/ThreadSafety.ml +++ b/infer/src/checkers/ThreadSafety.ml @@ -128,13 +128,35 @@ let make_results_table file_env = in map_post_computation_over_procs compute_post_for_procedure procs_to_analyze +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 + in + Some (current_class,thread_safe_annotated_classes) + | _ -> None (*shouldn't happen*) + +(** The addendum message says that a superclass is marked @ThreadSafe, + when the current class is not so marked*) +let calculate_addendum_message tenv pname = + match get_current_class_and_threadsafe_superclasses tenv pname with + | Some (current_class,thread_safe_annotated_classes) -> + if not (IList.mem Typename.equal current_class thread_safe_annotated_classes) then + match thread_safe_annotated_classes with + | hd::_ -> F.asprintf "\n Note: Superclass %a is marked @ThreadSafe." Typename.pp hd + | [] -> "" + else "" + | _ -> "" let report_thread_safety_errors ( _, tenv, pname, pdesc) writestate = let report_one_error access_path = let description = - F.asprintf "Method %a writes to field %a outside of synchronization" + F.asprintf "Method %a writes to field %a outside of synchronization. %s" Procname.pp pname AccessPath.pp_access_list access_path + (calculate_addendum_message tenv pname) in Checkers.ST.report_error tenv pname @@ -157,11 +179,14 @@ let process_results_table tab = the classes in a file. This might be tightened in future or even broadened in future based on other criteria *) let should_analyze_file file_env = - IList.exists - (fun (_, tenv, pname, _) -> - PatternMatch.check_class_attributes Annotations.ia_is_thread_safe tenv pname - ) - file_env + let current_class_or_super_marked_threadsafe = + fun (_, tenv, pname, _) -> + match get_current_class_and_threadsafe_superclasses tenv pname with + | Some (_, thread_safe_annotated_classes) -> + not (thread_safe_annotated_classes = []) + | _ -> false + in + IList.exists current_class_or_super_marked_threadsafe file_env (*Gathers results by analyzing all the methods in a file, then post-processes the results to check (approximation of) thread safety *) diff --git a/infer/src/checkers/patternMatch.ml b/infer/src/checkers/patternMatch.ml index 3f47af59c..33bda63bb 100644 --- a/infer/src/checkers/patternMatch.ml +++ b/infer/src/checkers/patternMatch.ml @@ -379,3 +379,16 @@ let check_class_attributes check tenv = function check_class_annots (Procname.java_get_class_type_name java_pname) | _ -> false + +(** find superclasss with attributes (e.g., @ThreadSafe), including current class*) +let rec find_superclasses_with_attributes check tenv tname = + match Tenv.lookup tenv tname with + | Some (struct_typ) -> + let result_from_supers = IList.flatten + (IList.map (find_superclasses_with_attributes check tenv) struct_typ.supers) + in + if check struct_typ.annots then + tname ::result_from_supers + else + result_from_supers + | _ -> [] diff --git a/infer/src/checkers/patternMatch.mli b/infer/src/checkers/patternMatch.mli index 190d9bde4..162ca89c6 100644 --- a/infer/src/checkers/patternMatch.mli +++ b/infer/src/checkers/patternMatch.mli @@ -113,3 +113,7 @@ val is_runtime_exception : Tenv.t -> Typename.t -> bool (** tests whether any class attributes (e.g., @ThreadSafe) pass check of first argument*) val check_class_attributes : (Annot.Item.t -> bool) -> Tenv.t -> Procname.t -> bool + +(** find superclasss with attributes (e.g., @ThreadSafe), including current class*) +val find_superclasses_with_attributes : (Annot.Item.t -> bool) -> Tenv.t + -> Typename.t -> Typename.t list diff --git a/infer/tests/codetoanalyze/java/checkers/ExtendsThreadSafeExample.java b/infer/tests/codetoanalyze/java/checkers/ExtendsThreadSafeExample.java new file mode 100644 index 000000000..844023ca7 --- /dev/null +++ b/infer/tests/codetoanalyze/java/checkers/ExtendsThreadSafeExample.java @@ -0,0 +1,24 @@ +/* + * Copyright (c) 2016 - 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. + */ +class ExtendsThreadSafeExample extends ThreadSafeExample{ + + Integer field; + + (* Presently,we will warn not just on overwridden methods from + @ThreadSafe class, but potentially on other methods in subclass *) + public void newmethodBad() { + field = 22; + } + + (* Bad now that it's overridden *) + public void tsOK() { + field = 44; + } + +} diff --git a/infer/tests/codetoanalyze/java/checkers/issues.exp b/infer/tests/codetoanalyze/java/checkers/issues.exp index f83baca07..87db03c4b 100644 --- a/infer/tests/codetoanalyze/java/checkers/issues.exp +++ b/infer/tests/codetoanalyze/java/checkers/issues.exp @@ -21,6 +21,8 @@ ExpensiveInheritanceExample.java, void ExpensiveInheritanceExample.doesReportBec ExpensiveInheritanceExample.java, void ExpensiveInheritanceExample.reportsAssumingObjectOfTypeA(), 2, CHECKERS_CALLS_EXPENSIVE_METHOD ExpensiveInheritanceExample.java, void ExpensiveInheritanceExample.reportsBecauseFooIsExpensiveInA(A), 1, CHECKERS_CALLS_EXPENSIVE_METHOD ExpensiveSubtypingExample.java, void ExpensiveSubtypingExample.m3(), 0, CHECKERS_EXPENSIVE_OVERRIDES_UNANNOTATED +ExtendsThreadSafeExample.java, void ExtendsThreadSafeExample.newmethodBad(), 0, CHECKERS_THREAD_SAFETY_WARNING +ExtendsThreadSafeExample.java, void ExtendsThreadSafeExample.tsOK(), 0, CHECKERS_THREAD_SAFETY_WARNING FragmentRetainsViewExample.java, void FragmentRetainsViewExample.onDestroyView(), 0, CHECKERS_FRAGMENT_RETAINS_VIEW FragmentRetainsViewExample.java, void FragmentRetainsViewExample.onDestroyView(), 0, CHECKERS_FRAGMENT_RETAINS_VIEW FragmentRetainsViewExample.java, void FragmentRetainsViewExample.onDestroyView(), 0, CHECKERS_FRAGMENT_RETAINS_VIEW