First extension to uninit inter-procedural analysis

Reviewed By: sblackshear

Differential Revision: D6511316

fbshipit-source-id: 239811e
master
Dino Distefano 7 years ago committed by Facebook Github Bot
parent 4795139459
commit d83a9445f1

@ -45,6 +45,7 @@ BUILD_SYSTEMS_TESTS += \
reactive \
run_hidden_linters \
tracebugs \
uninit \
utf8_in_procname \
DIRECT_TESTS += \

@ -1934,6 +1934,11 @@ and type_size =
"Consider the size of types during analysis, e.g. cannot use an int pointer to write to a char"
and uninit_interproc =
CLOpt.mk_bool ~long:"uninit-interproc"
"Run uninit check in the experimental interprocedural mode"
and unsafe_malloc =
CLOpt.mk_bool ~long:"unsafe-malloc"
~in_help:CLOpt.([(Analyze, manual_clang)])
@ -2688,6 +2693,8 @@ and type_size = !type_size
and uninit = !uninit
and uninit_interproc = !uninit_interproc
and unsafe_malloc = !unsafe_malloc
and whole_seconds = !whole_seconds

@ -678,6 +678,8 @@ val type_size : bool
val uninit : bool
val uninit_interproc : bool
val unsafe_malloc : bool
val whole_seconds : bool

