From b1e3d994bd365b35c22f313de9cc12f1abec9d3b Mon Sep 17 00:00:00 2001 From: Dulma Churchill Date: Tue, 14 Jun 2016 15:25:47 -0700 Subject: [PATCH] Report more cases of null dereference as empty vector access Differential Revision: D3429002 fbshipit-source-id: e87960f --- infer/src/backend/errdesc.ml | 17 ++++++++--- infer/src/backend/localise.ml | 4 +-- infer/src/backend/localise.mli | 2 +- .../cpp/errors/vector/empty_access.cpp | 30 ++++++++++++++++--- .../endtoend/cpp/VectorEmptyAccessTest.java | 4 ++- 5 files changed, 45 insertions(+), 12 deletions(-) diff --git a/infer/src/backend/errdesc.ml b/infer/src/backend/errdesc.ml index 8cb11780f..1a6c0c6ab 100644 --- a/infer/src/backend/errdesc.ml +++ b/infer/src/backend/errdesc.ml @@ -43,6 +43,14 @@ let method_of_pointer_wrapper pname = let is_vector_method pname = is_method_of_objc_cpp_class pname [vector_class] +let is_special_field class_names field_name_opt field = + let complete_fieldname = Ident.fieldname_to_complete_string field in + let field_ok = + match field_name_opt with + | Some field_name -> Utils.string_contains field_name complete_fieldname + | None -> true in + is_one_of_classes complete_fieldname class_names && field_ok + (** Check whether the hpred is a |-> representing a resource in the Racquire state *) let hpred_is_open_resource prop = function | Sil.Hpointsto(e, _, _) -> @@ -798,9 +806,7 @@ let explain_dereference_access outermost_array is_nullable _de_opt prop = Some (if outermost_array then remove_outermost_array_access de else de) in let value_str = match de_opt with | Some (Sil.Darrow ((Sil.Dpvaraddr pvar), f) as de) -> - let complete_fieldname = Ident.fieldname_to_complete_string f in - if is_one_of_classes complete_fieldname smart_pointers && - Utils.string_contains "data" complete_fieldname then + if is_special_field smart_pointers None f then Sil.dexp_to_string (Sil.Dpvaraddr pvar) else Sil.dexp_to_string de | Some de -> @@ -835,7 +841,10 @@ let create_dereference_desc | _ -> desc) | Some (Sil.Dretcall (Dconst (Cfun pname), this_dexp :: _, loc, _ )) when is_vector_method pname -> - Localise.desc_empty_vector_access pname (Sil.dexp_to_string this_dexp) loc + Localise.desc_empty_vector_access (Some pname) (Sil.dexp_to_string this_dexp) loc + | Some (Sil.Darrow (dexp, fieldname)) + when is_special_field [vector_class] (Some "beginPtr") fieldname -> + Localise.desc_empty_vector_access None (Sil.dexp_to_string dexp) loc | _ -> desc else desc in if use_buckets then Buckets.classify_access desc access_opt' de_opt is_nullable diff --git a/infer/src/backend/localise.ml b/infer/src/backend/localise.ml index ce8b958ec..96ea401fe 100644 --- a/infer/src/backend/localise.ml +++ b/infer/src/backend/localise.ml @@ -650,9 +650,9 @@ let desc_divide_by_zero expr_str loc = (at_line tags loc) in { no_desc with descriptions = [description]; tags = !tags } -let desc_empty_vector_access pname object_str loc = +let desc_empty_vector_access pname_opt object_str loc = let vector_str = Format.sprintf "Vector %s" object_str in - let desc = access_str_empty (Some pname) in + let desc = access_str_empty pname_opt in let tags = desc.tags in Tags.add tags Tags.empty_vector_access object_str; let descriptions = [vector_str; desc.problem_str; (at_line tags loc)] in diff --git a/infer/src/backend/localise.mli b/infer/src/backend/localise.mli index 3301b6cdf..8d94985b8 100644 --- a/infer/src/backend/localise.mli +++ b/infer/src/backend/localise.mli @@ -203,7 +203,7 @@ val desc_deallocate_static_memory : string -> Procname.t -> Location.t -> error_ val desc_divide_by_zero : string -> Location.t -> error_desc -val desc_empty_vector_access : Procname.t -> string -> Location.t -> error_desc +val desc_empty_vector_access : Procname.t option -> string -> Location.t -> error_desc val is_empty_vector_access_desc : error_desc -> bool diff --git a/infer/tests/codetoanalyze/cpp/errors/vector/empty_access.cpp b/infer/tests/codetoanalyze/cpp/errors/vector/empty_access.cpp index 71d36f234..ce855608c 100644 --- a/infer/tests/codetoanalyze/cpp/errors/vector/empty_access.cpp +++ b/infer/tests/codetoanalyze/cpp/errors/vector/empty_access.cpp @@ -115,15 +115,37 @@ int vector_param_access(std::vector& v) { return v.back(); // shouldn't report anything here } -/* -// FIXME: analyzer reports it as NULL_DEREFERENCE (B5) instead -// of EMPTY_VECTOR_ACCESS int vector_as_param_empty() { std::vector v; return vector_param_access(v); -}*/ +} int vector_as_param_nonempty() { std::vector v(1); return vector_param_access(v); } + +int vector_param_by_value_access(std::vector v) { + return v.back(); // shouldn't report anything here +} + +int vector_as_param_by_value_empty() { + std::vector v; + return vector_param_by_value_access(v); +} + +void vector_param_by_value_clear(std::vector v) { v.clear(); } + +int vector_as_param_by_value_clear_no_crash() { + std::vector v(1); + vector_param_by_value_clear(v); + return v[0]; +} + +void vector_param_clear(std::vector& v) { v.clear(); } + +int vector_as_param_clear() { + std::vector v(1); + vector_param_clear(v); + return v[0]; +} diff --git a/infer/tests/endtoend/cpp/VectorEmptyAccessTest.java b/infer/tests/endtoend/cpp/VectorEmptyAccessTest.java index 079d6aeab..74f1b18db 100644 --- a/infer/tests/endtoend/cpp/VectorEmptyAccessTest.java +++ b/infer/tests/endtoend/cpp/VectorEmptyAccessTest.java @@ -55,7 +55,9 @@ public class VectorEmptyAccessTest { "assign_empty", "empty_check_access_empty", "size_check0_empty", - //"vector_as_param_empty", + "vector_as_param_empty", + "vector_as_param_clear", + "vector_as_param_by_value_empty" }; InferResults inferResults = InferRunner.runInferCPP(inferCmd); assertThat(