Skip to content

Commit 8c82072

Browse files
davidpichardiefacebook-github-bot
authored andcommitted
First version of a devirtualization pre-analysis (Java frontend)
Reviewed By: jvillard Differential Revision: D20692190 fbshipit-source-id: e1ac5c454
1 parent a148812 commit 8c82072

File tree

7 files changed

+215
-4
lines changed

7 files changed

+215
-4
lines changed

infer/src/IR/Procdesc.ml

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,16 @@ module Node = struct
253253
true )
254254

255255

256+
(** Map and replace the instructions to be executed using a context *)
257+
let replace_instrs_using_context node ~f ~update_context ~context_at_node =
258+
let f node context instr = (update_context context instr, f node context instr) in
259+
let instrs' = Instrs.map_and_fold node.instrs ~f:(f node) ~init:context_at_node in
260+
if phys_equal instrs' node.instrs then false
261+
else (
262+
node.instrs <- instrs' ;
263+
true )
264+
265+
256266
(** Like [replace_instrs], but 1 instr gets replaced by 0, 1, or more instructions. *)
257267
let replace_instrs_by node ~f =
258268
let instrs' = Instrs.concat_map node.instrs ~f:(f node) in
@@ -558,6 +568,14 @@ let replace_instrs pdesc ~f =
558568
update_nodes pdesc ~update
559569

560570

571+
let replace_instrs_using_context pdesc ~f ~update_context ~context_at_node =
572+
let update node =
573+
Node.replace_instrs_using_context ~f ~update_context ~context_at_node:(context_at_node node)
574+
node
575+
in
576+
update_nodes pdesc ~update
577+
578+
561579
let replace_instrs_by pdesc ~f =
562580
let update node = Node.replace_instrs_by ~f node in
563581
update_nodes pdesc ~update

infer/src/IR/Procdesc.mli

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,15 @@ val replace_instrs : t -> f:(Node.t -> Sil.instr -> Sil.instr) -> bool
268268
(** Map and replace the instructions to be executed. Returns true if at least one substitution
269269
occured. *)
270270