@ -29,8 +29,6 @@ module Summary = Summary.Make (struct
let read_payload (summary: Specs.summary) = summary.payload.uninit
end)
let intraprocedural_only = true
let blacklisted_functions = [BuiltinDecl.__set_array_length]
let is_type_pointer t = match t.Typ.desc with Typ.Tptr _ -> true | _ -> false
@ -53,7 +51,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
module CFG = CFG
module Domain = RecordDomain
let report ap loc summary =
let report_intra ap loc summary =
let message = F.asprintf "The value read from %a was never initialized" AccessPath.pp ap in
let ltr = [Errlog.make_trace_element 0 loc "" []] in
let exn =
@ -64,6 +62,16 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
type extras = FormalMap.t * Specs.summary
let is_struct t = match t.Typ.desc with Typ.Tstruct _ -> true | _ -> false
let get_formals call =
match Ondemand.get_proc_desc call with
| Some proc_desc ->
Procdesc.get_formals proc_desc
| _ ->
[]
let should_report_var pdesc tenv uninit_vars ap =
match (AccessPath.get_typ ap tenv, ap) with
| Some typ, ((Var.ProgramVar pv, _), _) ->
@ -73,19 +81,37 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
false
let report_on_function_params pdesc tenv uninit_vars actuals loc extras =
List.iter
~f:(fun e ->
let nth_formal_param callee_pname idx =
let formals = get_formals callee_pname in
List.nth formals idx
let function_expects_a_pointer_as_nth_param callee_pname idx =
match nth_formal_param callee_pname idx with
| Some (_, typ) ->
is_type_pointer typ
| _ ->
false
let is_struct_field_passed_by_ref call t al idx =
is_struct t && List.length al > 0 && function_expects_a_pointer_as_nth_param call idx
let report_on_function_params call pdesc tenv uninit_vars actuals loc extras =
List.iteri
~f:(fun idx e ->
match e with
| HilExp.AccessPath ((var, t), al)
when should_report_var pdesc tenv uninit_vars ((var, t), al) && not (is_type_pointer t) ->
report ((var, t), al) loc (snd extras)
when should_report_var pdesc tenv uninit_vars ((var, t), al) && not (is_type_pointer t)
&& not (is_struct_field_passed_by_ref call t al idx) ->
report_intra ((var, t), al) loc (snd extras)
| _ ->
())
actuals
let remove_fields tenv base uninit_vars =
let remove_all_fields tenv base uninit_vars =
match base with
| _, {Typ.desc= Tptr ({Typ.desc= Tstruct name_struct}, _)} | _, {Typ.desc= Tstruct name_struct}
-> (
@ -100,19 +126,34 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
uninit_vars
let get_formals call =
match Ondemand.get_proc_desc call with
| Some proc_desc ->
Procdesc.get_formals proc_desc
let remove_init_fields base formal_var uninit_vars init_fields =
let subst_formal_actual_fields initialized_fields =
D.map
(fun ((v, t), a) ->
let v' = if Var.equal v formal_var then fst base else v in
let t' =
match t.desc with
| Typ.Tptr ({Typ.desc= Tstruct n}, _) ->
(* a pointer to struct needs to be changed into struct
as the actual is just type struct and it would make it
equality fail. Not sure why the actual are type struct when
passed by reference *)
{t with Typ.desc= Tstruct n}
| _ ->
t
in
((v', t'), a))
initialized_fields
in
match base with
| _, {Typ.desc= Tptr ({Typ.desc= Tstruct _}, _)} | _, {Typ.desc= Tstruct _} ->
D.diff uninit_vars (subst_formal_actual_fields init_fields)
| _ ->
[]
uninit_vars
let is_struct t = match Typ.name t with Some _ -> true | _ -> false
let function_expect_a_pointer call idx =
let formals = get_formals call in
match List.nth formals idx with Some (_, typ) -> is_type_pointer typ | _ -> false
let remove_array_element base uninit_vars =
D.remove (base, [AccessPath.ArrayAccess (snd base, [])]) uninit_vars
let is_dummy_constructor_of_a_struct call =
@ -126,36 +167,71 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
Typ.Procname.is_constructor call && is_dummy_constructor_of_struct
let is_pointer_assignment tenv lhs rhs =
HilExp.is_null_literal rhs
(* the rhs has type int when assigning the lhs to null *)
|| Option.equal Typ.equal (AccessPath.get_typ lhs tenv) (HilExp.get_typ tenv rhs)
&& is_type_pointer (snd (fst lhs))
(* checks that the set of initialized formal parameters defined in the precondition of
the function (init_formal_params) contains the (base of) nth formal parameter of the function *)
let init_nth_actual_param callee_pname idx init_formal_params =
match nth_formal_param callee_pname idx with
| None ->
None
| Some (fparam, t) ->
let var_fparam = Var.of_pvar (Pvar.mk fparam callee_pname) in
if D.exists
(fun (base, _) -> AccessPath.equal_base base (var_fparam, t))
init_formal_params
then Some var_fparam
else None
let remove_initialized_params pdesc call acc idx (base, al) remove_fields =
match Summary.read_summary pdesc call with
| Some {pre= initialized_formal_params; post= _} -> (
match init_nth_actual_param call idx initialized_formal_params with
| Some nth_formal ->
let acc' = D.remove (base, al) acc in
if remove_fields then remove_init_fields base nth_formal acc' initialized_formal_params
else acc'
| _ ->
acc )
| _ ->
acc
let exec_instr (astate: Domain.astate) {ProcData.pdesc; ProcData.extras; ProcData.tenv} _
(instr: HilInstr.t) =
let update_prepost (((_, lhs_typ), apl) as lhs_ap) rhs =
if FormalMap.is_formal (fst lhs_ap) (fst extras) && is_type_pointer lhs_typ
&& (not (is_pointer_assignment tenv lhs_ap rhs) || List.length apl > 0)
then
let pre' = D.add lhs_ap (fst astate.prepost) in
let post = snd astate.prepost in
(pre', post)
else astate.prepost
in
match instr with
| Assign
( (((lhs_var, lhs_typ), apl) as lhs_ap)
, HilExp.AccessPath (((_, rhs_typ) as rhs_base), al)
, loc ) ->
| Assign ((((lhs_var, lhs_typ), apl) as lhs_ap), (HilExp.AccessPath (rhs_base, al) as rhs), loc) ->
let uninit_vars' = D.remove lhs_ap astate.uninit_vars in
let uninit_vars =
if Int.equal (List.length apl) 0 then
(* if we assign to the root of a struct then we need to remove all the fields *)
remove_fields tenv (lhs_var, lhs_typ) uninit_vars'
remove_all_fields tenv (lhs_var, lhs_typ) uninit_vars'
else uninit_vars'
in
let prepost =
if FormalMap.is_formal rhs_base (fst extras)
&& match rhs_typ.desc with Typ.Tptr _ -> true | _ -> false
then
let pre' = D.add (rhs_base, al) (fst astate.prepost) in
let post = snd astate.prepost in
(pre', post)
else astate.prepost
in
let prepost = update_prepost lhs_ap rhs in
(* check on lhs_typ to avoid false positive when assigning a pointer to another *)
if should_report_var pdesc tenv uninit_vars (rhs_base, al) && not (is_type_pointer lhs_typ)
then report (rhs_base, al) loc (snd extras) ;
then report_intra (rhs_base, al) loc (snd extras) ;
{astate with uninit_vars; prepost}
| Assign (lhs, _, _) ->
| Assign (((lhs_ap, apl) as lhs), rhs, _) ->
let uninit_vars = D.remove lhs astate.uninit_vars in
{astate with uninit_vars}
let prepost = update_prepost (lhs_ap, apl) rhs in
{astate with uninit_vars; prepost}
| Call (_, Direct callee_pname, _, _, _)
when Typ.Procname.equal callee_pname BuiltinDecl.objc_cpp_throw ->
{astate with uninit_vars= D.empty}
@ -172,14 +248,19 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
when is_blacklisted_function call ->
D.remove (base, al) acc
| HilExp.AccessPath (((_, t) as base), al)
when is_struct t && List.length al > 0 && function_expect_a_pointer call idx ->
when is_struct_field_passed_by_ref call t al idx ->
(* Access to a field of a struct by reference *)
D.remove (base, al) acc
if Config.uninit_interproc then
remove_initialized_params pdesc call acc idx (base, al) false
else D.remove (base, al) acc
| HilExp.AccessPath ap when Typ.Procname.is_constructor call ->
remove_fields tenv (fst ap) (D.remove ap acc)
remove_all_fields tenv (fst ap) (D.remove ap acc)
| HilExp.AccessPath (((_, {Typ.desc= Tptr _}) as base), al) ->
let acc' = D.remove (base, al) acc in
remove_fields tenv base acc'
if Config.uninit_interproc then
remove_initialized_params pdesc call acc idx (base, al) true
else
let acc' = D.remove (base, al) acc in
remove_all_fields tenv base acc'
| HilExp.Closure (_, apl) ->
(* remove the captured variables of a block/lambda *)
List.fold ~f:(fun acc' (base, _) -> D.remove (base, []) acc') ~init:acc apl
@ -187,7 +268,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
acc)
~init:astate.uninit_vars actuals
in
report_on_function_params pdesc tenv uninit_vars actuals loc extras ;
report_on_function_params call pdesc tenv uninit_vars actuals loc extras ;
{astate with uninit_vars}
| Call _ | Assume _ ->
astate
@ -251,3 +332,4 @@ let checker {Callbacks.tenv; summary; proc_desc} : Specs.summary =
(Procdesc.get_proc_name proc_desc) ;
summary )
else summary

@ -14,6 +14,8 @@ module L = Logging
(** Forward analysis to compute uninitialized variables at each program point *)
module Domain = AbstractDomain.InvertedSet (AccessPath)
(* pre = set of parameters initialized inside the procedure;
post = set of uninit local variables of the procedure *)
type summary = {pre: Domain.t; post: Domain.t}
module VarPair = struct

@ -14,11 +14,10 @@ SOURCES = $(CODETOANALYZE_DIR)/A.m $(CODETOANALYZE_DIR)/B.m
OBJECTS = $(CODETOANALYZE_DIR)/A.o $(CODETOANALYZE_DIR)/B.o
INFER_OPTIONS = --biabduction-only --report-custom-error --developer-mode --project-root $(TESTS_DIR)
INFERPRINT_OPTIONS = --project-root $(TESTS_DIR) --issues-tests
CLEAN_EXTRA = -c
include $(TESTS_DIR)/infer.make
infer-out/report.json: $(CLANG_DEPS) $(SOURCES) $(MAKEFILE_LIST)
$(QUIET)$(REMOVE_DIR) buck-out && \
$(call silent_on_success,Testing analysis with Objective-C getters and setters,\
$(INFER_BIN) $(INFER_OPTIONS) --results-dir $(CURDIR)/infer-out -- clang $(CLEAN_EXTRA) $(SOURCES))
$(INFER_BIN) $(INFER_OPTIONS) --results-dir $(CURDIR)/infer-out -- clang -c $(SOURCES))

@ -0,0 +1,25 @@
# Copyright (c) 2017 - 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
# see explanations in cpp/errors/Makefile for the custom isystem
CLANG_OPTIONS = -x c++ -std=c++11 -nostdinc++ -isystem$(MODELS_DIR)/cpp/include -isystem$(CLANG_INCLUDES)/c++/v1/ -c
INFER_OPTIONS = --uninit-only --uninit-interproc --debug-exceptions --project-root $(TESTS_DIR)
INFERPRINT_OPTIONS = --issues-tests
CLEAN_EXTRA = -c
SOURCES = $(wildcard *.cpp)
include $(TESTS_DIR)/infer.make
infer-out/report.json: $(MAKEFILE_LIST)
infer-out/report.json: $(CLANG_DEPS) $(SOURCES) $(MAKEFILE_LIST)
$(QUIET)$(REMOVE_DIR) buck-out && \
$(call silent_on_success,Testing uninit analysis with inter-procedural mode,\
$(INFER_BIN) $(INFER_OPTIONS) --results-dir $(CURDIR)/infer-out -- clang $(CLEAN_EXTRA) $(SOURCES))

@ -0,0 +1,195 @@
/*
* Copyright (c) 2017 - 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.
*/
struct is {
int n;
int b;
};
void i_init(int* i) { *i = 10; }
void i_no_init(int* i) { i = 0; }
// error is detected before call if x was not init
int i_inc(int x) { return x++; }
int no_init_in_callee_bad() {
int a;
int b = 0;
i_no_init(&a);
b = a; // error
return b;
}
int init_in_calee_ok() {
int a;
int b = 0;
i_init(&a);
b = a; // OK
return b;
}
int no_init_field_in_calee_bad() {
struct is t;
int b = 0;
i_no_init(&t.n);
b = t.n; // error
return b;
}
int init_field_in_calee_ok() {
struct is t;
int b = 0;
i_init(&t.n);
b = t.n; // OK
return b;
}
int no_init_in_calee_bad2() {
int a;
int c = 0;
i_no_init(&a);
c = i_inc(a); // error
return c;
}
int init_in_calee_ok2() {
int a;
int c = 0;
i_init(&a);
c = i_inc(a); // ok
}
int i_no_init_return_bad() {
int x;
return x; // error
}
// this shows that in case a function return an uninit value, it gets the
// blame / rather than the caller.
int blame_on_callee() {
int a;
int c = i_no_init_return_bad();
a = c; // we don't flag the error here as it is flagged in no_init_return
// definition
return 0;
}
void i_maybe_init(int y, int* formal) {
if (y == 0) {
*formal = 5;
};
}
void i_must_init_ok(int y, int* formal) {
if (y == 0) {
*formal = 5;
} else {
*formal = 17;
};
}
int i_call_maybe_init_bad(int y) {
int x;
i_maybe_init(y, &x);
return x;
}
int i_call_must_init_ok(int y) {
int x;
i_must_init_ok(y, &x);
return x;
}
void i_square_init(int x, int& res) { res = x * x; }
int i_square_no_init(int x, int& res) { return res * res; }
void i_use_square_OK() {
int i;
i_square_init(2, i);
}
void FN_use_square_bad() {
int i;
i = i_square_no_init(2, i); // We should report here
}
int i_no_deref(int* x) {
int* y = 0;
x = y;
return *x; // this is not actually a deref of a formal
}
void i_init_x(int* x) {
int* y;
y = x;
*y = 25; // this is writing x. Need aliasing info
}
int FP_use_init_x_OK() {
int a;
i_init_x(&a);
return a; // We should not report here
}
struct s {
int n;
int b;
};
void init_some_field_of_struct(struct s* z) { z->n = 10; };
void init_full_struct(struct s* z) {
z->n = 10;
z->b = 17;
};
int call_init_some_field_of_struct_ok() {
struct s t;
init_some_field_of_struct(&t);
return t.n;
}
int call_init_some_field_of_struct_bad() {
struct s t;
init_some_field_of_struct(&t);
return t.b;
}
int call_init_full_struct_ok() {
struct s t;
init_full_struct(&t);
int i = t.n;
int b = t.b;
return 0;
}

@ -0,0 +1,7 @@
build_systems/uninit/inter_proc_uninit.cpp, FP_use_init_x_OK, 5, UNINITIALIZED_VALUE, []
build_systems/uninit/inter_proc_uninit.cpp, call_init_some_field_of_struct_bad, 4, UNINITIALIZED_VALUE, []
build_systems/uninit/inter_proc_uninit.cpp, i_call_maybe_init_bad, 3, UNINITIALIZED_VALUE, []
build_systems/uninit/inter_proc_uninit.cpp, i_no_init_return_bad, 2, UNINITIALIZED_VALUE, []
build_systems/uninit/inter_proc_uninit.cpp, no_init_field_in_calee_bad, 6, UNINITIALIZED_VALUE, []
build_systems/uninit/inter_proc_uninit.cpp, no_init_in_calee_bad2, 6, UNINITIALIZED_VALUE, []
build_systems/uninit/inter_proc_uninit.cpp, no_init_in_callee_bad, 6, UNINITIALIZED_VALUE, []

@ -13,7 +13,7 @@ CLANG_OPTIONS = -x c++ -std=c++11 -nostdinc++ -isystem$(MODELS_DIR)/cpp/include
INFER_OPTIONS = --uninit-only --debug-exceptions --project-root $(TESTS_DIR)
INFERPRINT_OPTIONS = --issues-tests
SOURCES = uninit.cpp struct.cpp
SOURCES = $(wildcard *.cpp)
include $(TESTS_DIR)/clang.make

@ -1,199 +0,0 @@
/*
* Copyright (c) 2017 - 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.
*/
void init(int* i) { *i = 10; }
void init_bool(bool* i) { *i = false; }
void no_init(int* i) { i = 0; }
void no_init_bool(bool* i) { i = 0; }
int inc(int x) { return x++; }
// error is detected before call as we copy x
// so no need to put it in the summary
int no_init_return_bad() {
int x;
return x; // error
}
void bad1() {
int a;
int b = a; // Error
int c = b; // Error but we do not report as it depends from line 20
}
int bad2() {
int a;
int b = 0;
int c = 0;
no_init(&a);
b = a; // error
}
int bad3() {
int a;
int b = 0;
int c = 0;
no_init(&a);
c = inc(a); // error
}
int ok1() {
int a;
int b = 0;
int c = 0;
init(&a);
c = a; // OK
}
int ok2() {
int a;
int b = 0;
int c = 0;
init(&a);
c = inc(a); // ok
}
int bad4() {
int a;
int b = 0;
int c = 0;
no_init(&a);
b = a; // report here error
c = inc(b); // do not report as it depends from line 31
return 0;
}
// this function shows that we correctly reportat
// line 88 but not report the error at line 90
int bad5() {
int a;
int b = 0;
int c = 0;
no_init(&a);
b = a; // error
return b; // should not report as it depends from line 31
}
// this shows that in case a function return an uninit value, it gets the blame
// rather than the caller.
int blame_on_callee() {
int a;
int c = no_init_return_bad();
a = c; // we don't flag the error here as it is flagged in no_init_return
// definition
return 0;
}
void maybe_init(int y, int* formal) {
if (y == 0) {
*formal = 5;
};
}
void must_init(int y, int* formal) {
if (y == 0) {
*formal = 5;
} else {
*formal = 17;
};
}
int call_maybe_init_bad(int y) {
int x;
maybe_init(y, &x);
return x;
}
int call_must_init_ok(int y) {
int x;
must_init(y, &x);
return x;
}
void square_init(int x, int& res) { res = x * x; }
int square_no_init(int x, int& res) { return res * res; }
void use_square_OK() {
int i;
square_init(2, i);
}
void use_square_bad() {
int i;
i = square_no_init(2, i); // Error
}
int no_deref(int* x) {
int* y = 0;
x = y;
return *x; // this is not actually a deref of a formal
}
void init_x(int* x) {
int* y;
y = x;
*y = 25; // this is writing x
}
int use_init_x_OK() {
int a;
init_x(&a);
return a;
}
void bool1_bad() {
bool a;
bool b = a;
bool c = b;
}
int bool2_bad() {
bool a;
bool b = 0;
bool c = 0;
no_init_bool(&a);
b = a; // error
}
int bool1_ok() {
bool a;
bool b = 0;
bool c = 0;
init_bool(&a);
c = a; // OK
}

@ -1,3 +1,6 @@
codetoanalyze/cpp/uninit/members.cpp, access_members_bad, 4, UNINITIALIZED_VALUE, []
codetoanalyze/cpp/uninit/members.cpp, access_members_bad2, 4, UNINITIALIZED_VALUE, []
codetoanalyze/cpp/uninit/members.cpp, access_members_bad3, 4, UNINITIALIZED_VALUE, []
codetoanalyze/cpp/uninit/struct.cpp, pass_basic_type_field_bad, 3, UNINITIALIZED_VALUE, []
codetoanalyze/cpp/uninit/struct.cpp, struct_uninit_bad, 3, UNINITIALIZED_VALUE, []
codetoanalyze/cpp/uninit/uninit.cpp, FP_no_warning_noreturn_callee_ok, 7, UNINITIALIZED_VALUE, []

@ -65,7 +65,7 @@ int global_struct_ok() {
return j;
}
void init_struct(struct s*);
void init_struct(struct s* z) { z->n = 10; };
int call_init_struct_ok() {
struct s t;
@ -74,6 +74,13 @@ int call_init_struct_ok() {
return t.n;
}
int FN_call_init_struct() {
struct s t;
init_struct(&t);
return t.b;
}
struct s init_all_fields(void);
int init_field_via_function_ok() {

@ -31,28 +31,6 @@ int bad1() {
return c;
}
int ok1() {
int a;
int b;
no_init(&a);
b = a; // OK only for intraprocedural case (we assume that something passed by
// reference is initialized). When analysis extended to
// interprocedural, it should report a warning.
return b;
}
int ok2() {
int a;
int c;
no_init(&a);
c = inc(a); // OK only for intraprocedural case (we assume that something
// passed by reference is initialized). When analysis extended to
// interprocedural, it should report a warning.
return c;
}
int ok3() {
int a;
int c;
@ -75,22 +53,6 @@ int ok4() {
return c;
}
int ok5() {
int a;
int b;
int c;
no_init(&a);
b = a; // OK only for intraprocedural case (we assume that something passed by
// reference is initialized). When analysis extended to
// interprocedural, it should report a warning.
c = inc(b); // do not report as it depends from line above
return c;
}
void square_init(int x, int& res) { res = x * x; }
int square_no_init(int x, int& res) { return res * res; }

Loading…
Cancel
Save