From 2991bd3fc3d983a17cf1082ab3ac85d929ec9280 Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Thu, 17 Mar 2016 10:12:51 -0700 Subject: [PATCH] using address-taken analysis for nullify placement rather than alias analysis Reviewed By: jeremydubreil Differential Revision: D3030423 fb-gh-sync-id: 447b040 shipit-source-id: 447b040 --- infer/src/backend/preanal.ml | 24 +++++++++++-------- .../c/errors/local_vars/local_vars.c | 12 ++++++++++ .../cpp/frontend/types/typeid_expr.cpp.dot | 20 ++++++++-------- .../codetoanalyze/objc/errors/npe/block.m | 9 +++++++ 4 files changed, 45 insertions(+), 20 deletions(-) diff --git a/infer/src/backend/preanal.ml b/infer/src/backend/preanal.ml index 980921ff7..19c5631b4 100644 --- a/infer/src/backend/preanal.ml +++ b/infer/src/backend/preanal.ml @@ -111,18 +111,21 @@ let rec def_instr cfg (instr: Sil.instr) acc = and def_instrl cfg instrs acc = IList.fold_left (fun acc' i -> def_instr cfg i acc') acc instrs -(* computes the addresses that are assigned to something or passed as parameters to*) -(* a functions. These will be considered becoming possibly aliased *) +(* computes the addresses that are assigned to something or passed as parameters to a function. + these will be considered aliased. *) let aliasing_instr cfg pdesc (instr: Sil.instr) acc = match instr with - | Sil.Set (_, _, e, _) -> use_exp cfg pdesc e acc + | Sil.Set (_, Sil.Tptr (_, _), exp, _) -> + use_exp cfg pdesc exp acc | Sil.Call (_, _, argl, _, _) -> - let argl'= fst (IList.split argl) in - IList.fold_left (fun acc' e' -> use_exp cfg pdesc e' acc') acc argl' - | Sil.Letderef _ | Sil.Prune _ -> acc - | Sil.Nullify _ -> acc - | Sil.Abstract _ | Sil.Remove_temps _ | Sil.Stackop _ | Sil.Declare_locals _ -> acc - | Sil.Goto_node _ -> acc + IList.fold_left + (fun acc (arg, typ) -> match typ with + | Sil.Tptr _ -> use_exp cfg pdesc arg acc + | _ -> acc) + acc + argl + | Sil.Set _ | Sil.Letderef _ | Sil.Prune _ | Sil.Nullify _ | Sil.Abstract _ | Sil.Remove_temps _ + | Sil.Stackop _ | Sil.Declare_locals _ | Sil.Goto_node _ -> acc (* computes possible alisased var *) let def_aliased_var cfg pdesc instrs acc = @@ -226,7 +229,8 @@ let analyze_proc cfg pdesc cand = try while true do let node = Worklist.pick () in - aliased_var := Vset.union (compute_aliased cfg node !aliased_var) !aliased_var; + if not (!Config.curr_language = Config.Java) then + aliased_var := Vset.union (compute_aliased cfg node !aliased_var) !aliased_var; let curr_live = Table.get_live node in let preds = AllPreds.get_preds node in let live_at_predecessors = diff --git a/infer/tests/codetoanalyze/c/errors/local_vars/local_vars.c b/infer/tests/codetoanalyze/c/errors/local_vars/local_vars.c index 20e081c12..c0281e6a2 100644 --- a/infer/tests/codetoanalyze/c/errors/local_vars/local_vars.c +++ b/infer/tests/codetoanalyze/c/errors/local_vars/local_vars.c @@ -49,3 +49,15 @@ int t() { } return 0; } + +int address_taken() { + int** x; + int* y; + int i = 7; + y = &i; + x = &y; + // if we don't reason about taken addresses while adding nullify instructions, + // we'll add + // `nullify(y)` here and report a false NPE on the next line + return **x; +} diff --git a/infer/tests/codetoanalyze/cpp/frontend/types/typeid_expr.cpp.dot b/infer/tests/codetoanalyze/cpp/frontend/types/typeid_expr.cpp.dot index 9eda0cbd7..5aabf539c 100644 --- a/infer/tests/codetoanalyze/cpp/frontend/types/typeid_expr.cpp.dot +++ b/infer/tests/codetoanalyze/cpp/frontend/types/typeid_expr.cpp.dot @@ -134,7 +134,7 @@ digraph iCFG { 32 -> 31 ; -31 [label="31: DeclStmt \n n$5=_fun___cxx_typeid(sizeof(class std::type_info ):void ,n$5.__type_name:void ,&t:int ) [line 31]\n n$6=*n$5:class std::type_info [line 31]\n n$7=_fun_std::type_info_name(n$5:class std::type_info &) [line 31]\n *&t_type_info:char *=n$7 [line 31]\n REMOVE_TEMPS(n$5,n$6,n$7); [line 31]\n " shape="box"] +31 [label="31: DeclStmt \n n$5=_fun___cxx_typeid(sizeof(class std::type_info ):void ,n$5.__type_name:void ,&t:int ) [line 31]\n n$6=*n$5:class std::type_info [line 31]\n n$7=_fun_std::type_info_name(n$5:class std::type_info &) [line 31]\n *&t_type_info:char *=n$7 [line 31]\n REMOVE_TEMPS(n$5,n$6,n$7); [line 31]\n NULLIFY(&t,false); [line 31]\n " shape="box"] 31 -> 30 ; @@ -142,11 +142,11 @@ digraph iCFG { 30 -> 25 ; -29 [label="29: Return Stmt \n *&return:int =(1 / 0) [line 36]\n NULLIFY(&person,false); [line 36]\n NULLIFY(&t,false); [line 36]\n APPLY_ABSTRACTION; [line 36]\n " shape="box"] +29 [label="29: Return Stmt \n *&return:int =(1 / 0) [line 36]\n NULLIFY(&person,false); [line 36]\n APPLY_ABSTRACTION; [line 36]\n " shape="box"] 29 -> 23 ; -28 [label="28: Return Stmt \n *&return:int =0 [line 34]\n NULLIFY(&person,false); [line 34]\n NULLIFY(&t,false); [line 34]\n APPLY_ABSTRACTION; [line 34]\n " shape="box"] +28 [label="28: Return Stmt \n *&return:int =0 [line 34]\n NULLIFY(&person,false); [line 34]\n APPLY_ABSTRACTION; [line 34]\n " shape="box"] 28 -> 23 ; @@ -163,14 +163,14 @@ digraph iCFG { 25 -> 26 ; 25 -> 27 ; -24 [label="24: + \n NULLIFY(&person_type_info,false); [line 33]\n NULLIFY(&t_type_info,false); [line 33]\n NULLIFY(&person,false); [line 33]\n NULLIFY(&t,false); [line 33]\n " ] +24 [label="24: + \n NULLIFY(&person_type_info,false); [line 33]\n NULLIFY(&t,false); [line 33]\n NULLIFY(&t_type_info,false); [line 33]\n NULLIFY(&person,false); [line 33]\n " ] 24 -> 23 ; 23 [label="23: Exit person_typeid_name \n " color=yellow style=filled] -22 [label="22: Start person_typeid_name\nFormals: \nLocals: person_type_info:char * t_type_info:char * t:int person:class Person \n DECLARE_LOCALS(&return,&person_type_info,&t_type_info,&t,&person); [line 28]\n NULLIFY(&person_type_info,false); [line 28]\n NULLIFY(&t_type_info,false); [line 28]\n " color=yellow style=filled] +22 [label="22: Start person_typeid_name\nFormals: \nLocals: person_type_info:char * t_type_info:char * t:int person:class Person \n DECLARE_LOCALS(&return,&person_type_info,&t_type_info,&t,&person); [line 28]\n NULLIFY(&person_type_info,false); [line 28]\n NULLIFY(&t,false); [line 28]\n NULLIFY(&t_type_info,false); [line 28]\n " color=yellow style=filled] 22 -> 33 ; @@ -182,11 +182,11 @@ digraph iCFG { 20 -> 15 ; -19 [label="19: Return Stmt \n *&return:int =(1 / 0) [line 25]\n NULLIFY(&person,false); [line 25]\n NULLIFY(&t,false); [line 25]\n APPLY_ABSTRACTION; [line 25]\n " shape="box"] +19 [label="19: Return Stmt \n *&return:int =(1 / 0) [line 25]\n NULLIFY(&person,false); [line 25]\n APPLY_ABSTRACTION; [line 25]\n " shape="box"] 19 -> 13 ; -18 [label="18: Return Stmt \n *&return:int =1 [line 23]\n NULLIFY(&person,false); [line 23]\n NULLIFY(&t,false); [line 23]\n APPLY_ABSTRACTION; [line 23]\n " shape="box"] +18 [label="18: Return Stmt \n *&return:int =1 [line 23]\n NULLIFY(&person,false); [line 23]\n APPLY_ABSTRACTION; [line 23]\n " shape="box"] 18 -> 13 ; @@ -198,19 +198,19 @@ digraph iCFG { 16 -> 18 ; -15 [label="15: Call _fun_std::type_info_operator== \n n$0=_fun___cxx_typeid(sizeof(class std::type_info ):void ,n$0.__type_name:void ,&t:int ) [line 22]\n n$1=_fun___cxx_typeid(sizeof(class std::type_info ):void ,n$1.__type_name:void ,&person:class Person ) [line 22]\n n$2=_fun_std::type_info_operator==(n$0:class std::type_info &,n$1:class std::type_info &) [line 22]\n " shape="box"] +15 [label="15: Call _fun_std::type_info_operator== \n n$0=_fun___cxx_typeid(sizeof(class std::type_info ):void ,n$0.__type_name:void ,&t:int ) [line 22]\n n$1=_fun___cxx_typeid(sizeof(class std::type_info ):void ,n$1.__type_name:void ,&person:class Person ) [line 22]\n n$2=_fun_std::type_info_operator==(n$0:class std::type_info &,n$1:class std::type_info &) [line 22]\n NULLIFY(&t,false); [line 22]\n " shape="box"] 15 -> 16 ; 15 -> 17 ; -14 [label="14: + \n NULLIFY(&person,false); [line 22]\n NULLIFY(&t,false); [line 22]\n " ] +14 [label="14: + \n NULLIFY(&t,false); [line 22]\n NULLIFY(&person,false); [line 22]\n " ] 14 -> 13 ; 13 [label="13: Exit person_typeid \n " color=yellow style=filled] -12 [label="12: Start person_typeid\nFormals: \nLocals: t:int person:class Person \n DECLARE_LOCALS(&return,&t,&person); [line 19]\n " color=yellow style=filled] +12 [label="12: Start person_typeid\nFormals: \nLocals: t:int person:class Person \n DECLARE_LOCALS(&return,&t,&person); [line 19]\n NULLIFY(&t,false); [line 19]\n " color=yellow style=filled] 12 -> 21 ; diff --git a/infer/tests/codetoanalyze/objc/errors/npe/block.m b/infer/tests/codetoanalyze/objc/errors/npe/block.m index 8d338f62b..e9c13d353 100644 --- a/infer/tests/codetoanalyze/objc/errors/npe/block.m +++ b/infer/tests/codetoanalyze/objc/errors/npe/block.m @@ -80,4 +80,13 @@ _block_field(); // Ivar not nullable } +- (int)check_nullify { + int i = 7; + int* x = &i; + int (^my_block)(void) = ^() { + return *x; // we'll get a false NPE here if we nullify x too early. + }; + return my_block(); +} + @end