[inferpython] restore the previous chaining of globals value

Summary:
In a previous diff, I changed the chaining of globals parameter because I was facing strange alias specialization behaviors.
But the new module object encoding seems to have make this problem disappear. I'm switching back to the old convention and it removes a FN.

The other chaining is semantically unsound because it gives the wrong globals to a call `foo()` if `foo` comes from a `from .. import foo`.

Reviewed By: jvillard

Differential Revision:
D66758255

Privacy Context Container: L1208441

fbshipit-source-id: b036db24947fa9df16fc47fcf2198fee7f9d8554

infer/man/man1/infer-analyze.txt

@@ -1079,14 +1079,6 @@ PULSE CHECKER OPTIONS
        --pulse-widen-threshold int
            Stop exploring new paths after int loop iterations
 
-       --python-globals { own-by-closures | own-by-module }
-           Specify the strategy to wire globals dictionnaire into each
-           function
-                - own-by-closures: each closure captured the global
-           dictionary
-           - own-by-module: each function is given the global dictionary as
-           argument (not referenced in the heap to avoid aliases)
-
 RACERD CHECKER OPTIONS
        --racerd-always-report-java
            Activates: Every Java class analysed is treated as if it were

infer/man/man1/infer-full.txt

@@ -1884,15 +1884,6 @@ OPTIONS
            file2.py but not with --pyc-file.
            See also infer-capture(1).
 
-       --python-globals { own-by-closures | own-by-module }
-           Specify the strategy to wire globals dictionnaire into each
-           function
-                - own-by-closures: each closure captured the global
-           dictionary
-           - own-by-module: each function is given the global dictionary as
-           argument (not referenced in the heap to avoid aliases)
-           See also infer-analyze(1).
-
        --qualified-cpp-name-block-list +string
            Skip analyzing the procedures under the qualified cpp type name.
            See also infer-analyze(1).

infer/man/man1/infer.txt

@@ -1884,15 +1884,6 @@ OPTIONS
            file2.py but not with --pyc-file.
            See also infer-capture(1).
 
-       --python-globals { own-by-closures | own-by-module }
-           Specify the strategy to wire globals dictionnaire into each
-           function
-                - own-by-closures: each closure captured the global
-           dictionary
-           - own-by-module: each function is given the global dictionary as
-           argument (not referenced in the heap to avoid aliases)
-           See also infer-analyze(1).
-
        --qualified-cpp-name-block-list +string
            Skip analyzing the procedures under the qualified cpp type name.
            See also infer-analyze(1).

infer/src/base/Config.ml

@@ -54,10 +54,6 @@ let string_of_scheduler scheduler =
   List.Assoc.find_exn (List.Assoc.inverse scheduler_symbols) ~equal:equal_scheduler scheduler
 
 
