[thread-safety] warn on unsafe accesses in overrides of methods annotated with @ThreadSafeMethod

Reviewed By: jeremydubreil

Differential Revision: D4439591

fbshipit-source-id: 268890b

infer/src/checkers/ThreadSafety.ml

@@ -589,8 +589,27 @@ let should_report_on_file file_env =
    This indicates that the method races with itself. To be refined later. *)
 let process_results_table file_env tab =
   let should_report_on_all_procs = should_report_on_file file_env in
-  let should_report ((_, _, _, pdesc) as proc_env) =
-    (should_report_on_all_procs || is_annotated Annotations.ia_is_thread_safe_method pdesc)
+  (* TODO (t15588153): clean this up *)
+  let is_thread_safe_method pdesc tenv =
+    let overrides_thread_safe_method pname tenv =
+      let check_method_attributes check pname =
+        match Specs.proc_resolve_attributes pname with
+        | None -> false
+        | Some attributes ->
+            let annotated_signature = Annotations.get_annotated_signature attributes in
+            let ret_annotation, _ = annotated_signature.Annotations.ret in
+            check ret_annotation in
+      let found = ref false in
+      PatternMatch.proc_iter_overridden_methods
+        (fun pn ->
+           found := !found || check_method_attributes Annotations.ia_is_thread_safe_method pn)
+        tenv
+        pname;
+      !found in
+    is_annotated Annotations.ia_is_thread_safe_method pdesc||
+    overrides_thread_safe_method (Procdesc.get_proc_name pdesc) tenv in
+  let should_report ((_, tenv, _, pdesc) as proc_env) =
+    (should_report_on_all_procs || is_thread_safe_method pdesc tenv)
     && should_report_on_proc proc_env in
   ResultsTableType.iter (* report errors for each method *)
     (fun proc_env (_, _, conditional_writes, unconditional_writes) ->

infer/tests/codetoanalyze/java/threadsafety/ThreadSafeExample.java

@@ -153,4 +153,23 @@ class NonThreadSafeClass {
     this.field = new Object(); // should warn
   }
 
+  @ThreadSafeMethod
+  public void safeMethod() {
+  }
+
+}
+
+class NonThreadSafeSubclass extends NonThreadSafeClass {
+
+  @Override
+  // overrides method annotated with @ThreadSafeMethod, 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
+  // 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();
+  }
 }

infer/tests/codetoanalyze/java/threadsafety/issues.exp

@@ -25,6 +25,7 @@ codetoanalyze/java/threadsafety/Ownership.java, void Ownership.writeToNotOwnedIn
 codetoanalyze/java/threadsafety/ThreadSafeExample.java, void ExtendsThreadSafeExample.newmethodBad(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.ExtendsThreadSafeExample.field]
 codetoanalyze/java/threadsafety/ThreadSafeExample.java, void ExtendsThreadSafeExample.tsOK(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.ExtendsThreadSafeExample.field]
 codetoanalyze/java/threadsafety/ThreadSafeExample.java, void NonThreadSafeClass.threadSafeMethod(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.NonThreadSafeClass.field]
+codetoanalyze/java/threadsafety/ThreadSafeExample.java, void NonThreadSafeSubclass.safeMethod(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.NonThreadSafeClass.field]
 codetoanalyze/java/threadsafety/ThreadSafeExample.java, void ThreadSafeExample.callPublicMethodBad(), 1, THREAD_SAFETY_VIOLATION, [call to void ThreadSafeExample.assignInPrivateMethodOk(),access to codetoanalyze.java.checkers.ThreadSafeExample.f]
 codetoanalyze/java/threadsafety/ThreadSafeExample.java, void ThreadSafeExample.deeperTraceBad(), 1, THREAD_SAFETY_VIOLATION, [call to void ThreadSafeExample.callAssignInPrivateMethod(),call to void ThreadSafeExample.assignInPrivateMethodOk(),access to codetoanalyze.java.checkers.ThreadSafeExample.f]
 codetoanalyze/java/threadsafety/ThreadSafeExample.java, void ThreadSafeExample.oddBad(), 1, THREAD_SAFETY_VIOLATION, [call to void ThreadSafeExample.evenOk(),access to codetoanalyze.java.checkers.ThreadSafeExample.f]