From 919d05b5ef10c8f645948e587097262d57809c82 Mon Sep 17 00:00:00 2001 From: Mehdi Bouaziz Date: Wed, 15 Aug 2018 08:13:27 -0700 Subject: [PATCH] Reporting cleanup 3: NodeKey Summary: - abstracted the type for a node key - moved it to its own module with an ugly `compute` to avoid cyclic dependencies... - renamed `node_id` to `node_id_key` where needed - moved key computation from `State` to `Procdesc.Node` Reviewed By: jvillard Differential Revision: D9332803 fbshipit-source-id: fe1ae8c1c --- infer/src/IR/Errlog.ml | 7 ++-- infer/src/IR/Errlog.mli | 4 +- infer/src/IR/NodeKey.ml | 19 ++++++++++ infer/src/IR/NodeKey.mli | 17 +++++++++ infer/src/IR/Procdesc.ml | 35 +++++++++++++++++ infer/src/IR/Procdesc.mli | 2 + infer/src/backend/InferPrint.ml | 2 +- infer/src/backend/reporting.ml | 36 ++++++++---------- infer/src/backend/reporting.mli | 4 +- infer/src/biabduction/State.ml | 58 +++++------------------------ infer/src/biabduction/State.mli | 4 +- infer/src/clang/cFrontend_errors.ml | 5 ++- 12 files changed, 112 insertions(+), 81 deletions(-) create mode 100644 infer/src/IR/NodeKey.ml create mode 100644 infer/src/IR/NodeKey.mli diff --git a/infer/src/IR/Errlog.ml b/infer/src/IR/Errlog.ml index 0cff9f8f9..e2db96354 100644 --- a/infer/src/IR/Errlog.ml +++ b/infer/src/IR/Errlog.ml @@ -60,7 +60,7 @@ let compute_local_exception_line loc_trace = List.fold_until ~init:(None, None) ~f:compute_local_exception_line ~finish:snd loc_trace -type node_id_key = {node_id: int; node_key: Caml.Digest.t} +type node_id_key = {node_id: int; node_key: NodeKey.t} type err_key = { severity: Exceptions.severity @@ -213,8 +213,8 @@ let update errlog_old errlog_new = ErrLogHash.iter (fun err_key l -> ignore (add_issue errlog_old err_key l)) errlog_new -let log_issue procname ~clang_method_kind severity err_log ~loc ~node_id:(node_id, node_key) - ~session ~ltr ~linters_def_file ~doc_url ~access ~extras exn = +let log_issue procname ~clang_method_kind severity err_log ~loc ~node_id_key ~session ~ltr + ~linters_def_file ~doc_url ~access ~extras exn = let lang = Typ.Procname.get_language procname in let error = Exceptions.recognize_exception exn in let severity = Option.value error.severity ~default:severity in @@ -254,7 +254,6 @@ let log_issue procname ~clang_method_kind severity err_log ~loc ~node_id:(node_i EventLogger.log issue ) ; if should_report && not hide_java_loc_zero && not hide_memory_error then let added = - let node_id_key = {node_id; node_key} in let err_data = { node_id_key ; session diff --git a/infer/src/IR/Errlog.mli b/infer/src/IR/Errlog.mli index bbf87b993..1e6cab3dd 100644 --- a/infer/src/IR/Errlog.mli +++ b/infer/src/IR/Errlog.mli @@ -34,7 +34,7 @@ val compute_local_exception_line : loc_trace -> int option This extra information adds value to the report itself, and may avoid digging into the trace to understand the cause of the report. *) -type node_id_key = private {node_id: int; node_key: Caml.Digest.t} +type node_id_key = {node_id: int; node_key: NodeKey.t} type err_key = private { severity: Exceptions.severity @@ -94,5 +94,5 @@ val update : t -> t -> unit val log_issue : Typ.Procname.t -> clang_method_kind:string option -> Exceptions.severity -> t -> loc:Location.t - -> node_id:int * Caml.Digest.t -> session:int -> ltr:loc_trace -> linters_def_file:string option + -> node_id_key:node_id_key -> session:int -> ltr:loc_trace -> linters_def_file:string option -> doc_url:string option -> access:string option -> extras:Jsonbug_t.extra option -> exn -> unit diff --git a/infer/src/IR/NodeKey.ml b/infer/src/IR/NodeKey.ml new file mode 100644 index 000000000..59d38737d --- /dev/null +++ b/infer/src/IR/NodeKey.ml @@ -0,0 +1,19 @@ +(* + * Copyright (c) 2018-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + *) + +open! IStd + +type t = Caml.Digest.t + +let to_string = Caml.Digest.to_hex + +let compute node ~simple_key ~succs ~preds = + let v = (simple_key node, List.rev_map ~f:simple_key succs, List.rev_map ~f:simple_key preds) in + Utils.better_hash v + + +let of_frontend_node_key = Utils.better_hash diff --git a/infer/src/IR/NodeKey.mli b/infer/src/IR/NodeKey.mli new file mode 100644 index 000000000..feab333ac --- /dev/null +++ b/infer/src/IR/NodeKey.mli @@ -0,0 +1,17 @@ +(* + * Copyright (c) 2018-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + *) + +open! IStd + +type t + +val to_string : t -> string + +val compute : + 'node -> simple_key:('node -> 'simple_key) -> succs:'node list -> preds:'node list -> t + +val of_frontend_node_key : string -> t diff --git a/infer/src/IR/Procdesc.ml b/infer/src/IR/Procdesc.ml index bdf96d82e..140f68c32 100644 --- a/infer/src/IR/Procdesc.ml +++ b/infer/src/IR/Procdesc.ml @@ -331,6 +331,41 @@ module Node = struct in let pp fmt = F.fprintf fmt "%s@.%a" str (pp_instrs pe ~instro:None ~sub_instrs:true) node in F.asprintf "%t" pp + + + (** simple key for a node: just look at the instructions *) + let simple_key node = + let add_instr instr = + if Sil.instr_is_auxiliary instr then None + else + let instr_key = + match instr with + | Sil.Load _ -> + 1 + | Sil.Store _ -> + 2 + | Sil.Prune _ -> + 3 + | Sil.Call _ -> + 4 + | Sil.Nullify _ -> + 5 + | Sil.Abstract _ -> + 6 + | Sil.Remove_temps _ -> + 7 + in + Some instr_key + in + get_instrs node |> IContainer.rev_filter_map_to_list ~fold:Instrs.fold ~f:add_instr + |> Utils.better_hash + + + (** key for a node: look at the current node, successors and predecessors *) + let compute_key node = + let succs = get_succs node in + let preds = get_preds node in + NodeKey.compute node ~simple_key ~succs ~preds end (* =============== END of module Node =============== *) diff --git a/infer/src/IR/Procdesc.mli b/infer/src/IR/Procdesc.mli index 7223aa9da..b9693edd5 100644 --- a/infer/src/IR/Procdesc.mli +++ b/infer/src/IR/Procdesc.mli @@ -146,6 +146,8 @@ module Node : sig Pp.env -> sub_instrs:bool -> instro:Sil.instr option -> Format.formatter -> t -> unit (** Print extended instructions for the node, highlighting the given subinstruction if present *) + + val compute_key : t -> NodeKey.t end (** Map with node id keys. *) diff --git a/infer/src/backend/InferPrint.ml b/infer/src/backend/InferPrint.ml index 3fefe342c..536e033cc 100644 --- a/infer/src/backend/InferPrint.ml +++ b/infer/src/backend/InferPrint.ml @@ -315,7 +315,7 @@ module JsonIssuePrinter = MakeJsonListPrinter (struct ; procedure_start_line ; file ; bug_trace= loc_trace_to_jsonbug_record err_data.loc_trace err_key.severity - ; node_key= err_data.node_id_key.node_key |> Caml.Digest.to_hex + ; node_key= err_data.node_id_key.node_key |> NodeKey.to_string ; key= compute_key bug_type proc_name file ; hash= compute_hash severity bug_type proc_name file qualifier ; dotty= error_desc_to_dotty_string err_key.err_desc diff --git a/infer/src/backend/reporting.ml b/infer/src/backend/reporting.ml index e21d19c60..8c0844a66 100644 --- a/infer/src/backend/reporting.ml +++ b/infer/src/backend/reporting.ml @@ -9,27 +9,27 @@ open! IStd module L = Logging type log_t = - ?loc:Location.t -> ?node_id:int * Caml.Digest.t -> ?session:int -> ?ltr:Errlog.loc_trace + ?loc:Location.t -> ?node_id_key:Errlog.node_id_key -> ?session:int -> ?ltr:Errlog.loc_trace -> ?linters_def_file:string -> ?doc_url:string -> ?access:string -> ?extras:Jsonbug_t.extra -> exn -> unit -let log_issue_from_errlog_internal procname ~clang_method_kind severity err_log ~loc ~node_id +let log_issue_from_errlog_internal procname ~clang_method_kind severity err_log ~loc ~node_id_key ~session ~ltr ~linters_def_file ~doc_url ~access ~extras exn = let issue_type = (Exceptions.recognize_exception exn).name in if not Config.filtering (* no-filtering takes priority *) || issue_type.IssueType.enabled then - Errlog.log_issue procname ~clang_method_kind severity err_log ~loc ~node_id ~session ~ltr + Errlog.log_issue procname ~clang_method_kind severity err_log ~loc ~node_id_key ~session ~ltr ~linters_def_file ~doc_url ~access ~extras exn -let log_issue_from_errlog procname severity errlog ~loc ~node_id ~ltr ~linters_def_file ~doc_url - exn = +let log_issue_from_errlog procname severity errlog ~loc ~node_id_key ~ltr ~linters_def_file + ~doc_url exn = let session = (State.get_session () :> int) in - log_issue_from_errlog_internal procname ~clang_method_kind:None severity errlog ~loc ~node_id + log_issue_from_errlog_internal procname ~clang_method_kind:None severity errlog ~loc ~node_id_key ~session ~ltr ~linters_def_file ~doc_url ~access:None ~extras:None exn -let log_issue_from_summary severity summary ?loc ?node_id ?session ?ltr ?linters_def_file ?doc_url - ?access ?extras exn = +let log_issue_from_summary severity summary ?loc ?node_id_key ?session ?ltr ?linters_def_file + ?doc_url ?access ?extras exn = let attrs = Summary.get_attributes summary in let procname = attrs.proc_name in let is_java_generated_method = @@ -51,26 +51,22 @@ let log_issue_from_summary severity summary ?loc ?node_id ?session ?ltr ?linters Some (ProcAttributes.string_of_clang_method_kind attrs.clang_method_kind) in let loc = match loc with None -> State.get_loc () | Some loc -> loc in - let node_id = - match node_id with - | None -> - (State.get_node_id_key () :> int * Caml.Digest.t) - | Some node_id -> - node_id + let node_id_key = + match node_id_key with None -> State.get_node_id_key () | Some node_id_key -> node_id_key in let session = match session with None -> (State.get_session () :> int) | Some session -> session in let ltr = match ltr with None -> State.get_loc_trace () | Some ltr -> ltr in - log_issue_from_errlog_internal procname ~clang_method_kind severity err_log ~loc ~node_id + log_issue_from_errlog_internal procname ~clang_method_kind severity err_log ~loc ~node_id_key ~session ~ltr ~linters_def_file ~doc_url ~access ~extras exn -let log_issue_deprecated severity proc_name ?loc ?node_id ?session ?ltr ?linters_def_file ?doc_url - ?access ?extras:_ exn = +let log_issue_deprecated severity proc_name ?loc ?node_id_key ?session ?ltr ?linters_def_file + ?doc_url ?access ?extras:_ exn = match Summary.get proc_name with | Some summary -> - log_issue_from_summary severity summary ?loc ?node_id ?session ?ltr ?linters_def_file + log_issue_from_summary severity summary ?loc ?node_id_key ?session ?ltr ?linters_def_file ?doc_url ?access exn | None -> L.(die InternalError) @@ -85,9 +81,9 @@ let log_warning = log_issue_from_summary Exceptions.Warning let log_issue_external procname severity ~loc ~ltr ?access exn = let errlog = IssueLog.get_errlog procname in - let node_id = (State.get_node_id_key () :> int * Caml.Digest.t) in + let node_id_key = State.get_node_id_key () in let session = (State.get_session () :> int) in - log_issue_from_errlog_internal procname ~clang_method_kind:None severity errlog ~loc ~node_id + log_issue_from_errlog_internal procname ~clang_method_kind:None severity errlog ~loc ~node_id_key ~session ~ltr ~linters_def_file:None ~doc_url:None ~access ~extras:None exn diff --git a/infer/src/backend/reporting.mli b/infer/src/backend/reporting.mli index 4bade88f4..38854253b 100644 --- a/infer/src/backend/reporting.mli +++ b/infer/src/backend/reporting.mli @@ -10,7 +10,7 @@ open! IStd (** Type of functions to report issues to the error_log in a spec. *) type log_t = - ?loc:Location.t -> ?node_id:int * Caml.Digest.t -> ?session:int -> ?ltr:Errlog.loc_trace + ?loc:Location.t -> ?node_id_key:Errlog.node_id_key -> ?session:int -> ?ltr:Errlog.loc_trace -> ?linters_def_file:string -> ?doc_url:string -> ?access:string -> ?extras:Jsonbug_t.extra -> exn -> unit @@ -21,7 +21,7 @@ val log_issue_deprecated : Exceptions.severity -> Typ.Procname.t -> log_t val log_issue_from_errlog : Typ.Procname.t -> Exceptions.severity -> Errlog.t -> loc:Location.t - -> node_id:int * Caml.Digest.t -> ltr:Errlog.loc_trace -> linters_def_file:string option + -> node_id_key:Errlog.node_id_key -> ltr:Errlog.loc_trace -> linters_def_file:string option -> doc_url:string option -> exn -> unit (** Report an issue of a given kind in the given error log. *) diff --git a/infer/src/biabduction/State.ml b/infer/src/biabduction/State.ml index 599cb5cdc..64a2cfd1f 100644 --- a/infer/src/biabduction/State.ml +++ b/infer/src/biabduction/State.ml @@ -20,8 +20,7 @@ type failure_stats = ; (* number of node failures (i.e. at least one instruction failure) *) mutable node_ok: int ; (* number of node successes (i.e. no instruction failures) *) - mutable first_failure: - (Location.t * (int * Caml.Digest.t) * int * Errlog.loc_trace * exn) option + mutable first_failure: (Location.t * Errlog.node_id_key * int * Errlog.loc_trace * exn) option (* exception at the first failure *) } module NodeHash = Procdesc.NodeHash @@ -95,46 +94,6 @@ let get_loc () = let get_node () = !gs.last_node -(** simple key for a node: just look at the instructions *) -let node_simple_key node = - let add_instr instr = - if Sil.instr_is_auxiliary instr then None - else - let instr_key = - match instr with - | Sil.Load _ -> - 1 - | Sil.Store _ -> - 2 - | Sil.Prune _ -> - 3 - | Sil.Call _ -> - 4 - | Sil.Nullify _ -> - 5 - | Sil.Abstract _ -> - 6 - | Sil.Remove_temps _ -> - 7 - in - Some instr_key - in - Procdesc.Node.get_instrs node |> IContainer.rev_filter_map_to_list ~fold:Instrs.fold ~f:add_instr - |> Utils.better_hash - - -(** key for a node: look at the current node, successors and predecessors *) -let node_key node = - let succs = Procdesc.Node.get_succs node in - let preds = Procdesc.Node.get_preds node in - let v = - ( node_simple_key node - , List.rev_map ~f:node_simple_key succs - , List.rev_map ~f:node_simple_key preds ) - in - Utils.better_hash v - - (** normalize the list of instructions by renaming let-bound ids *) let instrs_normalize instrs = let bound_ids = @@ -205,7 +164,10 @@ let mk_find_duplicate_nodes : Procdesc.t -> Procdesc.Node.t -> Procdesc.NodeSet. let get_node_id () = Procdesc.Node.get_id !gs.last_node -let get_node_id_key () = (Procdesc.Node.get_id !gs.last_node, node_key !gs.last_node) +let get_node_id_key () = + { Errlog.node_id= (Procdesc.Node.get_id !gs.last_node :> int) + ; node_key= Procdesc.Node.compute_key !gs.last_node } + let get_inst_update pos = let loc = get_loc () in @@ -289,17 +251,17 @@ let mark_instr_ok () = let mark_instr_fail exn = let loc = get_loc () in - let key = (get_node_id_key () :> int * Caml.Digest.t) in + let node_id_key = get_node_id_key () in let session = get_session () in let loc_trace = get_loc_trace () in let fs = get_failure_stats (get_node ()) in if is_none fs.first_failure then - fs.first_failure <- Some (loc, key, (session :> int), loc_trace, exn) ; + fs.first_failure <- Some (loc, node_id_key, (session :> int), loc_trace, exn) ; fs.instr_fail <- fs.instr_fail + 1 type log_issue = - Typ.Procname.t -> ?loc:Location.t -> ?node_id:int * Caml.Digest.t -> ?session:int + Typ.Procname.t -> ?loc:Location.t -> ?node_id_key:Errlog.node_id_key -> ?session:int -> ?ltr:Errlog.loc_trace -> ?linters_def_file:string -> ?doc_url:string -> ?access:string -> ?extras:Jsonbug_t.extra -> exn -> unit @@ -307,11 +269,11 @@ let process_execution_failures (log_issue: log_issue) pname = let do_failure _ fs = (* L.out "Node:%a node_ok:%d node_fail:%d@." Procdesc.Node.pp node fs.node_ok fs.node_fail; *) match (fs.node_ok, fs.first_failure) with - | 0, Some (loc, key, _, loc_trace, exn) when not Config.debug_exceptions -> + | 0, Some (loc, node_id_key, _, loc_trace, exn) when not Config.debug_exceptions -> let error = Exceptions.recognize_exception exn in let desc' = Localise.verbatim_desc ("exception: " ^ error.name.IssueType.unique_id) in let exn' = Exceptions.Analysis_stops (desc', error.ocaml_pos) in - log_issue pname ~loc ~node_id:key ~ltr:loc_trace exn' + log_issue pname ~loc ~node_id_key ~ltr:loc_trace exn' | _ -> () in diff --git a/infer/src/biabduction/State.mli b/infer/src/biabduction/State.mli index ad9abf3a2..98ca6aafc 100644 --- a/infer/src/biabduction/State.mli +++ b/infer/src/biabduction/State.mli @@ -37,7 +37,7 @@ val get_loc_trace : unit -> Errlog.loc_trace val get_node : unit -> Procdesc.Node.t (** Get last node seen in symbolic execution *) -val get_node_id_key : unit -> Procdesc.Node.id * Caml.Digest.t +val get_node_id_key : unit -> Errlog.node_id_key (** Get id and key of last node seen in symbolic execution *) val get_normalized_pre : @@ -75,7 +75,7 @@ val mk_find_duplicate_nodes : Procdesc.t -> Procdesc.Node.t -> Procdesc.NodeSet. and normalized (w.r.t. renaming of let - bound ids) list of instructions. *) type log_issue = - Typ.Procname.t -> ?loc:Location.t -> ?node_id:int * Caml.Digest.t -> ?session:int + Typ.Procname.t -> ?loc:Location.t -> ?node_id_key:Errlog.node_id_key -> ?session:int -> ?ltr:Errlog.loc_trace -> ?linters_def_file:string -> ?doc_url:string -> ?access:string -> ?extras:Jsonbug_t.extra -> exn -> unit diff --git a/infer/src/clang/cFrontend_errors.ml b/infer/src/clang/cFrontend_errors.ml index 50041cefa..6ceb37b63 100644 --- a/infer/src/clang/cFrontend_errors.ml +++ b/infer/src/clang/cFrontend_errors.ml @@ -456,9 +456,10 @@ let log_frontend_issue method_decl_opt (node: Ctl_parser_types.ast_node) | Stmt st -> CAst_utils.generate_key_stmt st in - let key = Utils.better_hash key_str in + let node_key = NodeKey.of_frontend_node_key key_str in Reporting.log_issue_from_errlog procname issue_desc.severity errlog exn ~loc:issue_desc.loc - ~ltr:trace ~node_id:(0, key) ~linters_def_file ~doc_url:issue_desc.doc_url + ~ltr:trace ~node_id_key:{Errlog.node_id= 0; node_key} ~linters_def_file + ~doc_url:issue_desc.doc_url let fill_issue_desc_info_and_log context ~witness ~current_node (issue_desc: CIssue.issue_desc)