[Eradicate] Improve detection of circularities in field initialization

Summary:
Eradicate detects circular field initializations (e.g. a field initialized with itself)
by checking in the typestate at the end of the constructor whether the origin
of the field is a field name in the current class.
This has the problem that the following initialization pattern is not recognized as correct:
  C(C x) { this.field = x.field }

To fix the issue, the origin information for field accesses x.f is extended
with the origin information of the inner object x.
Circularities are detected if the origin of x is "this".

Reviewed By: jberdine

Differential Revision: D4672472

fbshipit-source-id: 9277bdd
master
Cristiano Calcagno 8 years ago committed by Facebook Github Bot
parent f30a26f02c
commit 5aa714b237

@ -268,10 +268,8 @@ let check_constructor_initialization tenv
let origin_is_initialized = function let origin_is_initialized = function
| TypeOrigin.Undef -> | TypeOrigin.Undef ->
false false
| TypeOrigin.Field (f, _) -> | TypeOrigin.Field (TypeOrigin.Formal name, _, _) ->
(* field initialized with another field needing initialization *) let circular = String.equal (Mangled.to_string name) "this" in
let circular =
List.exists ~f:(fun (f', _, _) -> Ident.equal_fieldname f f') fields in
not circular not circular
| _ -> | _ ->
true in true in

@ -187,13 +187,16 @@ let rec typecheck_expr
let (_, ta, locs') = let (_, ta, locs') =
typecheck_expr typecheck_expr
find_canonical_duplicate visited checks tenv node instr_ref curr_pdesc typestate exp find_canonical_duplicate visited checks tenv node instr_ref curr_pdesc typestate exp
(typ, TypeAnnotation.const AnnotatedSignature.Nullable false TypeOrigin.ONone, locs) loc (typ,
in TypeAnnotation.const AnnotatedSignature.Nullable false TypeOrigin.ONone,
locs)
loc in
let exp_origin = TypeAnnotation.get_origin ta in
let tr_new = match EradicateChecks.get_field_annotation tenv fn typ with let tr_new = match EradicateChecks.get_field_annotation tenv fn typ with
| Some (t, ia) -> | Some (t, ia) ->
( (
t, t,
TypeAnnotation.from_item_annotation ia (TypeOrigin.Field (fn, loc)), TypeAnnotation.from_item_annotation ia (TypeOrigin.Field (exp_origin, fn, loc)),
locs' locs'
) )
| None -> tr_default in | None -> tr_default in
@ -283,7 +286,7 @@ let typecheck_instr
let default = exp, typestate in let default = exp, typestate in
(* If this is an assignment, update the typestate for a field access pvar. *) (* If this is an assignment, update the typestate for a field access pvar. *)
let update_typestate_fld pvar fn typ = let update_typestate_fld pvar origin fn typ =
match TypeState.lookup_pvar pvar typestate with match TypeState.lookup_pvar pvar typestate with
| Some _ when not is_assignment -> typestate | Some _ when not is_assignment -> typestate
| _ -> | _ ->
@ -292,7 +295,8 @@ let typecheck_instr
let range = let range =
( (
t, t,
TypeAnnotation.from_item_annotation ia (TypeOrigin.Field (fn, loc)), TypeAnnotation.from_item_annotation
ia (TypeOrigin.Field (origin, fn, loc)),
[loc] [loc]
) in ) in
TypeState.add pvar range typestate TypeState.add pvar range typestate
@ -340,8 +344,17 @@ let typecheck_instr
| Exp.Lvar _ -> | Exp.Lvar _ ->
default default
| Exp.Lfield (_exp, fn, typ) when ComplexExpressions.parameter_and_static_field () -> | Exp.Lfield (exp_, fn, typ) when ComplexExpressions.parameter_and_static_field () ->
let exp' = Idenv.expand_expr_temps idenv node _exp in let inner_origin =
(match exp_ with
| Exp.Lvar pvar -> TypeState.lookup_pvar pvar typestate
| Exp.Var id -> TypeState.lookup_id id typestate
| _ -> None)
|>
Option.value_map
~f:(fun (_, ta, _) -> TypeAnnotation.get_origin ta)
~default:TypeOrigin.ONone in
let exp' = Idenv.expand_expr_temps idenv node exp_ in
let is_parameter_field pvar = (* parameter.field *) let is_parameter_field pvar = (* parameter.field *)
let name = Pvar.get_name pvar in let name = Pvar.get_name pvar in
@ -359,13 +372,13 @@ let typecheck_instr
| Exp.Lvar pv when is_parameter_field pv || is_static_field pv -> | Exp.Lvar pv when is_parameter_field pv || is_static_field pv ->
let fld_name = pvar_to_str pv ^ Ident.fieldname_to_string fn in let fld_name = pvar_to_str pv ^ Ident.fieldname_to_string fn in
let pvar = Pvar.mk (Mangled.from_string fld_name) curr_pname in let pvar = Pvar.mk (Mangled.from_string fld_name) curr_pname in
let typestate' = update_typestate_fld pvar fn typ in let typestate' = update_typestate_fld pvar inner_origin fn typ in
(Exp.Lvar pvar, typestate') (Exp.Lvar pvar, typestate')
| Exp.Lfield (_exp', fn', _) when Ident.java_fieldname_is_outer_instance fn' -> | Exp.Lfield (_exp', fn', _) when Ident.java_fieldname_is_outer_instance fn' ->
(* handle double dereference when accessing a field from an outer class *) (* handle double dereference when accessing a field from an outer class *)
let fld_name = Ident.fieldname_to_string fn' ^ "_" ^ Ident.fieldname_to_string fn in let fld_name = Ident.fieldname_to_string fn' ^ "_" ^ Ident.fieldname_to_string fn in
let pvar = Pvar.mk (Mangled.from_string fld_name) curr_pname in let pvar = Pvar.mk (Mangled.from_string fld_name) curr_pname in
let typestate' = update_typestate_fld pvar fn typ in let typestate' = update_typestate_fld pvar inner_origin fn typ in
(Exp.Lvar pvar, typestate') (Exp.Lvar pvar, typestate')
| Exp.Lvar _ | Exp.Lfield _ when ComplexExpressions.all_nested_fields () -> | Exp.Lvar _ | Exp.Lfield _ when ComplexExpressions.all_nested_fields () ->
(* treat var.field1. ... .fieldn as a constant *) (* treat var.field1. ... .fieldn as a constant *)
@ -373,7 +386,7 @@ let typecheck_instr
match ComplexExpressions.exp_to_string tenv node' exp with match ComplexExpressions.exp_to_string tenv node' exp with
| Some exp_str -> | Some exp_str ->
let pvar = Pvar.mk (Mangled.from_string exp_str) curr_pname in let pvar = Pvar.mk (Mangled.from_string exp_str) curr_pname in
let typestate' = update_typestate_fld pvar fn typ in let typestate' = update_typestate_fld pvar inner_origin fn typ in
(Exp.Lvar pvar, typestate') (Exp.Lvar pvar, typestate')
| None -> | None ->
default default

@ -26,7 +26,7 @@ type proc_origin =
type t = type t =
| Const of Location.t | Const of Location.t
| Field of Ident.fieldname * Location.t | Field of t * Ident.fieldname * Location.t
| Formal of Mangled.t | Formal of Mangled.t
| Proc of proc_origin | Proc of proc_origin
| New | New
@ -36,17 +36,23 @@ type t =
let equal = [%compare.equal : t] let equal = [%compare.equal : t]
let to_string = function let rec to_string = function
| Const _ -> "Const" | Const _ ->
| Field (fn, _) -> "Field " ^ Ident.fieldname_to_simplified_string fn "Const"
| Formal s -> "Formal " ^ Mangled.to_string s | Field (o, fn, _) ->
"Field " ^ Ident.fieldname_to_simplified_string fn ^ (" (inner: " ^ to_string o ^ ")")
| Formal s ->
"Formal " ^ Mangled.to_string s
| Proc po -> | Proc po ->
Printf.sprintf Printf.sprintf
"Fun %s" "Fun %s"
(Procname.to_simplified_string po.pname) (Procname.to_simplified_string po.pname)
| New -> "New" | New ->
| ONone -> "ONone" "New"
| Undef -> "Undef" | ONone ->
"ONone"
| Undef ->
"Undef"
let get_description tenv origin = let get_description tenv origin =
let atline loc = let atline loc =
@ -54,7 +60,7 @@ let get_description tenv origin =
match origin with match origin with
| Const loc -> | Const loc ->
Some ("null constant" ^ atline loc, Some loc, None) Some ("null constant" ^ atline loc, Some loc, None)
| Field (fn, loc) -> | Field (_, fn, loc) ->
Some ("field " ^ Ident.fieldname_to_simplified_string fn ^ atline loc, Some loc, None) Some ("field " ^ Ident.fieldname_to_simplified_string fn ^ atline loc, Some loc, None)
| Formal s -> | Formal s ->
Some ("method parameter " ^ Mangled.to_string s, None, None) Some ("method parameter " ^ Mangled.to_string s, None, None)

@ -20,7 +20,7 @@ type proc_origin =
type t = type t =
| Const of Location.t (** A constant in the source *) | Const of Location.t (** A constant in the source *)
| Field of Ident.fieldname * Location.t (** A field access *) | Field of t * Ident.fieldname * Location.t (** A field access *)
| Formal of Mangled.t (** A formal parameter *) | Formal of Mangled.t (** A formal parameter *)
| Proc of proc_origin (** A procedure call *) | Proc of proc_origin (** A procedure call *)
| New (** A new object creation *) | New (** A new object creation *)

@ -138,4 +138,34 @@ public class FieldNotInitialized {
} }
} }
class InitCircular {
String s;
InitCircular() {
String tmp = s;
s = tmp; // s is not initialized: circular initialization
}
}
class InitWithOtherClass {
class OtherClass {
String s = "";
}
String s;
InitWithOtherClass(OtherClass x) {
s = x.s;
}
}
class InitWithThisClass {
String s;
InitWithThisClass(InitWithThisClass x) {
s = x.s;
}
}
} }

