From 0f1bdf664d2e471d1ef1c86f6c467f4fffaf88ec Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Fri, 3 Nov 2017 09:43:13 -0700 Subject: [PATCH] [clang] static data members are external globals unless defined in the file Summary: The issue is with classes defining static data members: ``` $ cat foo.h struct A { static int foo; }; $ cat foo.cpp #include "foo.h" int A::foo = 12; int f() { return A::foo; // should see A::foo as defined in this translation unit $ cat bar.cpp #include "foo.h" void g() { return A::foo; // should see A::foo defined externally ``` Previously, both foo.cpp and bar.cpp would see `A::foo` as defined within their translation unit, because it comes from the header. This is wrong, and static data members should be treated as extern unless they're defined in the same file. This doesn't change much except for frontend tests. SIOF FP fix in the next diff. update-submodule: facebook-clang-plugins Reviewed By: da319 Differential Revision: D6221744 fbshipit-source-id: bef88fd --- facebook-clang-plugins | 2 +- infer/src/checkers/Siof.ml | 22 ++++++++++++------- infer/src/clang/cGeneral_utils.ml | 7 +++--- .../shared/namespace/global_variable.cpp.dot | 12 +++++----- 4 files changed, 25 insertions(+), 18 deletions(-) diff --git a/facebook-clang-plugins b/facebook-clang-plugins index 6fe051624..4274a6ac7 160000 --- a/facebook-clang-plugins +++ b/facebook-clang-plugins @@ -1 +1 @@ -Subproject commit 6fe0516240c68e8a8aa123996ba54c6f9247f64a +Subproject commit 4274a6ac7a43a91d7296f6bbf2d1a2634a5ca443 diff --git a/infer/src/checkers/Siof.ml b/infer/src/checkers/Siof.ml index 73960c39c..6d3b7d2a3 100644 --- a/infer/src/checkers/Siof.ml +++ b/infer/src/checkers/Siof.ml @@ -238,6 +238,7 @@ let siof_check pdesc gname (summary: Specs.summary) = let checker {Callbacks.proc_desc; tenv; summary; get_procs_in_file} : Specs.summary = + let pname = Procdesc.get_proc_name proc_desc in let standard_streams_initialized_in_tu = let includes_iostream tu = let magic_iostream_marker = @@ -250,8 +251,7 @@ let checker {Callbacks.proc_desc; tenv; summary; get_procs_in_file} : Specs.summ (TUFile tu) |> Pvar.get_initializer_pname ) in - get_procs_in_file (Procdesc.get_proc_name proc_desc) - |> List.exists ~f:(Typ.Procname.equal magic_iostream_marker) + get_procs_in_file pname |> List.exists ~f:(Typ.Procname.equal magic_iostream_marker) in Option.value_map ~default:false ~f:includes_iostream (Procdesc.get_attributes proc_desc).ProcAttributes.translation_unit @@ -263,13 +263,19 @@ let checker {Callbacks.proc_desc; tenv; summary; get_procs_in_file} : Specs.summ else SiofDomain.VarNames.empty ) in let updated_summary = - match Analyzer.compute_post proc_data ~initial with - | Some post -> - Summary.update_summary post summary - | None -> - summary + (* If the function is constexpr then it doesn't participate in SIOF. The checker should be able + to figure this out when analyzing the function, but we might as well use the user's + specification if it's given to us. This also serves as an optimization as this skips the + analysis of the function. *) + if Typ.Procname.is_constexpr pname then Summary.update_summary initial summary + else + match Analyzer.compute_post proc_data ~initial with + | Some post -> + Summary.update_summary post summary + | None -> + summary in - ( match Typ.Procname.get_global_name_of_initializer (Procdesc.get_proc_name proc_desc) with + ( match Typ.Procname.get_global_name_of_initializer pname with | Some gname -> siof_check proc_desc gname updated_summary | None -> diff --git a/infer/src/clang/cGeneral_utils.ml b/infer/src/clang/cGeneral_utils.ml index 90cedfa60..5570f1987 100644 --- a/infer/src/clang/cGeneral_utils.ml +++ b/infer/src/clang/cGeneral_utils.ml @@ -136,13 +136,14 @@ let mk_sil_global_var {CFrontend_config.source_file} ?(mk_name= fun _ x -> x) na var_decl_info qt = let name_string, simple_name = get_var_name_mangled named_decl_info var_decl_info in let translation_unit = - match - (var_decl_info.Clang_ast_t.vdi_storage_class, var_decl_info.Clang_ast_t.vdi_init_expr) - with + match Clang_ast_t.((var_decl_info.vdi_storage_class, var_decl_info.vdi_init_expr)) with | Some "extern", None -> (* some compilers simply disregard "extern" when the global is given some initialisation code, which is why we make sure that [vdi_init_expr] is None here... *) Pvar.TUExtern + | _, None when var_decl_info.Clang_ast_t.vdi_is_static_data_member -> + (* non-const static data member get extern scope unless they are defined out of line here (in which case vdi_init_expr will not be None) *) + Pvar.TUExtern | _ -> Pvar.TUFile source_file in diff --git a/infer/tests/codetoanalyze/cpp/shared/namespace/global_variable.cpp.dot b/infer/tests/codetoanalyze/cpp/shared/namespace/global_variable.cpp.dot index 8ecf284ac..6c03757f9 100644 --- a/infer/tests/codetoanalyze/cpp/shared/namespace/global_variable.cpp.dot +++ b/infer/tests/codetoanalyze/cpp/shared/namespace/global_variable.cpp.dot @@ -26,15 +26,15 @@ digraph iCFG { "div0_static_field#12231470699631142739.dca5ebae856e9b404facab8151fb6246_2" [label="2: Exit div0_static_field \n " color=yellow style=filled] -"div0_static_field#12231470699631142739.dca5ebae856e9b404facab8151fb6246_3" [label="3: Return Stmt \n n$0=*&#GB$f1::A::v:int [line 37, column 15]\n n$1=*&#GB$B::v:int [line 37, column 26]\n *&return:int=(1 / ((n$0 + n$1) + 1)) [line 37, column 3]\n " shape="box"] +"div0_static_field#12231470699631142739.dca5ebae856e9b404facab8151fb6246_3" [label="3: Return Stmt \n n$0=*&#GB$f1::A::v:int [line 37, column 15]\n n$1=*&#GB$B::v:int [line 37, column 26]\n *&return:int=(1 / ((n$0 + n$1) + 1)) [line 37, column 3]\n " shape="box"] "div0_static_field#12231470699631142739.dca5ebae856e9b404facab8151fb6246_3" -> "div0_static_field#12231470699631142739.dca5ebae856e9b404facab8151fb6246_2" ; -"div0_static_field#12231470699631142739.dca5ebae856e9b404facab8151fb6246_4" [label="4: BinaryOperatorStmt: Assign \n *&#GB$f1::A::v:int=-2 [line 36, column 3]\n " shape="box"] +"div0_static_field#12231470699631142739.dca5ebae856e9b404facab8151fb6246_4" [label="4: BinaryOperatorStmt: Assign \n *&#GB$f1::A::v:int=-2 [line 36, column 3]\n " shape="box"] "div0_static_field#12231470699631142739.dca5ebae856e9b404facab8151fb6246_4" -> "div0_static_field#12231470699631142739.dca5ebae856e9b404facab8151fb6246_3" ; -"div0_static_field#12231470699631142739.dca5ebae856e9b404facab8151fb6246_5" [label="5: BinaryOperatorStmt: Assign \n *&#GB$B::v:int=1 [line 35, column 3]\n " shape="box"] +"div0_static_field#12231470699631142739.dca5ebae856e9b404facab8151fb6246_5" [label="5: BinaryOperatorStmt: Assign \n *&#GB$B::v:int=1 [line 35, column 3]\n " shape="box"] "div0_static_field#12231470699631142739.dca5ebae856e9b404facab8151fb6246_5" -> "div0_static_field#12231470699631142739.dca5ebae856e9b404facab8151fb6246_4" ; @@ -45,15 +45,15 @@ digraph iCFG { "div0_static_field_member_access#8775359855042425857.b606a4de40e2ad34cbe0f38ab2d7e485_2" [label="2: Exit div0_static_field_member_access \n " color=yellow style=filled] -"div0_static_field_member_access#8775359855042425857.b606a4de40e2ad34cbe0f38ab2d7e485_3" [label="3: Return Stmt \n n$0=*&#GB$f1::A::v:int [line 43, column 15]\n n$1=*&#GB$B::v:int [line 43, column 26]\n *&return:int=(1 / ((n$0 + n$1) + 1)) [line 43, column 3]\n " shape="box"] +"div0_static_field_member_access#8775359855042425857.b606a4de40e2ad34cbe0f38ab2d7e485_3" [label="3: Return Stmt \n n$0=*&#GB$f1::A::v:int [line 43, column 15]\n n$1=*&#GB$B::v:int [line 43, column 26]\n *&return:int=(1 / ((n$0 + n$1) + 1)) [line 43, column 3]\n " shape="box"] "div0_static_field_member_access#8775359855042425857.b606a4de40e2ad34cbe0f38ab2d7e485_3" -> "div0_static_field_member_access#8775359855042425857.b606a4de40e2ad34cbe0f38ab2d7e485_2" ; -"div0_static_field_member_access#8775359855042425857.b606a4de40e2ad34cbe0f38ab2d7e485_4" [label="4: BinaryOperatorStmt: Assign \n *&#GB$B::v:int=-2 [line 42, column 3]\n " shape="box"] +"div0_static_field_member_access#8775359855042425857.b606a4de40e2ad34cbe0f38ab2d7e485_4" [label="4: BinaryOperatorStmt: Assign \n *&#GB$B::v:int=-2 [line 42, column 3]\n " shape="box"] "div0_static_field_member_access#8775359855042425857.b606a4de40e2ad34cbe0f38ab2d7e485_4" -> "div0_static_field_member_access#8775359855042425857.b606a4de40e2ad34cbe0f38ab2d7e485_3" ; -"div0_static_field_member_access#8775359855042425857.b606a4de40e2ad34cbe0f38ab2d7e485_5" [label="5: BinaryOperatorStmt: Assign \n *&#GB$f1::A::v:int=1 [line 41, column 3]\n " shape="box"] +"div0_static_field_member_access#8775359855042425857.b606a4de40e2ad34cbe0f38ab2d7e485_5" [label="5: BinaryOperatorStmt: Assign \n *&#GB$f1::A::v:int=1 [line 41, column 3]\n " shape="box"] "div0_static_field_member_access#8775359855042425857.b606a4de40e2ad34cbe0f38ab2d7e485_5" -> "div0_static_field_member_access#8775359855042425857.b606a4de40e2ad34cbe0f38ab2d7e485_4" ;