From d7d3e16d6f4ae814a640be7edbd3f5b80db2d2d2 Mon Sep 17 00:00:00 2001 From: Daiva Naudziuniene Date: Thu, 31 May 2018 03:00:18 -0700 Subject: [PATCH] [uninit] Enable reporting on uninitialized pointers Reviewed By: jeremydubreil Differential Revision: D6567381 fbshipit-source-id: c35f6e3 --- infer/src/checkers/uninit.ml | 12 ++++----- .../tests/codetoanalyze/cpp/uninit/issues.exp | 6 +++++ .../codetoanalyze/cpp/uninit/members.cpp | 12 +++++++++ .../tests/codetoanalyze/cpp/uninit/uninit.cpp | 25 ++++++++++++++++--- .../codetoanalyze/objc/uninit/issues.exp | 1 + .../tests/codetoanalyze/objc/uninit/uninit.m | 16 ++++++++++++ 6 files changed, 63 insertions(+), 9 deletions(-) diff --git a/infer/src/checkers/uninit.ml b/infer/src/checkers/uninit.ml index 0b45ae0d6..7cb7cd5fc 100644 --- a/infer/src/checkers/uninit.ml +++ b/infer/src/checkers/uninit.ml @@ -28,12 +28,12 @@ end) let blacklisted_functions = [BuiltinDecl.__set_array_length] -let rec is_basic_type t = +let should_report_on_type t = match t.Typ.desc with - | Tint _ | Tfloat _ | Tvoid -> + | Tptr (_, Pk_reference) -> + false + | Tint _ | Tfloat _ | Tvoid | Tptr _ -> true - | Tptr (t', _) -> - is_basic_type t' | _ -> false @@ -76,7 +76,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct match (AccessExpression.get_typ access_expr tenv, base) with | Some typ, (Var.ProgramVar pv, _) -> not (Pvar.is_frontend_tmp pv) && not (Procdesc.is_captured_var pdesc pv) - && D.mem access_expr uninit_vars && is_basic_type typ + && D.mem access_expr uninit_vars && should_report_on_type typ | _, _ -> false @@ -271,7 +271,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct | None -> false | Some lhs_ap_typ -> - not (Typ.is_pointer lhs_ap_typ) + not (Typ.is_reference lhs_ap_typ) in ( match rhs_expr with | AccessExpression rhs_access_expr -> diff --git a/infer/tests/codetoanalyze/cpp/uninit/issues.exp b/infer/tests/codetoanalyze/cpp/uninit/issues.exp index f346faa84..e95e248d7 100644 --- a/infer/tests/codetoanalyze/cpp/uninit/issues.exp +++ b/infer/tests/codetoanalyze/cpp/uninit/issues.exp @@ -3,12 +3,18 @@ codetoanalyze/cpp/uninit/default_constr.cpp, init_bad, 3, UNINITIALIZED_VALUE, E codetoanalyze/cpp/uninit/members.cpp, access_members_bad, 4, UNINITIALIZED_VALUE, ERROR, [] codetoanalyze/cpp/uninit/members.cpp, access_members_bad2, 4, UNINITIALIZED_VALUE, ERROR, [] codetoanalyze/cpp/uninit/members.cpp, access_members_bad3, 4, UNINITIALIZED_VALUE, ERROR, [] +codetoanalyze/cpp/uninit/members.cpp, access_pointer_members_bad, 3, UNINITIALIZED_VALUE, ERROR, [] +codetoanalyze/cpp/uninit/members.cpp, access_pointer_members_bad, 4, UNINITIALIZED_VALUE, ERROR, [] +codetoanalyze/cpp/uninit/members.cpp, access_pointer_members_bad, 5, UNINITIALIZED_VALUE, ERROR, [] codetoanalyze/cpp/uninit/struct.cpp, pass_basic_type_field_bad, 3, UNINITIALIZED_VALUE, ERROR, [] codetoanalyze/cpp/uninit/struct.cpp, struct_uninit_bad, 3, UNINITIALIZED_VALUE, ERROR, [] codetoanalyze/cpp/uninit/uninit.cpp, FP_no_warning_noreturn_callee_ok, 7, UNINITIALIZED_VALUE, ERROR, [] +codetoanalyze/cpp/uninit/uninit.cpp, FP_pointer_param_void_star_ok, 4, UNINITIALIZED_VALUE, ERROR, [] +codetoanalyze/cpp/uninit/uninit.cpp, FP_union_ok, 6, UNINITIALIZED_VALUE, ERROR, [] codetoanalyze/cpp/uninit/uninit.cpp, bad1, 2, UNINITIALIZED_VALUE, ERROR, [] codetoanalyze/cpp/uninit/uninit.cpp, bad2, 2, UNINITIALIZED_VALUE, ERROR, [] codetoanalyze/cpp/uninit/uninit.cpp, branch1_FP, 11, UNINITIALIZED_VALUE, ERROR, [] +codetoanalyze/cpp/uninit/uninit.cpp, copy_pointer_bad, 3, UNINITIALIZED_VALUE, ERROR, [] codetoanalyze/cpp/uninit/uninit.cpp, loop1_FP, 10, UNINITIALIZED_VALUE, ERROR, [] codetoanalyze/cpp/uninit/uninit.cpp, no_init_return_bad, 2, UNINITIALIZED_VALUE, ERROR, [] codetoanalyze/cpp/uninit/uninit.cpp, ret_undef_bad, 2, UNINITIALIZED_VALUE, ERROR, [] diff --git a/infer/tests/codetoanalyze/cpp/uninit/members.cpp b/infer/tests/codetoanalyze/cpp/uninit/members.cpp index 8742faf06..bb4c3b607 100644 --- a/infer/tests/codetoanalyze/cpp/uninit/members.cpp +++ b/infer/tests/codetoanalyze/cpp/uninit/members.cpp @@ -11,7 +11,9 @@ class A { public: + int* ptr; std::string a_name; + std::string* p_a_name; int i; A() {} // user defined default constructor. @@ -63,3 +65,13 @@ int access_members_bad3() { return 0; }; + +int access_pointer_members_bad() { + A a; + + int* p = a.ptr; + int i = a.i; + std::string* pn = a.p_a_name; + + return 0; +}; diff --git a/infer/tests/codetoanalyze/cpp/uninit/uninit.cpp b/infer/tests/codetoanalyze/cpp/uninit/uninit.cpp index c91587efa..9b295c9ee 100644 --- a/infer/tests/codetoanalyze/cpp/uninit/uninit.cpp +++ b/infer/tests/codetoanalyze/cpp/uninit/uninit.cpp @@ -181,11 +181,11 @@ int ret_undef_bad() { return *p; // report as p was not initialized } -int ret_undef_FP() { +int copy_pointer_bad() { int* p; int* q; - p = q; // no report as we copy an address - return *p; // NO report as we don't keep track of aliasing (for now) + p = q; // error + return *p; } void use_an_int2(int*); @@ -263,3 +263,22 @@ int FP_no_warning_noreturn_callee_ok(bool t) { } return x; } + +void some_f(void* p); + +int* FP_pointer_param_void_star_ok() { + A a; + int* res; + some_f(&a); // the type of a here is void*, hence no fields are found + return a.ptr; // false positive +} + +short FP_union_ok() { + union { + int* a; + short* b; + } u; + init(u.a); + short* p = u.b; // false positive + return *p; +} diff --git a/infer/tests/codetoanalyze/objc/uninit/issues.exp b/infer/tests/codetoanalyze/objc/uninit/issues.exp index 145bff75c..8757b6588 100644 --- a/infer/tests/codetoanalyze/objc/uninit/issues.exp +++ b/infer/tests/codetoanalyze/objc/uninit/issues.exp @@ -1,4 +1,5 @@ codetoanalyze/objc/uninit/arrays.m, array_length_undef_bad, 2, UNINITIALIZED_VALUE, ERROR, [] codetoanalyze/objc/uninit/arrays.m, bad1, 3, UNINITIALIZED_VALUE, ERROR, [] codetoanalyze/objc/uninit/arrays.m, bad2, 2, UNINITIALIZED_VALUE, ERROR, [] +codetoanalyze/objc/uninit/uninit.m, FP_switch_ok, 11, UNINITIALIZED_VALUE, ERROR, [] codetoanalyze/objc/uninit/uninit_blocks.m, A_bad1, 5, UNINITIALIZED_VALUE, ERROR, [] diff --git a/infer/tests/codetoanalyze/objc/uninit/uninit.m b/infer/tests/codetoanalyze/objc/uninit/uninit.m index 17404fa8c..8fb66a451 100644 --- a/infer/tests/codetoanalyze/objc/uninit/uninit.m +++ b/infer/tests/codetoanalyze/objc/uninit/uninit.m @@ -30,4 +30,20 @@ return labelSize.height; // Here we should not report uninit } +typedef NS_ENUM(NSUInteger, SomeEnum) { ValueA, ValueB }; + +CGColorRef FP_switch_ok(SomeEnum e, CGColorRef defaultcolor) { + CGColorRef color; + + switch (e) { + case ValueA: + color = defaultcolor; + break; + case ValueB: + color = defaultcolor; + break; + } + return color; // false positive because of exausted switch +} + @end