From 7ae58d78c3183dc9b9a50784e22b8dc3ae7df809 Mon Sep 17 00:00:00 2001
From: Sam Blackshear <shb@fb.com>
Date: Fri, 28 Oct 2016 10:50:56 -0700
Subject: [PATCH] [infer] fix .class synchronization false-positive in
 guarded-by check

Summary: Doing `sychronized(A.class)` where `A` is an inner class was not previously recognized by the `GuardedBy` checker.

Reviewed By: peterogithub

Differential Revision: D4095094

fbshipit-source-id: c832f9e
---
 infer/src/backend/rearrange.ml                |  9 +++++-
 .../java/infer/GuardedByExample.java          | 30 +++++++++++++++++--
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/infer/src/backend/rearrange.ml b/infer/src/backend/rearrange.ml
index c69616bae..a11c68837 100644
--- a/infer/src/backend/rearrange.ml
+++ b/infer/src/backend/rearrange.ml
@@ -626,7 +626,13 @@ let add_guarded_by_constraints tenv prop lexp pdesc =
   let guarded_by_str_is_this guarded_by_str =
     string_is_suffix "this" guarded_by_str in
   let guarded_by_str_is_class guarded_by_str class_str =
-    string_is_suffix guarded_by_str (class_str ^ ".class") in
+    let dollar_normalize s =
+      String.map
+        (function
+          | '$' -> '.'
+          | c -> c)
+        s in
+    string_is_suffix (dollar_normalize guarded_by_str) (dollar_normalize (class_str ^ ".class")) in
   let guarded_by_str_is_current_class guarded_by_str = function
     | Procname.Java java_pname ->
         (* programmers write @GuardedBy("MyClass.class") when the field is guarded by the class *)
@@ -674,6 +680,7 @@ let add_guarded_by_constraints tenv prop lexp pdesc =
     IList.find_map_opt
       (function
         | Sil.Hpointsto ((Const (Cclass clazz) as lhs_exp), _, Exp.Sizeof (typ, _, _))
+        | Sil.Hpointsto (_, Sil.Eexp (Const (Cclass clazz) as lhs_exp, _), Exp.Sizeof (typ, _, _))
           when guarded_by_str_is_class guarded_by_str (Ident.name_to_string clazz) ->
             Some (Sil.Eexp (lhs_exp, Sil.inst_none), typ)
         | Sil.Hpointsto (_, Estruct (flds, _), Exp.Sizeof (typ, _, _)) ->
diff --git a/infer/tests/codetoanalyze/java/infer/GuardedByExample.java b/infer/tests/codetoanalyze/java/infer/GuardedByExample.java
index 8f92eed68..3eec0df40 100644
--- a/infer/tests/codetoanalyze/java/infer/GuardedByExample.java
+++ b/infer/tests/codetoanalyze/java/infer/GuardedByExample.java
@@ -190,16 +190,17 @@ public class GuardedByExample {
     sGuardedByClass.toString();
   }
 
-  static void synchronizeOnClassOk() {
+  static void synchronizeOnClassOk1() {
     synchronized(GuardedByExample.class) {
       sGuardedByClass.toString(); // should not warn here
+      sGuardedByClass = new Object(); // or here
     }
   }
 
   void synchronizedOnThisBad() {
     sGuardedByClass.toString();
   }
-  
+
   Object dontReportOnCompilerGenerated() {
     return new Object() {
       public void accessInAnonClassOk() {
@@ -324,6 +325,13 @@ public class GuardedByExample {
     Object guardedByInnerThis2;
     @GuardedBy("GuardedByExample$Inner.this")
     Object guardedByInnerThis3;
+    @GuardedBy("Inner.class")
+    Object guardedByInnerClass1;
+    @GuardedBy("GuardedByExample.Inner.class")
+    Object guardedByInnerClass2;
+    @GuardedBy("GuardedByExample$Inner.class")
+    Object guardedByInnerClass3;
+
 
     synchronized void okAccess1() {
       guardedByInnerThis1 = null;
@@ -336,8 +344,24 @@ public class GuardedByExample {
     synchronized void okAccess3() {
       guardedByInnerThis3 = null;
     }
-  }
 
+    void okInnerClassGuard1() {
+      synchronized (Inner.class) {
+        guardedByInnerClass1 = new Object();
+        guardedByInnerClass2 = new Object();
+        guardedByInnerClass3 = new Object();
+      }
+    }
+
+    void okInnerClassGuard2() {
+      synchronized (GuardedByExample.Inner.class) {
+        guardedByInnerClass1 = new Object();
+        guardedByInnerClass2 = new Object();
+        guardedByInnerClass3 = new Object();
+      }
+    }
+
+  }
 
   // TODO: report on these cases
   /*