Skip to content

Commit

Permalink
[multicore][summary] do not cache incomplete summaries
Browse files Browse the repository at this point in the history
Summary:
Caching an empty summary when the summary is first created creates the potential for races since the cache is shared.
This diff stops caching empty summaries and changes the logic of storing summaries so that mutations happen before caching.
Summaries remain mutable and specialisation, in particular, may still create problems, but this is for the future.

Reviewed By: jvillard

Differential Revision:
D68198006

Privacy Context Container: L1208441

fbshipit-source-id: 297e9e5941201810efb53b477ac9c169ce86f629
  • Loading branch information
ngorogiannis authored and facebook-github-bot committed Jan 15, 2025
1 parent 370f5fc commit ca4f3ff
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 18 deletions.
24 changes: 10 additions & 14 deletions infer/src/backend/Summary.ml
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,6 @@ module OnDisk = struct
(** Save summary for the procedure into the spec database *)
let rec store (analysis_req : AnalysisRequest.t)
({proc_name; dependencies; payloads} as summary : t) =
(* Make sure the summary in memory is identical to the saved one *)
add_to_cache proc_name analysis_req summary ;
summary.dependencies <- Dependencies.(Complete (freeze proc_name dependencies)) ;
let report_summary = ReportSummary.of_full_summary summary in
let summary_metadata =
Expand All @@ -304,6 +302,8 @@ module OnDisk = struct
~merge_pulse_payload:(Payloads.SQLite.serialize payloads)
~merge_report_summary:(ReportSummary.SQLite.serialize report_summary)
~merge_summary_metadata:(SummaryMetadata.SQLite.serialize summary_metadata) ;
(* Make sure the summary in memory is identical to the saved one *)
add_to_cache proc_name analysis_req summary ;
summary
with SqliteUtils.DataTooBig ->
(* Serialization exceeded size limits, write and return an empty summary *)
Expand All @@ -317,18 +317,14 @@ module OnDisk = struct
store analysis_req new_summary


let reset proc_name analysis_req =
let summary =
{ sessions= 0
; payloads= Payloads.empty
; stats= Stats.empty
; proc_name
; err_log= Errlog.empty ()
; dependencies= Dependencies.reset proc_name
; is_complete_result= false }
in
add_to_cache proc_name analysis_req summary ;
summary
let empty proc_name =
{ sessions= 0
; payloads= Payloads.empty
; stats= Stats.empty
; proc_name
; err_log= Errlog.empty ()
; dependencies= Dependencies.reset proc_name
; is_complete_result= false }


let delete_all ~procedures =
Expand Down
4 changes: 2 additions & 2 deletions infer/src/backend/Summary.mli
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ module OnDisk : sig
val get : lazy_payloads:bool -> AnalysisRequest.t -> Procname.t -> t option
(** Return the summary option for the procedure name *)

val reset : Procname.t -> AnalysisRequest.t -> t
(** Reset a summary rebuilding the dependents and preserving the proc attributes if present. *)
val empty : Procname.t -> t
(** Empty summary. *)

val store : AnalysisRequest.t -> t -> t
(** Save summary for the procedure into the spec database and return it. If the operation fails,
Expand Down
2 changes: 1 addition & 1 deletion infer/src/backend/ondemand.ml
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ let run_proc_analysis exe_env tenv analysis_req specialization_context ?caller_p
| None ->
if Config.debug_mode then
DotCfg.emit_proc_desc callee_attributes.translation_unit callee_pdesc |> ignore ;
Summary.OnDisk.reset callee_pname analysis_req
Summary.OnDisk.empty callee_pname
| Some (current_summary, _) ->
current_summary
in
Expand Down
2 changes: 1 addition & 1 deletion infer/src/unit/analyzerTester.ml
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ struct
set_succs last_node [exit_node] ~exn_handlers:no_exn_handlers ;
Procdesc.set_exit_node pdesc exit_node ;
Cfg.store src_file cfg ;
(Summary.OnDisk.reset pname AnalysisRequest.all, assert_map, pdesc)
(Summary.OnDisk.empty pname, assert_map, pdesc)


let create_test test_program make_analysis_data ~initial pp_opt _ =
Expand Down

0 comments on commit ca4f3ff

Please sign in to comment.