From 86a1bbf1a79e5db7751f3fc0b1d8465f4da50855 Mon Sep 17 00:00:00 2001 From: Nikos Gorogiannis Date: Wed, 21 Aug 2019 08:25:22 -0700 Subject: [PATCH] [racerd] output access expressions language-sensitively Summary: Use whatever information we can to decide whether to use C or Java syntax when outputting an access expression, now that we store them as such. Also, make cluster callbacks explicitly set the language, as this was not done before and led to some confusion (Clang being set when analysing a Java file). Reviewed By: skcho Differential Revision: D16884160 fbshipit-source-id: 40adf9f35 --- infer/src/backend/callbacks.ml | 5 +- infer/src/concurrency/RacerD.ml | 6 +-- infer/src/concurrency/RacerDDomain.ml | 20 ++++--- infer/src/concurrency/RacerDDomain.mli | 3 ++ .../tests/codetoanalyze/cpp/racerd/issues.exp | 52 +++++++++---------- .../codetoanalyze/objcpp/racerd/issues.exp | 4 +- 6 files changed, 51 insertions(+), 39 deletions(-) diff --git a/infer/src/backend/callbacks.ml b/infer/src/backend/callbacks.ml index f085f0247..5e994a799 100644 --- a/infer/src/backend/callbacks.ml +++ b/infer/src/backend/callbacks.ml @@ -91,5 +91,8 @@ let iterate_cluster_callbacks all_procs exe_env source_file = true in List.iter - ~f:(fun {language; callback} -> if language_matches language then callback environment) + ~f:(fun {language; callback} -> + if language_matches language then ( + Language.curr_language := language ; + callback environment ) ) !cluster_callbacks diff --git a/infer/src/concurrency/RacerD.ml b/infer/src/concurrency/RacerD.ml index de93e4a29..81d634949 100644 --- a/infer/src/concurrency/RacerD.ml +++ b/infer/src/concurrency/RacerD.ml @@ -676,15 +676,15 @@ let get_reporting_explanation report_kind tenv pname thread = let pp_container_access fmt (access_exp, access_pname) = F.fprintf fmt "container %a via call to %s" - (MF.wrap_monospaced AccessPath.pp) - (AccessExpression.to_access_path access_exp) + (MF.wrap_monospaced RacerDDomain.pp_exp) + access_exp (MF.monospaced_to_string (Typ.Procname.get_method access_pname)) let pp_access fmt (t : RacerDDomain.TraceElem.t) = match t.elem with | Read {exp} | Write {exp} -> - (MF.wrap_monospaced AccessPath.pp) fmt (AccessExpression.to_access_path exp) + (MF.wrap_monospaced RacerDDomain.pp_exp) fmt exp | ContainerRead {exp; pname} | ContainerWrite {exp; pname} -> pp_container_access fmt (exp, pname) | InterfaceCall _ as access -> diff --git a/infer/src/concurrency/RacerDDomain.ml b/infer/src/concurrency/RacerDDomain.ml index 6fadd9e82..04c455ff0 100644 --- a/infer/src/concurrency/RacerDDomain.ml +++ b/infer/src/concurrency/RacerDDomain.ml @@ -21,6 +21,14 @@ let should_skip_var v = || match v with Var.ProgramVar pvar -> Pvar.is_static_local pvar | _ -> false +let pp_exp fmt exp = + match !Language.curr_language with + | Clang -> + AccessExpression.pp fmt exp + | Java -> + AccessPath.pp fmt (AccessExpression.to_access_path exp) + + module Access = struct type t = | Read of {exp: AccessExpression.t} @@ -88,18 +96,16 @@ module Access = struct F.fprintf fmt "Call to un-annotated interface method %a" Typ.Procname.pp pname + let mono_lang_pp = MF.wrap_monospaced pp_exp + let pp_human fmt = function | Read {exp} | Write {exp} -> - F.fprintf fmt "access to `%a`" AccessPath.pp (AccessExpression.to_access_path exp) + F.fprintf fmt "access to %a" mono_lang_pp exp | ContainerRead {exp; pname} -> - F.fprintf fmt "Read of container %a via call to %s" - (MF.wrap_monospaced AccessPath.pp) - (AccessExpression.to_access_path exp) + F.fprintf fmt "Read of container %a via call to %s" mono_lang_pp exp (MF.monospaced_to_string (Typ.Procname.get_method pname)) | ContainerWrite {exp; pname} -> - F.fprintf fmt "Write to container %a via call to %s" - (MF.wrap_monospaced AccessPath.pp) - (AccessExpression.to_access_path exp) + F.fprintf fmt "Write to container %a via call to %s" mono_lang_pp exp (MF.monospaced_to_string (Typ.Procname.get_method pname)) | InterfaceCall pname -> F.fprintf fmt "Call to un-annotated interface method %a" Typ.Procname.pp pname diff --git a/infer/src/concurrency/RacerDDomain.mli b/infer/src/concurrency/RacerDDomain.mli index bcb5e5d66..ee7cbcc0a 100644 --- a/infer/src/concurrency/RacerDDomain.mli +++ b/infer/src/concurrency/RacerDDomain.mli @@ -9,6 +9,9 @@ open! IStd module AccessExpression = HilExp.AccessExpression module F = Format +val pp_exp : F.formatter -> AccessExpression.t -> unit +(** language sensitive pretty-printer *) + module Access : sig type t = | Read of {exp: AccessExpression.t} (** Field or array read *) diff --git a/infer/tests/codetoanalyze/cpp/racerd/issues.exp b/infer/tests/codetoanalyze/cpp/racerd/issues.exp index fa21856fb..4c6c7f280 100644 --- a/infer/tests/codetoanalyze/cpp/racerd/issues.exp +++ b/infer/tests/codetoanalyze/cpp/racerd/issues.exp @@ -1,26 +1,26 @@ -codetoanalyze/cpp/racerd/basics.cpp, basics::Basic::get2, 36, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this.suspiciously_written`,,access to `this.suspiciously_written`] -codetoanalyze/cpp/racerd/basics.cpp, basics::Basic::get4, 43, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this.suspiciously_read`,,access to `this.suspiciously_read`] -codetoanalyze/cpp/racerd/basics.cpp, basics::Basic::get5, 45, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,call to basics::Basic::get_private_suspiciously_read,access to `this.suspiciously_read`,,access to `this.suspiciously_read`] -codetoanalyze/cpp/racerd/basics.cpp, basics::Basic::test_double_lock_bad, 81, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this.single_lock_suspiciously_read`,,access to `this.single_lock_suspiciously_read`] -codetoanalyze/cpp/racerd/dereferencing.cpp, dereferencing::Basic::call1, 51, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,call to dereferencing::Basic::mixed_deref_race,access to `this.x.x2.a.b.c`,,call to dereferencing::Basic::call1,call to dereferencing::Basic::mixed_deref_race,access to `this.x.x2.a.b.c`] -codetoanalyze/cpp/racerd/dereferencing.cpp, dereferencing::Basic::call1, 51, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,call to dereferencing::Basic::mixed_deref_race,access to `this.x.x1.u`,,call to dereferencing::Basic::call1,call to dereferencing::Basic::mixed_deref_race,access to `this.x.x1.u`] -codetoanalyze/cpp/racerd/dereferencing.cpp, dereferencing::Basic::call1, 51, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,call to dereferencing::Basic::mixed_deref_race,access to `this.x.x1.w`,,call to dereferencing::Basic::call1,call to dereferencing::Basic::mixed_deref_race,access to `this.x.x1.w`] -codetoanalyze/cpp/racerd/dereferencing.cpp, dereferencing::Basic::test_unlock, 59, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,call to dereferencing::Basic::call1,call to dereferencing::Basic::mixed_deref_race,access to `this.x.x2.a.b.c`,,call to dereferencing::Basic::call1,call to dereferencing::Basic::mixed_deref_race,access to `this.x.x2.a.b.c`] -codetoanalyze/cpp/racerd/dereferencing.cpp, dereferencing::Basic::test_unlock, 59, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,call to dereferencing::Basic::call1,call to dereferencing::Basic::mixed_deref_race,access to `this.x.x1.u`,,call to dereferencing::Basic::call1,call to dereferencing::Basic::mixed_deref_race,access to `this.x.x1.u`] -codetoanalyze/cpp/racerd/dereferencing.cpp, dereferencing::Basic::test_unlock, 59, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,call to dereferencing::Basic::call1,call to dereferencing::Basic::mixed_deref_race,access to `this.x.x1.w`,,call to dereferencing::Basic::call1,call to dereferencing::Basic::mixed_deref_race,access to `this.x.x1.w`] -codetoanalyze/cpp/racerd/locals_ownership.cpp, locals::Ownership::test2_bad, 49, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `x.f`,,access to `x.f`] -codetoanalyze/cpp/racerd/locals_ownership.cpp, locals::Ownership::test3_bad, 65, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `x.f`,,access to `x.f`] -codetoanalyze/cpp/racerd/lock_guard.cpp, basics::LockGuard::get2, 34, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this.suspiciously_written`,,access to `this.suspiciously_written`] -codetoanalyze/cpp/racerd/lock_guard.cpp, basics::LockGuard::get4, 40, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this.suspiciously_read`,,access to `this.suspiciously_read`] -codetoanalyze/cpp/racerd/lock_guard.cpp, basics::LockGuard::test1, 44, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this.suspiciously_read`,,access to `this.suspiciously_read`] -codetoanalyze/cpp/racerd/lock_guard_with_scope.cpp, basics::LockGuardWithScope::get2, 37, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this.suspiciously_written`,,access to `this.suspiciously_written`] -codetoanalyze/cpp/racerd/lock_guard_with_scope.cpp, basics::LockGuardWithScope::get4, 43, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this.suspiciously_read`,,access to `this.suspiciously_read`] -codetoanalyze/cpp/racerd/reporting.cpp, reporting::Basic::call1, 24, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,call to reporting::Basic::test,access to `this.x.x1.w`,,call to reporting::Basic::call1,call to reporting::Basic::test,access to `this.x.x1.w`] -codetoanalyze/cpp/racerd/reporting.cpp, reporting::Basic::test_unlock, 32, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,call to reporting::Basic::call1,call to reporting::Basic::test,access to `this.x.x1.w`,,call to reporting::Basic::call1,call to reporting::Basic::test,access to `this.x.x1.w`] -codetoanalyze/cpp/racerd/std_lock.cpp, basics::StdLock::get_bad, 31, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this.not_guarded`,,access to `this.not_guarded`] -codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock::get2, 45, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this.suspiciously_written1`,,access to `this.suspiciously_written1`] -codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock::get2, 46, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this.suspiciously_written2`,,access to `this.suspiciously_written2`] -codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock::get4, 55, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this.suspiciously_read1`,,access to `this.suspiciously_read1`] -codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock::get4, 56, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this.suspiciously_read2`,,access to `this.suspiciously_read2`] -codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock::get5, 64, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this.suspiciously_read1`,,access to `this.suspiciously_read1`] -codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock::get6, 73, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this.suspiciously_read1`,,access to `this.suspiciously_read1`] +codetoanalyze/cpp/racerd/basics.cpp, basics::Basic::get2, 36, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this->suspiciously_written`,,access to `this->suspiciously_written`] +codetoanalyze/cpp/racerd/basics.cpp, basics::Basic::get4, 43, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this->suspiciously_read`,,access to `this->suspiciously_read`] +codetoanalyze/cpp/racerd/basics.cpp, basics::Basic::get5, 45, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,call to basics::Basic::get_private_suspiciously_read,access to `this->suspiciously_read`,,access to `this->suspiciously_read`] +codetoanalyze/cpp/racerd/basics.cpp, basics::Basic::test_double_lock_bad, 81, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this->single_lock_suspiciously_read`,,access to `this->single_lock_suspiciously_read`] +codetoanalyze/cpp/racerd/dereferencing.cpp, dereferencing::Basic::call1, 51, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,call to dereferencing::Basic::mixed_deref_race,access to `this->x.x1->w`,,call to dereferencing::Basic::call1,call to dereferencing::Basic::mixed_deref_race,access to `this->x.x1->w`] +codetoanalyze/cpp/racerd/dereferencing.cpp, dereferencing::Basic::call1, 51, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,call to dereferencing::Basic::mixed_deref_race,access to `*(this->x.x2)->a.b.c`,,call to dereferencing::Basic::call1,call to dereferencing::Basic::mixed_deref_race,access to `*(this->x.x2)->a.b.c`] +codetoanalyze/cpp/racerd/dereferencing.cpp, dereferencing::Basic::call1, 51, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,call to dereferencing::Basic::mixed_deref_race,access to `this->x.x1->u`,,call to dereferencing::Basic::call1,call to dereferencing::Basic::mixed_deref_race,access to `this->x.x1->u`] +codetoanalyze/cpp/racerd/dereferencing.cpp, dereferencing::Basic::test_unlock, 59, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,call to dereferencing::Basic::call1,call to dereferencing::Basic::mixed_deref_race,access to `this->x.x1->w`,,call to dereferencing::Basic::call1,call to dereferencing::Basic::mixed_deref_race,access to `this->x.x1->w`] +codetoanalyze/cpp/racerd/dereferencing.cpp, dereferencing::Basic::test_unlock, 59, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,call to dereferencing::Basic::call1,call to dereferencing::Basic::mixed_deref_race,access to `this->x.x1->u`,,call to dereferencing::Basic::call1,call to dereferencing::Basic::mixed_deref_race,access to `this->x.x1->u`] +codetoanalyze/cpp/racerd/dereferencing.cpp, dereferencing::Basic::test_unlock, 59, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,call to dereferencing::Basic::call1,call to dereferencing::Basic::mixed_deref_race,access to `*(this->x.x2)->a.b.c`,,call to dereferencing::Basic::call1,call to dereferencing::Basic::mixed_deref_race,access to `*(this->x.x2)->a.b.c`] +codetoanalyze/cpp/racerd/locals_ownership.cpp, locals::Ownership::test2_bad, 49, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `x->f`,,access to `x->f`] +codetoanalyze/cpp/racerd/locals_ownership.cpp, locals::Ownership::test3_bad, 65, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `x->f`,,access to `x->f`] +codetoanalyze/cpp/racerd/lock_guard.cpp, basics::LockGuard::get2, 34, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this->suspiciously_written`,,access to `this->suspiciously_written`] +codetoanalyze/cpp/racerd/lock_guard.cpp, basics::LockGuard::get4, 40, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this->suspiciously_read`,,access to `this->suspiciously_read`] +codetoanalyze/cpp/racerd/lock_guard.cpp, basics::LockGuard::test1, 44, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this->suspiciously_read`,,access to `this->suspiciously_read`] +codetoanalyze/cpp/racerd/lock_guard_with_scope.cpp, basics::LockGuardWithScope::get2, 37, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this->suspiciously_written`,,access to `this->suspiciously_written`] +codetoanalyze/cpp/racerd/lock_guard_with_scope.cpp, basics::LockGuardWithScope::get4, 43, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this->suspiciously_read`,,access to `this->suspiciously_read`] +codetoanalyze/cpp/racerd/reporting.cpp, reporting::Basic::call1, 24, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,call to reporting::Basic::test,access to `this->x.x1->w`,,call to reporting::Basic::call1,call to reporting::Basic::test,access to `this->x.x1->w`] +codetoanalyze/cpp/racerd/reporting.cpp, reporting::Basic::test_unlock, 32, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,call to reporting::Basic::call1,call to reporting::Basic::test,access to `this->x.x1->w`,,call to reporting::Basic::call1,call to reporting::Basic::test,access to `this->x.x1->w`] +codetoanalyze/cpp/racerd/std_lock.cpp, basics::StdLock::get_bad, 31, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this->not_guarded`,,access to `this->not_guarded`] +codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock::get2, 45, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this->suspiciously_written1`,,access to `this->suspiciously_written1`] +codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock::get2, 46, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this->suspiciously_written2`,,access to `this->suspiciously_written2`] +codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock::get4, 55, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this->suspiciously_read1`,,access to `this->suspiciously_read1`] +codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock::get4, 56, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this->suspiciously_read2`,,access to `this->suspiciously_read2`] +codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock::get5, 64, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this->suspiciously_read1`,,access to `this->suspiciously_read1`] +codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock::get6, 73, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this->suspiciously_read1`,,access to `this->suspiciously_read1`] diff --git a/infer/tests/codetoanalyze/objcpp/racerd/issues.exp b/infer/tests/codetoanalyze/objcpp/racerd/issues.exp index 7eae5ce97..c791cd64d 100644 --- a/infer/tests/codetoanalyze/objcpp/racerd/issues.exp +++ b/infer/tests/codetoanalyze/objcpp/racerd/issues.exp @@ -1,2 +1,2 @@ -codetoanalyze/objcpp/racerd/Basic.mm, Basic::read_bad, 21, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `self._data`,,access to `self._data`] -codetoanalyze/objcpp/racerd/Private.mm, Private::read_other_bad, 28, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `self._other_data`,,access to `self._other_data`] +codetoanalyze/objcpp/racerd/Basic.mm, Basic::read_bad, 21, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `self->_data`,,access to `self->_data`] +codetoanalyze/objcpp/racerd/Private.mm, Private::read_other_bad, 28, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `self->_other_data`,,access to `self->_other_data`]