-type python_globals = OwnByClosures | OwnByModule [@@deriving equal]
-
-let python_globals_symbols = [("own-by-closures", OwnByClosures); ("own-by-module", OwnByModule)]
-
 type pulse_taint_config =
   { sources: Pulse_config_t.matchers
   ; sanitizers: Pulse_config_t.matchers
@@ -2997,16 +2993,6 @@ and python_skip_db =
   CLOpt.mk_bool ~long:"python-skip-db" ~default:false "Skip the DB writing during Python capture"
 
 
-and python_globals =
-  CLOpt.mk_symbol ~long:"python-globals" ~default:OwnByModule ~eq:equal_python_globals
-    ~in_help:InferCommand.[(Analyze, manual_pulse)]
-    ~symbols:python_globals_symbols
-    "Specify the strategy to wire globals dictionnaire into each function\n\
-    \     - own-by-closures: each closure captured the global dictionary\n\
-     - own-by-module: each function is given the global dictionary as argument (not referenced in \
-     the heap to avoid aliases)"
-
-
 and qualified_cpp_name_block_list =
   CLOpt.mk_string_list ~long:"qualified-cpp-name-block-list" ~meta:"string"
     ~in_help:InferCommand.[(Analyze, manual_generic)]
@@ -4710,8 +4696,6 @@ and python_files_index = !python_files_index
 
 and python_skip_db = !python_skip_db
 
-and python_globals = !python_globals
-
 and qualified_cpp_name_block_list = RevList.to_list !qualified_cpp_name_block_list
 
 and quiet = !quiet

infer/src/base/Config.mli

@@ -35,8 +35,6 @@ type scheduler = File | Restart | SyntacticCallGraph [@@deriving equal]
 
 val string_of_scheduler : scheduler -> string
 
-type python_globals = OwnByClosures | OwnByModule [@@deriving equal]
-
 val build_system_of_exe_name : string -> build_system
 
 val string_of_build_system : build_system -> string
@@ -755,8 +753,6 @@ val python_files_index : string option
 
 val python_skip_db : bool
 
-val python_globals : python_globals
-
 val qualified_cpp_name_block_list : string list
 
 val quiet : bool

infer/src/pulse/PulseModelsPython.ml

@@ -115,24 +115,17 @@ let build_tuple args : model =
   assign_ret tuple
 
 
-let build_class globals closure _name _args : model =
+let build_class closure _name _args : model =
   let open DSL.Syntax in
   start_model
   @@ fun () ->
   let* class_ = Dict.make [] [] in
-  let gen_closure_args _ =
-    let locals = class_ in
-    match Config.python_globals with
-    | OwnByClosures ->
-        ret [locals]
-    | OwnByModule ->
-        ret [globals; locals]
-  in
+  let gen_closure_args _ = ret [class_] in
   let* _ = apply_python_closure closure gen_closure_args in
   assign_ret class_
 
 
-let call_dsl ~closure ~globals ~arg_names:_ ~args : DSL.aval DSL.model_monad =
+let call_dsl ~closure ~arg_names:_ ~args : DSL.aval DSL.model_monad =
   (* TODO: take into account named args *)
   let open DSL.Syntax in
   let gen_closure_args opt_proc_attrs =
@@ -150,21 +143,17 @@ let call_dsl ~closure ~globals ~arg_names:_ ~args : DSL.aval DSL.model_monad =
           List.mapi args ~f:(fun i _ -> Printf.sprintf "arg_%d" i)
     in
     let* locals = Dict.make python_args args in
-    match Config.python_globals with
-    | OwnByClosures ->
-        ret [locals]
-    | OwnByModule ->
-        ret [globals; locals]
+    ret [locals]
   in
   apply_python_closure closure gen_closure_args
 
 
-let call closure globals arg_names args : model =
+let call closure arg_names args : model =
   (* TODO: take into account named args *)
   let open DSL.Syntax in
   start_model
   @@ fun () ->
-  let* value = call_dsl ~closure ~globals ~arg_names ~args in
+  let* value = call_dsl ~closure ~arg_names ~args in
   assign_ret value
 
 
@@ -189,7 +178,7 @@ let modelled_python_call module_name fun_name args : DSL.aval option DSL.model_m
       ret None
 
 
-let call_method globals name obj arg_names args : model =
+let call_method name obj arg_names args : model =
   (* TODO: take into account named args *)
   let open DSL.Syntax in
   start_model
@@ -208,16 +197,15 @@ let call_method globals name obj arg_names args : model =
             L.d_printfln "calling method %a on module object %s" (Pp.option F.pp_print_string)
               opt_str_name module_name ;
             let* closure = Dict.get obj name in
-            call_dsl ~closure ~globals:obj ~arg_names ~args
+            call_dsl ~closure ~arg_names ~args
         | Some res ->
             L.d_printfln "catching special call %a on module object %s"
               (Pp.option F.pp_print_string) opt_str_name module_name ;
             ret res )
     | _ ->
         let* closure = Dict.get obj name in
         (* TODO: for OO method, gives self argument *)
-        (* this is incorrect for a module *)
-        call_dsl ~closure ~globals ~arg_names ~args
+        call_dsl ~closure ~arg_names ~args
   in
   assign_ret res
 
@@ -371,9 +359,9 @@ let yield_from _ _ : model =
 let matchers : matcher list =
   let open ProcnameDispatcher.Call in
   let arg = capt_arg_payload in