271+
val replace_instrs_using_context :
272+
t
273+
-> f:(Node.t -> 'a -> Sil.instr -> Sil.instr)
274+
-> update_context:('a -> Sil.instr -> 'a)
275+
-> context_at_node:(Node.t -> 'a)
276+
-> bool
277+
(** Map and replace the instructions to be executed using a context that we built with previous
278+
instructions in the node. Returns true if at least one substitution occured. *)
279+
271280
val replace_instrs_by : t -> f:(Node.t -> Sil.instr -> Sil.instr array) -> bool
272281
(** Like [replace_instrs], but slower, and each instruction may be replaced by 0, 1, or more
273282
instructions. *)

infer/src/backend/Devirtualizer.ml

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
(*
2+
* Copyright (c) Facebook, Inc. and its affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*)
7+
8+
open! IStd
9+
module L = Logging
10+
11+
module VDom = AbstractDomain.Flat (JavaClassName)
12+
(** value domain, with the following concretization function [gamma]:
13+
14+
{[
15+
gamma(VDom.top) = { any value }
16+
gamma(VDom.v A) = { any ref of exact class A }
17+
gamma(VDom.bot) = emptyset
18+
]} *)
19+
20+
module CFG = ProcCfg.Normal
21+
module Domain = AbstractDomain.Map (Var) (VDom)
22+
23+
let get_var (astate : Domain.t) (v : Var.t) : VDom.t =
24+
match Domain.find_opt v astate with Some ab -> ab | None -> VDom.bottom
25+
26+
27+
let rec eval_expr (astate : Domain.t) (expr : Exp.t) : VDom.t =
28+
match expr with
29+
| Var id ->
30+
get_var astate (Var.of_id id)
31+
| UnOp _ | BinOp _ | Exn _ | Closure _ ->
32+
VDom.top
33+
| Const _ ->
34+
VDom.top (* could be more precise for Cstr and Cclass *)
35+
| Cast (_, e) ->
36+
eval_expr astate e (* could be more precise for final class *)
37+
| Lvar v ->
38+
get_var astate (Var.of_pvar v)
39+
| Lfield _ ->
40+
VDom.top (* could be more precise for final class *)
41+
| Lindex _ ->
42+
VDom.top
43+
| Sizeof _ ->
44+
VDom.top
45+
46+
47+
let eval_fun pname args =
48+
(* can be extended later if we decide to handle more builtins *)
49+
if Procname.equal pname BuiltinDecl.__new then
50+
match args with
51+
| (_, typ) :: _ when Typ.is_pointer typ -> (
52+
match Typ.name (Typ.strip_ptr typ) with Some (Typ.JavaClass cn) -> VDom.v cn | _ -> VDom.top )
53+
| _ ->
54+
VDom.top
55+
else VDom.top
56+
57+
58+
let eval_instr (astate : Domain.t) (instr : Sil.instr) : Domain.t =
59+
match instr with
60+
| Load {id} when Ident.is_none id ->
61+
astate
62+
| Load {id; e} ->
63+
let aval = eval_expr astate e in
64+
Domain.add (Var.of_id id) aval astate
65+
| Call ((id, _), Const (Const.Cfun pname), args, _, _) ->
66+
let aval = eval_fun pname args in
67+
Domain.add (Var.of_id id) aval astate
68+
| Call ((id, _), _, _, _, _) ->
69+
Domain.add (Var.of_id id) VDom.top astate
70+
| Store {e1= Lvar pvar; e2} ->
71+
let aval = eval_expr astate e2 in
72+
Domain.add (Var.of_pvar pvar) aval astate
73+
| Store _ | Prune _ | Metadata _ ->
74+
astate
75+
76+
77+
module TransferFunctions = struct
78+
module CFG = CFG
79+
module Domain = Domain
80+
81+
type analysis_data = unit
82+
83+
let exec_instr astate _ _node instr = eval_instr astate instr
84+
85+
let pp_session_name node fmt =
86+
Format.fprintf fmt "devirtualizer analysis %a" CFG.Node.pp_id (CFG.Node.id node)
87+
end
88+
89+
module Analyzer = AbstractInterpreter.MakeRPO (TransferFunctions)
90+
91+
let analyze_at_node (map : Analyzer.invariant_map) node : Domain.t =
92+
match Analyzer.InvariantMap.find_opt (Procdesc.Node.get_id node) map with
93+
| Some abstate ->
94+
abstate.pre
95+
| None ->
96+
Domain.bottom
97+
98+
99+
(* inspired from biabduction/Symexec.ml, function resolve_method *)
100+
let resolve_method tenv class_name proc_name =
101+
let method_exists pname methods = List.exists ~f:(Procname.equal pname) methods in
102+
let rec resolve class_name =
103+
let resolved_proc_name = Procname.replace_class proc_name class_name in
104+
match Tenv.lookup tenv class_name with
105+
| Some {methods; supers} when Typ.Name.is_class class_name -> (
106+
if method_exists resolved_proc_name methods then Some resolved_proc_name
107+
else match supers with super_classname :: _ -> resolve super_classname | _ -> None )
108+
| _ ->
109+
None
110+
in
111+
resolve class_name
112+
113+
114+
let process summary tenv =
115+
let pdesc = Summary.get_proc_desc summary in
116+
let node_cfg = CFG.from_pdesc pdesc in
117+
let all_params = Procdesc.get_pvar_formals pdesc in
118+
let initial =
119+
(* all params -> top, bottom otherwise *)
120+
(* we could use param type (if final) to get more precision *)
121+
List.fold_left ~init:Domain.empty
122+
~f:(fun acc (pvar, _) -> Domain.add (Var.of_pvar pvar) VDom.top acc)
123+
all_params
124+
in
125+
let map = Analyzer.exec_cfg node_cfg () ~initial in
126+
let is_virtual_call call_flags =
127+
call_flags.CallFlags.cf_virtual || call_flags.CallFlags.cf_interface
128+
in
129+
let replace_instr node (astate : Domain.t) (instr : Sil.instr) : Sil.instr =
130+
let kind = `ExecNode in
131+
let pp_name fmt = Format.pp_print_string fmt "devirtualizer" in
132+
NodePrinter.with_session (CFG.Node.underlying_node node) ~kind ~pp_name ~f:(fun () ->
133+
match instr with
134+
| Call
135+
( ret_id_typ
136+
, Const (Const.Cfun callee_pname)
137+
, ((this_expr, _) :: _ as actual_params)
138+
, loc
139+
, call_flags )
140+
when is_virtual_call call_flags -> (
141+
L.d_printfln "virtual call %a " Procname.pp callee_pname ;
142+
let aval = eval_expr astate this_expr in
143+
match VDom.get aval with
144+
| Some dyn_typ -> (
145+
match resolve_method tenv (Typ.JavaClass dyn_typ) callee_pname with
146+
| None ->
147+
L.d_printfln "(unexpected: no resolved method found)" ;
148+
instr
149+
| Some resolved_callee_pname ->
150+
let resolved_call_flags =
151+
{call_flags with cf_virtual= false; cf_interface= false}
152+
in
153+
L.d_printfln "replaced by nonvirtual <%a>\n" Procname.pp resolved_callee_pname ;
154+
Sil.Call
155+
( ret_id_typ
156+
, Const (Const.Cfun resolved_callee_pname)
157+
, actual_params
158+
, loc
159+
, resolved_call_flags ) )
160+
| _ ->
161+
Logging.debug Capture Verbose "unchanged\n" ;
162+
instr )
163+
| _ ->
164+
instr )
165+
in
166+
let update_context = eval_instr in
167+
let context_at_node node = analyze_at_node map node in
168+
let _has_changed : bool =
169+
Procdesc.replace_instrs_using_context pdesc ~f:replace_instr ~update_context ~context_at_node
170+
in
171+
()
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
(*
2+
* Copyright (c) Facebook, Inc. and its affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*)
7+
8+
(** {1 Simple devirtualization pre-analysis using a flow-sensitive tracking of dynamic classes} *)
9+
10+
open! IStd
11+
12+
val process : Summary.t -> Tenv.t -> unit
13+
(** Run the devirtualization pass by replacing some virtual calls by resolved calls *)

infer/src/backend/preanal.ml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,5 +379,6 @@ let do_preanalysis exe_env pdesc =
379379
FunctionPointerSubstitution.process pdesc ;
380380
Liveness.process summary tenv ;
381381
AddAbstractionInstructions.process pdesc ;
382+
if Procname.is_java proc_name then Devirtualizer.process summary tenv ;
382383
NoReturn.process tenv pdesc ;
383384
()

infer/tests/codetoanalyze/java/biabduction/issues.exp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ codetoanalyze/java/biabduction/CloseableAsResourceExample.java, codetoanalyze.ja
2626
codetoanalyze/java/biabduction/CloseableAsResourceExample.java, codetoanalyze.java.infer.CloseableAsResourceExample.leakFoundWhenIndirectlyImplementingCloseable():void, 1, RESOURCE_LEAK, no_bucket, ERROR, [start of procedure leakFoundWhenIndirectlyImplementingCloseable(),start of procedure CloseableAsResourceExample$MyResource(...),return from a call to CloseableAsResourceExample$MyResource.<init>(CloseableAsResourceExample)]
2727
codetoanalyze/java/biabduction/CloseableAsResourceExample.java, codetoanalyze.java.infer.CloseableAsResourceExample.notClosingCloseable():void, 1, RESOURCE_LEAK, no_bucket, ERROR, [start of procedure notClosingCloseable(),start of procedure SomeResource(),return from a call to SomeResource.<init>()]
2828
codetoanalyze/java/biabduction/CloseableAsResourceExample.java, codetoanalyze.java.infer.CloseableAsResourceExample.notClosingWrapper():void, 2, RESOURCE_LEAK, no_bucket, ERROR, [start of procedure notClosingWrapper(),start of procedure Resource(),return from a call to Resource.<init>(),start of procedure Sub(...),start of procedure Wrapper(...),return from a call to Wrapper.<init>(Resource),return from a call to Sub.<init>(Resource),start of procedure close(),return from a call to void Resource.close()]
29-
codetoanalyze/java/biabduction/CloseableAsResourceExample.java, codetoanalyze.java.infer.CloseableAsResourceExample.skippedVritualCallDoesNotCloseResourceOnReceiver():void, 2, RESOURCE_LEAK, no_bucket, ERROR, [start of procedure skippedVritualCallDoesNotCloseResourceOnReceiver(),start of procedure SomeResource(),return from a call to SomeResource.<init>(),Skipping foo(...): unknown method,Definition of foo(...)]
3029
codetoanalyze/java/biabduction/CloseableAsResourceExample.java, codetoanalyze.java.infer.CloseableAsResourceExample.sourceOfNullWithResourceLeak():codetoanalyze.java.infer.T, 1, RESOURCE_LEAK, no_bucket, ERROR, [start of procedure sourceOfNullWithResourceLeak(),start of procedure SomeResource(),return from a call to SomeResource.<init>()]
3130
codetoanalyze/java/biabduction/CloseableAsResourceExample.java, codetoanalyze.java.infer.CloseableAsResourceExample.withException():void, 4, RESOURCE_LEAK, no_bucket, ERROR, [start of procedure withException(),start of procedure SomeResource(),return from a call to SomeResource.<init>(),start of procedure doSomething(),Skipping star(): unknown method,Definition of star(),Taking true branch,start of procedure LocalException(),return from a call to LocalException.<init>(),exception codetoanalyze.java.infer.LocalException,return from a call to void SomeResource.doSomething()]
3231
codetoanalyze/java/biabduction/CursorLeaks.java, codetoanalyze.java.infer.CursorLeaks.completeDownloadNotClosed(android.app.DownloadManager):int, 8, RESOURCE_LEAK, no_bucket, ERROR, [start of procedure completeDownloadNotClosed(...),Taking false branch,Skipping getColumnIndex(...): unknown method]

infer/tests/codetoanalyze/java/nullsafe/issues.exp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -325,19 +325,19 @@ codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default
325325
codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_convertingNullableToNonnullIsBad():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, ERROR, [`nonStrictClass_convertingNullableToNonnullIsBad()`: return type is declared non-nullable but the method returns a nullable value: call to getNullable() at line 137.]
326326
codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNonnullFieldAfterCheckIsOK():void, 2, ERADICATE_CONDITION_REDUNDANT, no_bucket, ADVICE, [The condition NonStrict.nonnull might be always true according to the existing annotations.]
327327
codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNonnullFieldIsBad():void, 2, ERADICATE_UNCHECKED_USAGE_IN_NULLSAFE, no_bucket, ERROR, [`NonStrict.nonnull`: `@NullsafeStrict` mode prohibits using values coming from non-strict classes without a check. This field is used at line 133. Either add a local check for null or assertion, or make `NonStrict` nullsafe strict.]
328-
codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNonnullMethodAfterCheckIsOK():void, 2, ERADICATE_CONDITION_REDUNDANT, no_bucket, ADVICE, [The condition lang.String(o)V might be always true: `NonStrict.getNonnull()` is not annotated as `@Nullable`.]
328+
codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNonnullMethodAfterCheckIsOK():void, 2, ERADICATE_CONDITION_REDUNDANT, no_bucket, ADVICE, [The condition lang.String(o) might be always true: `NonStrict.getNonnull()` is not annotated as `@Nullable`.]
329329
codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNonnullMethodIsBad():void, 2, ERADICATE_UNCHECKED_USAGE_IN_NULLSAFE, no_bucket, ERROR, [`NonStrict.getNonnull()`: `@NullsafeStrict` mode prohibits using values coming from non-strict classes without a check. Result of this call is used at line 115. Either add a local check for null or assertion, or make `NonStrict` nullsafe strict.]
330330
codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNonnullStaticMethodIsBad():void, 2, ERADICATE_UNCHECKED_USAGE_IN_NULLSAFE, no_bucket, ERROR, [`NonStrict.staticNonnull()`: `@NullsafeStrict` mode prohibits using values coming from non-strict classes without a check. Result of this call is used at line 124. Either add a local check for null or assertion, or make `NonStrict` nullsafe strict.]
331331
codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNullableFieldIsBad():void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, ERROR, [`__new(...).nullable` is nullable and is not locally checked for null when calling `toString()`.]
332-
codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNullableMethodIsBad():void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, ERROR, [`__new(...).getNullable()` is nullable and is not locally checked for null when calling `toString()`.]
332+
codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNullableMethodIsBad():void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, ERROR, [`getNullable(...)` is nullable and is not locally checked for null when calling `toString()`: call to getNullable() at line 110.]
333333
codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNullableStaticMethodIsBad():void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, ERROR, [`staticNullable()` is nullable and is not locally checked for null when calling `toString()`.]
334334
codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.sameClass_convertingNullableToNonnullIsBad():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, ERROR, [`sameClass_convertingNullableToNonnullIsBad()`: return type is declared non-nullable but the method returns a nullable value: call to getNullable() at line 65.]
335335
codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.sameClass_dereferenceNullableFieldIsBad():void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, ERROR, [`Strict.nullable` is nullable and is not locally checked for null when calling `toString()`.]
336336
codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.sameClass_dereferenceNullableMethodIsBad():void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, ERROR, [`getNullable()` is nullable and is not locally checked for null when calling `toString()`.]
337337
codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.sameClass_dereferenceNullableStaticMethodIsBad():void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, ERROR, [`staticNullable()` is nullable and is not locally checked for null when calling `toString()`.]
338338
codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.strictClass_convertingNullableToNonnullIsBad():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, ERROR, [`strictClass_convertingNullableToNonnullIsBad()`: return type is declared non-nullable but the method returns a nullable value: call to getNullable() at line 99.]
339339
codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.strictClass_dereferenceNullableFieldIsBad():void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, ERROR, [`__new(...).nullable` is nullable and is not locally checked for null when calling `toString()`.]
340-
codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.strictClass_dereferenceNullableMethodIsBad():void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, ERROR, [`__new(...).getNullable()` is nullable and is not locally checked for null when calling `toString()`.]
340+
codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.strictClass_dereferenceNullableMethodIsBad():void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, ERROR, [`getNullable(...)` is nullable and is not locally checked for null when calling `toString()`: call to getNullable() at line 75.]
341341
codetoanalyze/java/nullsafe/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.strictClass_dereferenceNullableStaticMethodIsBad():void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, ERROR, [`staticNullable()` is nullable and is not locally checked for null when calling `toString()`.]
342342
codetoanalyze/java/nullsafe/StrictModeForThirdParty.java, Linters_dummy_method, 19, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], StrictModeForThirdParty, codetoanalyze.java.nullsafe_default, issues: 5, curr_mode: "Strict"
343343
codetoanalyze/java/nullsafe/StrictModeForThirdParty.java, codetoanalyze.java.nullsafe_default.StrictModeForThirdParty.dereferenceFieldIsBAD():void, 1, ERADICATE_UNVETTED_THIRD_PARTY_IN_NULLSAFE, no_bucket, ERROR, [`ThirdPartyTestClass.nonNullableField`: `@NullsafeStrict` mode prohibits using values coming from third-party classes without a check. This field is used at line 49. Either add a local check for null or assertion, or access `nonNullableField` via a nullsafe strict getter.]

0 commit comments

Comments
 (0)