From e91742afea1375ed3d9e75d95092814a54e37134 Mon Sep 17 00:00:00 2001 From: Peter O'Hearn Date: Mon, 31 Oct 2016 06:32:02 -0700 Subject: [PATCH] Support @SuppressLint("InvalidAccessToGuardedField") Summary: Don't issue GuardedBy warning on methods annotated with SuppressLint("InvalidAccessToGuardedField") Reviewed By: sblackshear Differential Revision: D4101133 fbshipit-source-id: f301eb1 --- infer/src/backend/rearrange.ml | 24 ++++++++++++++++++- infer/src/checkers/annotations.ml | 1 + infer/src/checkers/annotations.mli | 1 + .../java/infer/GuardedByExample.java | 18 +++++++++----- .../tests/codetoanalyze/java/infer/issues.exp | 1 + 5 files changed, 38 insertions(+), 7 deletions(-) diff --git a/infer/src/backend/rearrange.ml b/infer/src/backend/rearrange.ml index a11c68837..6c5d1ea69 100644 --- a/infer/src/backend/rearrange.ml +++ b/infer/src/backend/rearrange.ml @@ -657,6 +657,18 @@ let add_guarded_by_constraints tenv prop lexp pdesc = else None in IList.find_map_opt annot_extract_guarded_by_str item_annot in + let extract_suppress_warnings_str item_annot = + let annot_suppress_warnings_str ((annot: Annot.t), _) = + if Annotations.annot_ends_with annot Annotations.suppress_lint + then + match annot.parameters with + | [suppr_str] -> + Some suppr_str + | _ -> + None + else + None in + IList.find_map_opt annot_suppress_warnings_str item_annot in (* if [fld] is annotated with @GuardedBy("mLock"), return mLock *) let get_guarded_by_fld_str fld typ = match StructTyp.get_field_type_and_annotation ~lookup fld typ with @@ -754,6 +766,14 @@ let add_guarded_by_constraints tenv prop lexp pdesc = let guardedby_is_self_referential = string_equal "itself" (String.lowercase guarded_by_str) || string_is_suffix guarded_by_str (Ident.fieldname_to_string accessed_fld) in + let proc_has_suppress_guarded_by_annot pdesc = + let proc_signature = + Annotations.get_annotated_signature (Cfg.Procdesc.get_attributes pdesc) in + let proc_annot, _ = proc_signature.Annotations.ret in + match extract_suppress_warnings_str proc_annot with + | Some suppression_str-> + suppression_str = "InvalidAccessToGuardedField" + | None -> false in let should_warn pdesc = (* adding this check implements "by reference" semantics for guarded-by rather than "by value" semantics. if this access is through a local L or field V.f @@ -778,7 +798,9 @@ let add_guarded_by_constraints tenv prop lexp pdesc = not (Annotations.pdesc_has_annot pdesc Annotations.visibleForTesting) && not (Procname.java_is_access_method (Cfg.Procdesc.get_proc_name pdesc)) && not (is_accessible_through_local_ref lexp) && - not guardedby_is_self_referential in + not guardedby_is_self_referential && + not (proc_has_suppress_guarded_by_annot pdesc) + in match find_guarded_by_exp guarded_by_str prop.Prop.sigma with | Some (Sil.Eexp (guarded_by_exp, _), typ) -> if is_read_write_lock typ diff --git a/infer/src/checkers/annotations.ml b/infer/src/checkers/annotations.ml index c04c9fd6c..f88d5fed8 100644 --- a/infer/src/checkers/annotations.ml +++ b/infer/src/checkers/annotations.ml @@ -121,6 +121,7 @@ let no_allocation = "NoAllocation" let ignore_allocations = "IgnoreAllocations" let suppress_warnings = "SuppressWarnings" +let suppress_lint = "SuppressLint" let privacy_source = "PrivacySource" let privacy_sink = "PrivacySink" let integrity_source = "IntegritySource" diff --git a/infer/src/checkers/annotations.mli b/infer/src/checkers/annotations.mli index 65434c96d..ffc4a0dad 100644 --- a/infer/src/checkers/annotations.mli +++ b/infer/src/checkers/annotations.mli @@ -114,3 +114,4 @@ val pp_annotated_signature : Procname.t -> Format.formatter -> annotated_signatu val visibleForTesting : string val guarded_by : string +val suppress_lint : string diff --git a/infer/tests/codetoanalyze/java/infer/GuardedByExample.java b/infer/tests/codetoanalyze/java/infer/GuardedByExample.java index 3eec0df40..8fbbf9d1c 100644 --- a/infer/tests/codetoanalyze/java/infer/GuardedByExample.java +++ b/infer/tests/codetoanalyze/java/infer/GuardedByExample.java @@ -14,17 +14,13 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import com.google.common.annotations.VisibleForTesting; +import android.annotation.SuppressLint; +import javax.annotation.concurrent.GuardedBy; import java.io.Closeable; public class GuardedByExample { - @Retention(RetentionPolicy.CLASS) - @Target({ElementType.FIELD, ElementType.METHOD}) - public @interface GuardedBy { - String value(); - } - static class AutoCloseableReadWriteUpdateLock implements Closeable { @Override public void close() {} } @@ -72,6 +68,16 @@ public class GuardedByExample { this.f.toString(); } + @SuppressLint("InvalidAccessToGuardedField") + void readFBadButSuppressed() { + this.f.toString(); + } + + @SuppressLint("SomeOtherWarning") + void readFBadButSuppressedOther() { + this.f.toString(); + } + void writeFBad() { this.f = new Object(); } diff --git a/infer/tests/codetoanalyze/java/infer/issues.exp b/infer/tests/codetoanalyze/java/infer/issues.exp index 9b6b5a563..d22c95952 100644 --- a/infer/tests/codetoanalyze/java/infer/issues.exp +++ b/infer/tests/codetoanalyze/java/infer/issues.exp @@ -92,6 +92,7 @@ GuardedByExample.java, String GuardedByExample$3.readFromInnerClassBad1(), 2, UN GuardedByExample.java, String GuardedByExample$4.readFromInnerClassBad2(), 1, UNSAFE_GUARDED_BY_ACCESS GuardedByExample.java, void GuardedByExample.readFAfterBlockBad(), 3, UNSAFE_GUARDED_BY_ACCESS GuardedByExample.java, void GuardedByExample.readFBad(), 1, UNSAFE_GUARDED_BY_ACCESS +GuardedByExample.java, void GuardedByExample.readFBadButSuppressedOther(), 1, UNSAFE_GUARDED_BY_ACCESS GuardedByExample.java, void GuardedByExample.readFBadWrongAnnotation(), 1, UNSAFE_GUARDED_BY_ACCESS GuardedByExample.java, void GuardedByExample.readFBadWrongLock(), 2, UNSAFE_GUARDED_BY_ACCESS GuardedByExample.java, void GuardedByExample.readHBad(), 2, UNSAFE_GUARDED_BY_ACCESS