@ -1,5 +1,6 @@
codetoanalyze/java/eradicate/ActivityFieldNotInitialized.java, ActivityFieldNotInitialized$BadActivityWithOnCreate.<init>(ActivityFieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, [Field `ActivityFieldNotInitialized$BadActivityWithOnCreate.field` is not initialized in the constructor and is not declared `@Nullable`] codetoanalyze/java/eradicate/ActivityFieldNotInitialized.java, ActivityFieldNotInitialized$BadActivityWithOnCreate.<init>(ActivityFieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, [Field `ActivityFieldNotInitialized$BadActivityWithOnCreate.field` is not initialized in the constructor and is not declared `@Nullable`]
codetoanalyze/java/eradicate/FieldNotInitialized.java, FieldNotInitialized$ConditionalFieldInit.<init>(FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, [Field `FieldNotInitialized$ConditionalFieldInit.o1` is not initialized in the constructor and is not declared `@Nullable`] codetoanalyze/java/eradicate/FieldNotInitialized.java, FieldNotInitialized$ConditionalFieldInit.<init>(FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, [Field `FieldNotInitialized$ConditionalFieldInit.o1` is not initialized in the constructor and is not declared `@Nullable`]
codetoanalyze/java/eradicate/FieldNotInitialized.java, FieldNotInitialized$InitCircular.<init>(FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, [Field `FieldNotInitialized$InitCircular.s` is not initialized in the constructor and is not declared `@Nullable`]
codetoanalyze/java/eradicate/FieldNotInitialized.java, FieldNotInitialized$OnlyRead.<init>(FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, [Field `FieldNotInitialized$OnlyRead.o` is not initialized in the constructor and is not declared `@Nullable`] codetoanalyze/java/eradicate/FieldNotInitialized.java, FieldNotInitialized$OnlyRead.<init>(FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, [Field `FieldNotInitialized$OnlyRead.o` is not initialized in the constructor and is not declared `@Nullable`]
codetoanalyze/java/eradicate/FieldNotInitialized.java, FieldNotInitialized$OnlyReadIndirect.<init>(FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, [Field `FieldNotInitialized$OnlyReadIndirect.o1` is not initialized in the constructor and is not declared `@Nullable`] codetoanalyze/java/eradicate/FieldNotInitialized.java, FieldNotInitialized$OnlyReadIndirect.<init>(FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, [Field `FieldNotInitialized$OnlyReadIndirect.o1` is not initialized in the constructor and is not declared `@Nullable`]
codetoanalyze/java/eradicate/FieldNotInitialized.java, FieldNotInitialized$Swap.<init>(FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, [Field `FieldNotInitialized$Swap.o1` is not initialized in the constructor and is not declared `@Nullable`] codetoanalyze/java/eradicate/FieldNotInitialized.java, FieldNotInitialized$Swap.<init>(FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, [Field `FieldNotInitialized$Swap.o1` is not initialized in the constructor and is not declared `@Nullable`]

Loading…
Cancel
Save