-  [ -"$builtins" &:: "py_build_class" <>$ arg $+ arg $+ arg $+++$--> build_class
-  ; -"$builtins" &:: "py_call" <>$ arg $+ arg $+ arg $+++$--> call
-  ; -"$builtins" &:: "py_call_method" <>$ arg $+ arg $+ arg $+ arg $+++$--> call_method
+  [ -"$builtins" &:: "py_build_class" <>$ arg $+ arg $+++$--> build_class
+  ; -"$builtins" &:: "py_call" <>$ arg $+ arg $+++$--> call
+  ; -"$builtins" &:: "py_call_method" <>$ arg $+ arg $+ arg $+++$--> call_method
   ; -"$builtins" &:: "py_build_tuple" &::.*+++> build_tuple
   ; -"$builtins" &:: "py_gen_start_coroutine" <>--> gen_start_coroutine
   ; -"$builtins" &:: "py_get_awaitable" <>$ arg $--> get_awaitable

infer/src/python/PyIR2Textual.ml

@@ -197,11 +197,7 @@ let rec of_exp exp : Textual.Exp.t =
     ->
       let proc = mk_qualified_proc_name (RegularFunction qual_name) in
       let closure =
-        match Config.python_globals with
-        | OwnByClosures ->
-            Textual.Exp.Closure {proc; captured= [exp_globals]; params= [Parameter.locals]}
-        | OwnByModule ->
-            Textual.Exp.Closure {proc; captured= []; params= [Parameter.globals; Parameter.locals]}
+        Textual.Exp.Closure {proc; captured= [exp_globals]; params= [Parameter.locals]}
       in
       call_builtin str_py_make_function
         ( closure
@@ -367,10 +363,6 @@ let builtin_name builtin =
       "py_get_previous_exception"
 
 
-let builtin_needs_globals_as_argument builtin =
-  match (builtin : BuiltinCaller.t) with BuildClass -> true | _ -> false
-
-
 let str_py_store_name = "py_store_name"
 
 let of_stmt loc stmt : Textual.Instr.t =
@@ -410,21 +402,18 @@ let of_stmt loc stmt : Textual.Instr.t =
         ; exp= call_builtin "py_store_subscript" [of_exp lhs; of_exp index; of_exp rhs]
         ; loc }
   | Call {lhs; exp; args; arg_names} ->
-      let args = of_exp exp :: exp_globals :: of_exp arg_names :: List.map ~f:of_exp args in
+      let args = of_exp exp :: of_exp arg_names :: List.map ~f:of_exp args in
       Let {id= Some (mk_ident lhs); exp= call_builtin "py_call" args; loc}
   | CallMethod {lhs; name; self_if_needed; args; arg_names} ->
       Let
         { id= Some (mk_ident lhs)
         ; exp=
             call_builtin "py_call_method"
-              ( exp_globals :: exp_of_ident_str name :: of_exp self_if_needed :: of_exp arg_names
+              ( exp_of_ident_str name :: of_exp self_if_needed :: of_exp arg_names
               :: List.map ~f:of_exp args )
         ; loc }
   | BuiltinCall {lhs; call; args; arg_names= _} ->
-      let args =
-        if builtin_needs_globals_as_argument call then exp_globals :: List.map ~f:of_exp args
-        else List.map ~f:of_exp args
-      in
+      let args = List.map ~f:of_exp args in
       Let {id= Some (mk_ident lhs); exp= call_builtin (builtin_name call) args; loc}
   | Delete {scope= Global; ident} ->
       Let {id= None; exp= call_builtin "py_delete_global" [exp_of_ident_str ident; exp_globals]; loc}
@@ -715,7 +704,7 @@ let gen_type module_name ~allow_classes name node =
       when QualifiedProcName.equal py_make_string proc ->
         let acc = DefaultType.add_string_constant ident str acc in
         find_next_declaration acc instrs
-    | Instr.Let {id= Some ident; exp= Call {proc; args= [_; _; Var ident_str_const]}} :: instrs
+    | Instr.Let {id= Some ident; exp= Call {proc; args= [_; Var ident_str_const]}} :: instrs
       when QualifiedProcName.equal py_build_class proc
            && DefaultType.is_string_constant ident_str_const acc ->
         let name = DefaultType.get_string_constant ident_str_const acc |> Option.value_exn in