[cost] Revert subclass lookup heuristics

Summary:
In Cost/Inferbo checkers, it tried to find a subclass with heuristics when a method of interface or
abstract class is called.  While it makes preciser analysis results in general, sometimes introduced
tricky FPs since the behavior of the heuristics depends on targets.  This diff revert the heuristics
to suppress the FPs.

Reviewed By: ngorogiannis

Differential Revision: D22924485

fbshipit-source-id: 9f151231f
master
Sungkeun Cho 5 years ago committed by Facebook GitHub Bot
parent 19799336d4
commit 161b3484f9

@ -197,54 +197,6 @@ let store_global tenv =
store_to_filename tenv global_tenv_path store_to_filename tenv global_tenv_path
let get_summary_formals tenv ~get_summary ~get_formals =
let get pname =
match (get_summary pname, get_formals pname) with
| Some summary, Some formals ->
`Found (summary, formals)
| _, _ ->
`NotFound
in
let found_from_subclass pname = function
| `Found (summary, formals) ->
`FoundFromSubclass (pname, summary, formals)
| v ->
v
in
let rec get_summary_formals_aux pname =
match get pname with
| `Found _ as v ->
v
| `NotFound -> (
match Procname.get_class_type_name pname with
| None ->
`NotFound
| Some class_name when String.is_prefix (Typ.Name.name class_name) ~prefix:"java." ->
(* Note: We do not search sub-classes of `java.` because the super-classes are too
general. Selecting one arbitrary sub-class of them does not help in making preciser
analysis results. *)
`NotFound
| Some class_name -> (
match lookup tenv class_name with
| Some {Struct.java_class_info= Some info; subs}
when Struct.equal_java_class_kind info.kind Interface
&& Int.equal (Typ.Name.Set.cardinal subs) 1 ->
let unique_sub = Typ.Name.Set.choose subs in
Logging.d_printfln_escaped "Found a unique sub-class %a" Typ.Name.pp unique_sub ;
let sub_pname = Procname.replace_class pname unique_sub in
get_summary_formals_aux sub_pname |> found_from_subclass sub_pname
| Some {Struct.java_class_info= Some info; subs}
when Struct.equal_java_class_kind info.kind AbstractClass ->
Option.value_map (Typ.Name.Set.min_elt_opt subs) ~default:`NotFound ~f:(fun sub ->
Logging.d_printfln_escaped "Found an arbitrary sub-class %a" Typ.Name.pp sub ;
let sub_pname = Procname.replace_class pname sub in
get sub_pname |> found_from_subclass sub_pname )
| _ ->
`NotFound ) )
in
fun pname -> get_summary_formals_aux pname
let resolve_method ~method_exists tenv class_name proc_name = let resolve_method ~method_exists tenv class_name proc_name =
let visited = ref Typ.Name.Set.empty in let visited = ref Typ.Name.Set.empty in
let rec resolve_name_struct (class_name : Typ.Name.t) (class_struct : Struct.t) = let rec resolve_name_struct (class_name : Typ.Name.t) (class_struct : Struct.t) =

