From bfab195d08ee9acb81939a6a852b97ce1c1a71cf Mon Sep 17 00:00:00 2001 From: Daiva Naudziuniene Date: Mon, 3 Sep 2018 03:17:08 -0700 Subject: [PATCH] [dead store] Do not report dead stores on constexpr Summary: We report dead store false positives in template arguments when constexpr is used. To remove the false positives, with the expense of some false negatives, we do not report dead stores on constexpr anymore. Reviewed By: mbouaziz Differential Revision: D9608095 fbshipit-source-id: 91b0c71c4 --- infer/src/IR/ProcAttributes.ml | 3 ++- infer/src/IR/ProcAttributes.mli | 2 +- infer/src/checkers/liveness.ml | 8 +++++++- infer/src/clang/cTrans.ml | 17 ++++++++++++----- infer/src/clang/cVar_decl.ml | 8 +++++++- infer/src/java/jTrans.ml | 2 +- .../cpp/liveness/dead_stores_constexpr.cpp | 2 +- .../tests/codetoanalyze/cpp/liveness/issues.exp | 2 -- .../cpp/liveness/non_type_template_param.cpp | 6 ++++++ 9 files changed, 37 insertions(+), 13 deletions(-) diff --git a/infer/src/IR/ProcAttributes.ml b/infer/src/IR/ProcAttributes.ml index efb52585f..c39a81e42 100644 --- a/infer/src/IR/ProcAttributes.ml +++ b/infer/src/IR/ProcAttributes.ml @@ -41,7 +41,8 @@ let string_of_var_attribute = function Modify_in_block -> "" let var_attribute_equal = [%compare.equal: var_attribute] -type var_data = {name: Mangled.t; typ: Typ.t; attributes: var_attribute list} [@@deriving compare] +type var_data = {name: Mangled.t; typ: Typ.t; attributes: var_attribute list; is_constexpr: bool} +[@@deriving compare] let pp_var_data fmt {name; typ; attributes} = F.fprintf fmt "@[{ name=@ %a;@ typ=@ %a" Mangled.pp name (Typ.pp_full Pp.text) typ ; diff --git a/infer/src/IR/ProcAttributes.mli b/infer/src/IR/ProcAttributes.mli index 260b4bacd..a6e69ca8f 100644 --- a/infer/src/IR/ProcAttributes.mli +++ b/infer/src/IR/ProcAttributes.mli @@ -23,7 +23,7 @@ type var_attribute = Modify_in_block val var_attribute_equal : var_attribute -> var_attribute -> bool (** Equality for var_attribute *) -type var_data = {name: Mangled.t; typ: Typ.t; attributes: var_attribute list} +type var_data = {name: Mangled.t; typ: Typ.t; attributes: var_attribute list; is_constexpr: bool} type t = { access: PredSymb.access (** visibility access *) diff --git a/infer/src/checkers/liveness.ml b/infer/src/checkers/liveness.ml index f938bed3e..4933e1c61 100644 --- a/infer/src/checkers/liveness.ml +++ b/infer/src/checkers/liveness.ml @@ -170,11 +170,17 @@ let checker {Callbacks.tenv; summary; proc_desc} : Summary.t = | _ -> false in + let locals = Procdesc.get_locals proc_desc in + let is_constexpr pvar = + List.find locals ~f:(fun local_data -> + Mangled.equal (Pvar.get_name pvar) local_data.ProcAttributes.name ) + |> Option.exists ~f:(fun local -> local.ProcAttributes.is_constexpr) + in let should_report pvar typ live_vars captured_by_ref_vars = (* T32000971: a temporay fix for dead store false positives until we support __unused__ attribute *) let is_unused_tmp_var pvar = String.equal "__tmp" (Pvar.to_string pvar) in not - ( Pvar.is_frontend_tmp pvar || Pvar.is_return pvar || Pvar.is_global pvar + ( Pvar.is_frontend_tmp pvar || Pvar.is_return pvar || Pvar.is_global pvar || is_constexpr pvar || is_unused_tmp_var pvar || VarSet.mem (Var.of_pvar pvar) captured_by_ref_vars || Domain.mem (Var.of_pvar pvar) live_vars diff --git a/infer/src/clang/cTrans.ml b/infer/src/clang/cTrans.ml index bdf783e08..795080135 100644 --- a/infer/src/clang/cTrans.ml +++ b/infer/src/clang/cTrans.ml @@ -210,7 +210,9 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s let context = trans_state.context in let procdesc = context.CContext.procdesc in let pvar, typ = mk_temp_sil_var_for_expr context.CContext.tenv procdesc var_name expr_info in - let var_data = ProcAttributes.{name= Pvar.get_name pvar; typ; attributes= []} in + let var_data = + ProcAttributes.{name= Pvar.get_name pvar; typ; attributes= []; is_constexpr= false} + in Procdesc.append_locals procdesc [var_data] ; (Exp.Lvar pvar, typ) @@ -246,7 +248,7 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s let procdesc = trans_state.context.CContext.procdesc in let pvar = mk_temp_sil_var procdesc "__temp_return_" in let var_data : ProcAttributes.var_data = - {name= Pvar.get_name pvar; typ= return_type; attributes= []} + {name= Pvar.get_name pvar; typ= return_type; attributes= []; is_constexpr= false} in Procdesc.append_locals procdesc [var_data] ; Exp.Lvar pvar @@ -1136,7 +1138,7 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s let pvar = Pvar.mk_tmp "__temp_construct_" (Procdesc.get_proc_name procdesc) in let class_type = CType_decl.get_type_from_expr_info ei context.CContext.tenv in let var_data : ProcAttributes.var_data = - {name= Pvar.get_name pvar; typ= class_type; attributes= []} + {name= Pvar.get_name pvar; typ= class_type; attributes= []; is_constexpr= false} in Procdesc.append_locals procdesc [var_data] ; (Exp.Lvar pvar, class_type) @@ -1541,7 +1543,10 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s in Procdesc.node_set_succs_exn context.procdesc join_node succ_nodes [] ; let pvar = mk_temp_sil_var procdesc "SIL_temp_conditional___" in - let var_data = ProcAttributes.{name= Pvar.get_name pvar; typ= var_typ; attributes= []} in + let var_data = + ProcAttributes. + {name= Pvar.get_name pvar; typ= var_typ; attributes= []; is_constexpr= false} + in Procdesc.append_locals procdesc [var_data] ; let continuation' = mk_cond_continuation trans_state.continuation in let trans_state' = {trans_state with continuation= continuation'; succ_nodes= []} in @@ -2903,7 +2908,9 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s let var_exp_typ = (Exp.Lvar pvar, typ_tmp) in let res_trans = init_expr_trans trans_state var_exp_typ stmt_info (Some temp_exp) in let _, typ = res_trans.return in - let var_data = ProcAttributes.{name= Pvar.get_name pvar; typ; attributes= []} in + let var_data = + ProcAttributes.{name= Pvar.get_name pvar; typ; attributes= []; is_constexpr= false} + in Procdesc.append_locals procdesc [var_data] ; res_trans diff --git a/infer/src/clang/cVar_decl.ml b/infer/src/clang/cVar_decl.ml index 49a3c560e..c7e65a711 100644 --- a/infer/src/clang/cVar_decl.ml +++ b/infer/src/clang/cVar_decl.ml @@ -69,7 +69,13 @@ let add_var_to_locals procdesc var_decl typ pvar = | VarDecl (decl_info, _, _, vdi) -> if not vdi.Clang_ast_t.vdi_is_global then let attributes = get_var_attribute decl_info in - let var_data : ProcAttributes.var_data = {name= Pvar.get_name pvar; typ; attributes} in + let is_constexpr = + vdi.Clang_ast_t.vdi_is_const_expr + || (Typ.is_const typ.Typ.quals && vdi.Clang_ast_t.vdi_is_init_expr_cxx11_constant) + in + let var_data : ProcAttributes.var_data = + {name= Pvar.get_name pvar; typ; attributes; is_constexpr} + in Procdesc.append_locals procdesc [var_data] | _ -> assert false diff --git a/infer/src/java/jTrans.ml b/infer/src/java/jTrans.ml index 48ba60807..f24db28a2 100644 --- a/infer/src/java/jTrans.ml +++ b/infer/src/java/jTrans.ml @@ -417,7 +417,7 @@ let create_cm_procdesc source_file program linereader icfg cm proc_name = let locals_ = translate_locals program tenv formals bytecode jbir_code in let locals = List.map locals_ ~f:(fun (name, typ) -> - ({name; typ; attributes= []} : ProcAttributes.var_data) ) + ({name; typ; attributes= []; is_constexpr= false} : ProcAttributes.var_data) ) in let method_annotation = JAnnotation.translate_method cm.Javalib.cm_annotations in let proc_attributes = diff --git a/infer/tests/codetoanalyze/cpp/liveness/dead_stores_constexpr.cpp b/infer/tests/codetoanalyze/cpp/liveness/dead_stores_constexpr.cpp index 672f0eab7..139a3a93a 100644 --- a/infer/tests/codetoanalyze/cpp/liveness/dead_stores_constexpr.cpp +++ b/infer/tests/codetoanalyze/cpp/liveness/dead_stores_constexpr.cpp @@ -41,7 +41,7 @@ void FN_init_capture_reassign_bad() { } // expected DEAD_STORE -void capture_constexpr_bad() { +void FN_capture_constexpr_bad() { constexpr int x = 1; foo(2); } diff --git a/infer/tests/codetoanalyze/cpp/liveness/issues.exp b/infer/tests/codetoanalyze/cpp/liveness/issues.exp index a1d027711..2a8fd1d86 100644 --- a/infer/tests/codetoanalyze/cpp/liveness/issues.exp +++ b/infer/tests/codetoanalyze/cpp/liveness/issues.exp @@ -18,7 +18,5 @@ codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::plus_plus2_bad, 2, DEAD codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::plus_plus3_bad, 2, DEAD_STORE, no_bucket, ERROR, [Write of unused value] codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::reassign_param_bad, 0, DEAD_STORE, no_bucket, ERROR, [Write of unused value] codetoanalyze/cpp/liveness/dead_stores.cpp, dead_stores::use_then_dead_bad, 3, DEAD_STORE, no_bucket, ERROR, [Write of unused value] -codetoanalyze/cpp/liveness/dead_stores_constexpr.cpp, FP_capture_constexpr_good, 1, DEAD_STORE, no_bucket, ERROR, [Write of unused value] -codetoanalyze/cpp/liveness/dead_stores_constexpr.cpp, capture_constexpr_bad, 1, DEAD_STORE, no_bucket, ERROR, [Write of unused value] codetoanalyze/cpp/liveness/non_type_template_param.cpp, X<3>_isZeroBad, 1, DEAD_STORE, no_bucket, ERROR, [Write of unused value] codetoanalyze/cpp/liveness/non_type_template_param.cpp, instanciateTemplateBad, 3, DEAD_STORE, no_bucket, ERROR, [Write of unused value] diff --git a/infer/tests/codetoanalyze/cpp/liveness/non_type_template_param.cpp b/infer/tests/codetoanalyze/cpp/liveness/non_type_template_param.cpp index 6f0d4da3a..a6f7bc038 100644 --- a/infer/tests/codetoanalyze/cpp/liveness/non_type_template_param.cpp +++ b/infer/tests/codetoanalyze/cpp/liveness/non_type_template_param.cpp @@ -20,3 +20,9 @@ int instanciateTemplateBad() { int unused = 1; return 0; } + +void instanciateTemplateConstOk() { + const int foo = 7; + X x; + x.isZeroBad(); +}