diff --git a/Makefile b/Makefile index ca7fd8143..f9e4442e0 100644 --- a/Makefile +++ b/Makefile @@ -48,7 +48,7 @@ BUILD_SYSTEMS_TESTS += \ utf8_in_procname \ DIRECT_TESTS += \ - c_biabduction c_bufferoverrun c_errors c_frontend c_performance \ + c_biabduction c_bufferoverrun c_errors c_frontend c_performance c_uninit \ cpp_bufferoverrun cpp_errors cpp_frontend cpp_liveness cpp_ownership cpp_quandary \ cpp_racerd cpp_siof cpp_uninit cpp_nullable cpp_conflicts cpp_linters-for-test-only \ diff --git a/infer/src/checkers/uninit.ml b/infer/src/checkers/uninit.ml index 5ca5015fa..8522941c5 100644 --- a/infer/src/checkers/uninit.ml +++ b/infer/src/checkers/uninit.ml @@ -100,7 +100,12 @@ module TransferFunctions (CFG : ProcCfg.S) = struct && function_expects_a_pointer_as_nth_param call idx - let report_on_function_params call pdesc tenv uninit_vars actuals loc extras = + let is_fld_or_array_elem_passed_by_ref t access_expr idx call = + is_struct_field_passed_by_ref call t access_expr idx + || is_array_element_passed_by_ref call t access_expr idx + + + let report_on_function_params pdesc tenv uninit_vars actuals loc extras call = List.iteri ~f:(fun idx e -> match e with @@ -290,9 +295,10 @@ module TransferFunctions (CFG : ProcCfg.S) = struct in {astate with uninit_vars= uninit_vars'} else astate - | Call (_, HilInstr.Direct call, actuals, _, loc) -> + | Call (_, call, actuals, _, loc) -> (* in case of intraprocedural only analysis we assume that parameters passed by reference to a function will be initialized inside that function *) + let pname_opt = match call with Direct pname -> Some pname | Indirect _ -> None in let uninit_vars = List.foldi ~init:astate.uninit_vars actuals ~f:(fun idx acc actual_exp -> match actual_exp with @@ -302,23 +308,29 @@ module TransferFunctions (CFG : ProcCfg.S) = struct match access_expr with AddressOf ae -> ae | _ -> access_expr in match AccessExpression.get_base access_expr with - | _, {Typ.desc= Tarray _} when is_blacklisted_function call -> + | _, {Typ.desc= Tarray _} + when Option.value_map ~default:false ~f:is_blacklisted_function pname_opt -> D.remove access_expr acc | _, t - when is_struct_field_passed_by_ref call t access_expr idx - || is_array_element_passed_by_ref call t access_expr idx -> - (* Access to a field of a struct by reference *) - if Config.uninit_interproc then - remove_initialized_params pdesc call acc idx access_expr_to_remove false - else D.remove access_expr_to_remove acc - | base when Typ.Procname.is_constructor call -> + (* Access to a field of a struct or an element of an array by reference *) + when Option.value_map ~default:false + ~f:(is_fld_or_array_elem_passed_by_ref t access_expr idx) + pname_opt -> ( + match pname_opt with + | Some pname when Config.uninit_interproc -> + remove_initialized_params pdesc pname acc idx access_expr_to_remove false + | _ -> + D.remove access_expr_to_remove acc ) + | base + when Option.value_map ~default:false ~f:Typ.Procname.is_constructor pname_opt -> remove_all_fields tenv base (D.remove access_expr_to_remove acc) - | (_, {Typ.desc= Tptr _}) as base -> - if Config.uninit_interproc then - remove_initialized_params pdesc call acc idx access_expr_to_remove true - else + | (_, {Typ.desc= Tptr _}) as base -> ( + match pname_opt with + | Some pname when Config.uninit_interproc -> + remove_initialized_params pdesc pname acc idx access_expr_to_remove true + | _ -> D.remove access_expr_to_remove acc |> remove_all_fields tenv base - |> remove_all_array_elements base |> remove_dereference_access base + |> remove_all_array_elements base |> remove_dereference_access base ) | _ -> acc ) | HilExp.Closure (_, apl) -> @@ -329,9 +341,11 @@ module TransferFunctions (CFG : ProcCfg.S) = struct | _ -> acc ) in - report_on_function_params call pdesc tenv uninit_vars actuals loc extras ; + Option.value_map ~default:() + ~f:(report_on_function_params pdesc tenv uninit_vars actuals loc extras) + pname_opt ; {astate with uninit_vars} - | Call _ | Assume _ -> + | Assume _ -> astate diff --git a/infer/tests/codetoanalyze/c/uninit/Makefile b/infer/tests/codetoanalyze/c/uninit/Makefile new file mode 100644 index 000000000..2b48b6edd --- /dev/null +++ b/infer/tests/codetoanalyze/c/uninit/Makefile @@ -0,0 +1,17 @@ +# Copyright (c) 2018 - present Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +TESTS_DIR = ../../.. + +ANALYZER = checkers +CLANG_OPTIONS = -c +INFER_OPTIONS = -F --project-root $(TESTS_DIR) --uninit-only +INFERPRINT_OPTIONS = --issues-tests + +SOURCES = $(wildcard *.c) + +include $(TESTS_DIR)/clang.make diff --git a/infer/tests/codetoanalyze/c/uninit/issues.exp b/infer/tests/codetoanalyze/c/uninit/issues.exp new file mode 100644 index 000000000..05ad7a847 --- /dev/null +++ b/infer/tests/codetoanalyze/c/uninit/issues.exp @@ -0,0 +1 @@ +codetoanalyze/c/uninit/uninit.c, dereference_bad, 2, UNINITIALIZED_VALUE, ERROR, [] diff --git a/infer/tests/codetoanalyze/c/uninit/uninit.c b/infer/tests/codetoanalyze/c/uninit/uninit.c new file mode 100644 index 000000000..1acff5ebb --- /dev/null +++ b/infer/tests/codetoanalyze/c/uninit/uninit.c @@ -0,0 +1,24 @@ +/* + * Copyright (c) 2018 - present Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +typedef struct A { + int field; +} B; + +typedef B (*entry_fn)(); + +int test_higher_order(entry_fn f) { + B event = f(); + return event.field; +} + +int dereference_bad() { + int* p; + return *p; +}