@ -76,22 +76,4 @@ val resolve_method :
[method_exists adapted_procname methods] should check if [adapted_procname] ([procname] but with [method_exists adapted_procname methods] should check if [adapted_procname] ([procname] but with
its class potentially changed to some [other_class]) is among the [methods] of [other_class]. *) its class potentially changed to some [other_class]) is among the [methods] of [other_class]. *)
val get_summary_formals :
t
-> get_summary:(Procname.t -> 'summary option)
-> get_formals:(Procname.t -> 'formals option)
-> Procname.t
-> [ `NotFound
| `Found of 'summary * 'formals
| `FoundFromSubclass of Procname.t * 'summary * 'formals ]
(** Get summary and formals of the given proc name with heuristics for finding a method in
non-abstract sub-classes
- If the class is an interface: Find its unique sub-class and apply the heuristics recursively.
- If the class is an abstract class: Find/use its own summary if possible. If not found, find
one (arbitrary but deterministic) summary from its sub-classes.
- Otherwise: Find its own summary. *)
module SQLite : SqliteUtils.Data with type t = per_file module SQLite : SqliteUtils.Data with type t = per_file

@ -390,14 +390,11 @@ module TransferFunctions = struct
let {BoUtils.ReplaceCallee.pname= callee_pname; params; is_params_ref} = let {BoUtils.ReplaceCallee.pname= callee_pname; params; is_params_ref} =
BoUtils.ReplaceCallee.replace_make_shared tenv get_formals callee_pname params BoUtils.ReplaceCallee.replace_make_shared tenv get_formals callee_pname params
in in
match match (get_summary callee_pname, get_formals callee_pname) with
(callee_pname, Tenv.get_summary_formals tenv ~get_summary ~get_formals callee_pname) | Some callee_exit_mem, Some callee_formals ->
with
| callee_pname, `Found (callee_exit_mem, callee_formals)
| _, `FoundFromSubclass (callee_pname, callee_exit_mem, callee_formals) ->
instantiate_mem ~is_params_ref integer_type_widths id callee_formals callee_pname instantiate_mem ~is_params_ref integer_type_widths id callee_formals callee_pname
params mem callee_exit_mem location params mem callee_exit_mem location
| _, `NotFound -> | _, _ ->
(* This may happen for procedures with a biabduction model too. *) (* This may happen for procedures with a biabduction model too. *)
L.d_printfln_escaped "/!\\ Unknown call to %a" Procname.pp callee_pname ; L.d_printfln_escaped "/!\\ Unknown call to %a" Procname.pp callee_pname ;
ScubaLogging.cost_log_message ~label:"unmodeled_function_inferbo" ScubaLogging.cost_log_message ~label:"unmodeled_function_inferbo"

@ -83,8 +83,8 @@ module InstrBasicCostWithReason = struct
in in
CostDomain.of_operation_cost (model model_env ~ret inferbo_mem) CostDomain.of_operation_cost (model model_env ~ret inferbo_mem)
| None -> ( | None -> (
match Tenv.get_summary_formals tenv ~get_summary ~get_formals callee_pname with match (get_summary callee_pname, get_formals callee_pname) with
| `Found ({CostDomain.post= callee_cost_record}, callee_formals) -> ( | Some {CostDomain.post= callee_cost_record}, Some callee_formals -> (
let instantiated_cost = let instantiated_cost =
CostDomain.map callee_cost_record ~f:(fun callee_cost -> CostDomain.map callee_cost_record ~f:(fun callee_cost ->
instantiate_cost integer_type_widths ~inferbo_caller_mem:inferbo_mem instantiate_cost integer_type_widths ~inferbo_caller_mem:inferbo_mem
@ -96,18 +96,7 @@ module InstrBasicCostWithReason = struct
(Some callee_pname) (Some callee_pname)
| _ -> | _ ->
instantiated_cost ) instantiated_cost )
| `FoundFromSubclass | _, _ ->
(callee_pname, {CostDomain.post= callee_cost_record}, callee_formals) ->
(* Note: It ignores top cost of subclass to avoid its propagations. *)
let instantiated_cost =
CostDomain.map callee_cost_record ~f:(fun callee_cost ->
instantiate_cost integer_type_widths ~inferbo_caller_mem:inferbo_mem
~callee_pname ~callee_formals ~params ~callee_cost ~loc )
in
if BasicCostWithReason.is_top (CostDomain.get_operation_cost instantiated_cost)
then CostDomain.unit_cost_atomic_operation
else instantiated_cost
| `NotFound ->
ScubaLogging.cost_log_message ~label:"unmodeled_function_cost_analysis" ScubaLogging.cost_log_message ~label:"unmodeled_function_cost_analysis"
~message: ~message:
(F.asprintf "Unmodeled Function[Cost Analysis] : %a" Procname.pp (F.asprintf "Unmodeled Function[Cost Analysis] : %a" Procname.pp

@ -16,7 +16,7 @@ class InheritanceTest {
} }
} }
public void call_interface_method_Good(MyInterface x) { public void call_interface_method_Good_FP(MyInterface x) {
int a[] = new int[10]; int a[] = new int[10];
a[x.foo()] = 0; a[x.foo()] = 0;
} }

@ -27,8 +27,9 @@ codetoanalyze/java/bufferoverrun/ArrayListTest.java, ArrayListTest.remove_in_loo
codetoanalyze/java/bufferoverrun/ArrayMember.java, codetoanalyze.java.bufferoverrun.ArrayMember.load_array_member_Bad():void, 4, BUFFER_OVERRUN_L1, no_bucket, ERROR, [<Offset trace>,Parameter `this.buf[*]`,Assignment,<Length trace>,Array declaration,Array access: Offset: [max(10, this.buf[*].lb), min(10, this.buf[*].ub)] Size: 10] codetoanalyze/java/bufferoverrun/ArrayMember.java, codetoanalyze.java.bufferoverrun.ArrayMember.load_array_member_Bad():void, 4, BUFFER_OVERRUN_L1, no_bucket, ERROR, [<Offset trace>,Parameter `this.buf[*]`,Assignment,<Length trace>,Array declaration,Array access: Offset: [max(10, this.buf[*].lb), min(10, this.buf[*].ub)] Size: 10]
codetoanalyze/java/bufferoverrun/CompressedData.java, codetoanalyze.java.bufferoverrun.CompressedData.decompressData(codetoanalyze.java.bufferoverrun.CompressedData$D):int, 9, INTEGER_OVERFLOW_L5, no_bucket, ERROR, [<LHS trace>,Parameter `this.yy`,<RHS trace>,Parameter `d.cci[*].s`,Assignment,Binary operation: ([0, this.yy - 1] × d.cci[*].s):signed32] codetoanalyze/java/bufferoverrun/CompressedData.java, codetoanalyze.java.bufferoverrun.CompressedData.decompressData(codetoanalyze.java.bufferoverrun.CompressedData$D):int, 9, INTEGER_OVERFLOW_L5, no_bucket, ERROR, [<LHS trace>,Parameter `this.yy`,<RHS trace>,Parameter `d.cci[*].s`,Assignment,Binary operation: ([0, this.yy - 1] × d.cci[*].s):signed32]
codetoanalyze/java/bufferoverrun/External.java, External.external_function_Bad(external.library.SomeExternalClass):void, 1, INTEGER_OVERFLOW_L5, no_bucket, ERROR, [<LHS trace>,Assignment,<RHS trace>,Assignment,Binary operation: ([-oo, +oo] + [-oo, +oo]):signed32] codetoanalyze/java/bufferoverrun/External.java, External.external_function_Bad(external.library.SomeExternalClass):void, 1, INTEGER_OVERFLOW_L5, no_bucket, ERROR, [<LHS trace>,Assignment,<RHS trace>,Assignment,Binary operation: ([-oo, +oo] + [-oo, +oo]):signed32]
codetoanalyze/java/bufferoverrun/InheritanceTest.java, InheritanceTest.call_interface_method2(InheritanceTest$MyInterface2):void, 2, BUFFER_OVERRUN_L1, no_bucket, ERROR, [<Offset trace>,Call,Assignment,Assignment,<Length trace>,Array declaration,Array access: Offset: 10 Size: 10] codetoanalyze/java/bufferoverrun/InheritanceTest.java, InheritanceTest.call_interface_method2(InheritanceTest$MyInterface2):void, 2, BUFFER_OVERRUN_U5, no_bucket, ERROR, [<Offset trace>,Unknown value from: int InheritanceTest$MyInterface2.foo(),Assignment,<Length trace>,Array declaration,Array access: Offset: [-oo, +oo] Size: 10]
codetoanalyze/java/bufferoverrun/InheritanceTest.java, InheritanceTest.call_interface_method_Bad(InheritanceTest$MyInterface):void, 2, BUFFER_OVERRUN_L1, no_bucket, ERROR, [<Offset trace>,Call,Assignment,Assignment,<Length trace>,Array declaration,Array access: Offset: 5 Size: 5] codetoanalyze/java/bufferoverrun/InheritanceTest.java, InheritanceTest.call_interface_method_Bad(InheritanceTest$MyInterface):void, 2, BUFFER_OVERRUN_U5, no_bucket, ERROR, [<Offset trace>,Unknown value from: int InheritanceTest$MyInterface.foo(),Assignment,<Length trace>,Array declaration,Array access: Offset: [-oo, +oo] Size: 5]
codetoanalyze/java/bufferoverrun/InheritanceTest.java, InheritanceTest.call_interface_method_Good_FP(InheritanceTest$MyInterface):void, 2, BUFFER_OVERRUN_U5, no_bucket, ERROR, [<Offset trace>,Unknown value from: int InheritanceTest$MyInterface.foo(),Assignment,<Length trace>,Array declaration,Array access: Offset: [-oo, +oo] Size: 10]
codetoanalyze/java/bufferoverrun/StringTest.java, StringTest.constant_Bad():void, 2, BUFFER_OVERRUN_L1, no_bucket, ERROR, [<Length trace>,Array declaration,Array access: Offset: 5 Size: 5] codetoanalyze/java/bufferoverrun/StringTest.java, StringTest.constant_Bad():void, 2, BUFFER_OVERRUN_L1, no_bucket, ERROR, [<Length trace>,Array declaration,Array access: Offset: 5 Size: 5]
codetoanalyze/java/bufferoverrun/StringTest.java, StringTest.constant_explicit_constructor_Bad():void, 2, BUFFER_OVERRUN_L1, no_bucket, ERROR, [<Length trace>,Array declaration,Array access: Offset: 5 Size: 5] codetoanalyze/java/bufferoverrun/StringTest.java, StringTest.constant_explicit_constructor_Bad():void, 2, BUFFER_OVERRUN_L1, no_bucket, ERROR, [<Length trace>,Array declaration,Array access: Offset: 5 Size: 5]
codetoanalyze/java/bufferoverrun/StringTest.java, StringTest.copy_constructor_Bad():void, 3, BUFFER_OVERRUN_L1, no_bucket, ERROR, [<Length trace>,Array declaration,Array access: Offset: 5 Size: 5] codetoanalyze/java/bufferoverrun/StringTest.java, StringTest.copy_constructor_Bad():void, 3, BUFFER_OVERRUN_L1, no_bucket, ERROR, [<Length trace>,Array declaration,Array access: Offset: 5 Size: 5]

@ -16,7 +16,7 @@ class InheritanceTest {
} }
} }
public void call_interface_method_linear(MyInterface c, int x) { public void call_interface_method_linear_FN(MyInterface c, int x) {
c.foo(x); c.foo(x);
} }
@ -41,7 +41,7 @@ class InheritanceTest {
} }
/* By heuristics, [Impl1.foo] is selected. It is hard to say good or bad. */ /* By heuristics, [Impl1.foo] is selected. It is hard to say good or bad. */
public void call_interface_method2_linear(MyInterface2 c, int x) { public void call_interface_method2_linear_FN(MyInterface2 c, int x) {
c.foo(x); c.foo(x);
} }

@ -10,7 +10,7 @@ interface LambdaTestI {
} }
class LambdaTest { class LambdaTest {
void call_lambda(int x) { void call_lambda_FN(int x) {
/* Three methods are auto-generated here: /* Three methods are auto-generated here:
- LambdaTest.callsite_LambdaTest$Lambda$_2_0 - LambdaTest.callsite_LambdaTest$Lambda$_2_0
- LambdaTest.access_LambdaTest$Lambda$_2_0 - LambdaTest.access_LambdaTest$Lambda$_2_0

@ -199,8 +199,8 @@ codetoanalyze/java/performance/InheritanceTest.java, InheritanceTest$UniqueImpl.
codetoanalyze/java/performance/InheritanceTest.java, InheritanceTest$UniqueImpl4.<init>(InheritanceTest), 6, OnUIThread:false, [] codetoanalyze/java/performance/InheritanceTest.java, InheritanceTest$UniqueImpl4.<init>(InheritanceTest), 6, OnUIThread:false, []
codetoanalyze/java/performance/InheritanceTest.java, InheritanceTest$UniqueImpl4.top_cost(InheritanceTest$MyInterface3):void, , OnUIThread:false, [Unbounded loop,Loop] codetoanalyze/java/performance/InheritanceTest.java, InheritanceTest$UniqueImpl4.top_cost(InheritanceTest$MyInterface3):void, , OnUIThread:false, [Unbounded loop,Loop]
codetoanalyze/java/performance/InheritanceTest.java, InheritanceTest.<init>(), 3, OnUIThread:false, [] codetoanalyze/java/performance/InheritanceTest.java, InheritanceTest.<init>(), 3, OnUIThread:false, []
codetoanalyze/java/performance/InheritanceTest.java, InheritanceTest.call_interface_method2_linear(InheritanceTest$MyInterface2,int):void, 8 + 5 ⋅ x, OnUIThread:false, [{x},Call to void InheritanceTest$Impl1.foo(int),Loop] codetoanalyze/java/performance/InheritanceTest.java, InheritanceTest.call_interface_method2_linear_FN(InheritanceTest$MyInterface2,int):void, 4, OnUIThread:false, []
codetoanalyze/java/performance/InheritanceTest.java, InheritanceTest.call_interface_method_linear(InheritanceTest$MyInterface,int):void, 8 + 5 ⋅ x, OnUIThread:false, [{x},Call to void InheritanceTest$UniqueImpl.foo(int),Loop] codetoanalyze/java/performance/InheritanceTest.java, InheritanceTest.call_interface_method_linear_FN(InheritanceTest$MyInterface,int):void, 4, OnUIThread:false, []
codetoanalyze/java/performance/InheritanceTest.java, InheritanceTest.ignore_top_costed_sub_method_constant(InheritanceTest$MyInterface3,InheritanceTest$MyInterface4):void, 4, OnUIThread:false, [] codetoanalyze/java/performance/InheritanceTest.java, InheritanceTest.ignore_top_costed_sub_method_constant(InheritanceTest$MyInterface3,InheritanceTest$MyInterface4):void, 4, OnUIThread:false, []
codetoanalyze/java/performance/IntTest.java, IntTest.<init>(), 3, OnUIThread:false, [] codetoanalyze/java/performance/IntTest.java, IntTest.<init>(), 3, OnUIThread:false, []
codetoanalyze/java/performance/IntTest.java, IntTest.control_var_band_add_constant(int,int):void, 186, OnUIThread:false, [] codetoanalyze/java/performance/IntTest.java, IntTest.control_var_band_add_constant(int,int):void, 186, OnUIThread:false, []
@ -253,7 +253,7 @@ codetoanalyze/java/performance/JsonUtils.java, libraries.marauder.analytics.util
codetoanalyze/java/performance/JsonUtils.java, libraries.marauder.analytics.utils.json.JsonUtils.serialize(java.lang.StringBuilder,long):void, 13, OnUIThread:false, [] codetoanalyze/java/performance/JsonUtils.java, libraries.marauder.analytics.utils.json.JsonUtils.serialize(java.lang.StringBuilder,long):void, 13, OnUIThread:false, []
codetoanalyze/java/performance/JsonUtils.java, libraries.marauder.analytics.utils.json.JsonUtils.serialize(long):java.lang.String, 6, OnUIThread:false, [] codetoanalyze/java/performance/JsonUtils.java, libraries.marauder.analytics.utils.json.JsonUtils.serialize(long):java.lang.String, 6, OnUIThread:false, []
codetoanalyze/java/performance/LambdaTest.java, LambdaTest.<init>(), 3, OnUIThread:false, [] codetoanalyze/java/performance/LambdaTest.java, LambdaTest.<init>(), 3, OnUIThread:false, []
codetoanalyze/java/performance/LambdaTest.java, LambdaTest.call_lambda(int):void, 21 + 5 ⋅ x, OnUIThread:false, [{x},Call to void LambdaTest$Lambda$_2_0.abstractFun(int),Call to void LambdaTest.access_LambdaTest$Lambda$_2_0(int),Call to void LambdaTest.lambda$call_lambda$0(int),Loop] codetoanalyze/java/performance/LambdaTest.java, LambdaTest.call_lambda_FN(int):void, 13, OnUIThread:false, []
codetoanalyze/java/performance/ListTest.java, ListTest$MyOwnObj.<init>(ListTest), 8, OnUIThread:false, [] codetoanalyze/java/performance/ListTest.java, ListTest$MyOwnObj.<init>(ListTest), 8, OnUIThread:false, []
codetoanalyze/java/performance/ListTest.java, ListTest$MyOwnObj.my_put():void, 5, OnUIThread:false, [] codetoanalyze/java/performance/ListTest.java, ListTest$MyOwnObj.my_put():void, 5, OnUIThread:false, []
codetoanalyze/java/performance/ListTest.java, ListTest.<init>(), 3, OnUIThread:false, [] codetoanalyze/java/performance/ListTest.java, ListTest.<init>(), 3, OnUIThread:false, []

Loading…
Cancel
Save