diff --git a/infer/src/pulse/Pulse.ml b/infer/src/pulse/Pulse.ml index a98106ec9..f05c3bcc8 100644 --- a/infer/src/pulse/Pulse.ml +++ b/infer/src/pulse/Pulse.ml @@ -70,6 +70,10 @@ module PulseTransferFunctions = struct ~ret ~actuals ~formals_opt astate | _ -> L.d_printfln "Skipping indirect call %a@\n" Exp.pp call_exp ; + let astate = + let arg_values = List.map actuals ~f:(fun ((value, _), _) -> value) in + PulseOperations.conservatively_initialize_args arg_values astate + in Ok [ Domain.ContinueProgram (PulseOperations.unknown_call call_loc (SkippedUnknownCall call_exp) ~ret ~actuals @@ -138,16 +142,6 @@ module PulseTransferFunctions = struct astate - (* NOTE: At the moment, it conservatively assumes that all reachable addresses from function - arguments are initialized by callee. *) - let conservatively_initialize_args func_args ({AbductiveDomain.post} as astate) = - let arg_values = - List.map func_args ~f:(fun {ProcnameDispatcher.Call.FuncArg.arg_payload= value, _} -> value) - in - let reachable_values = BaseDomain.reachable_addresses_from arg_values (post :> BaseDomain.t) in - AbstractValue.Set.fold AbductiveDomain.initialize reachable_values astate - - let dispatch_call ({InterproceduralAnalysis.proc_desc; tenv} as analysis_data) ret call_exp actuals call_loc flags astate = (* evaluate all actuals *) @@ -161,7 +155,6 @@ module PulseTransferFunctions = struct :: rev_func_args ) ) in let func_args = List.rev rev_func_args in - let astate = conservatively_initialize_args func_args astate in let callee_pname = proc_name_of_call call_exp in let model = match callee_pname with @@ -177,6 +170,13 @@ module PulseTransferFunctions = struct match model with | Some (model, callee_procname) -> L.d_printfln "Found model for call@\n" ; + let astate = + let arg_values = + List.map func_args ~f:(fun {ProcnameDispatcher.Call.FuncArg.arg_payload= value, _} -> + value ) + in + PulseOperations.conservatively_initialize_args arg_values astate + in model analysis_data ~callee_procname call_loc ~ret astate | None -> PerfEvent.(log (fun logger -> log_begin_event logger ~name:"pulse interproc call" ())) ; diff --git a/infer/src/pulse/PulseAbductiveDomain.ml b/infer/src/pulse/PulseAbductiveDomain.ml index 42578cad9..99188ef52 100644 --- a/infer/src/pulse/PulseAbductiveDomain.ml +++ b/infer/src/pulse/PulseAbductiveDomain.ml @@ -118,6 +118,8 @@ let leq ~lhs ~rhs = ~rhs:(rhs.post :> BaseDomain.t) +let initialize address astate = {astate with post= PostDomain.initialize address astate.post} + module Stack = struct let is_abducible astate var = (* HACK: formals are pre-registered in the initial state *) @@ -202,9 +204,14 @@ module AddressAttributes = struct abduce_attribute addr (MustBeValid access_trace) astate - let check_initialized addr astate = - let+ () = BaseAddressAttributes.check_initialized addr (astate.post :> base_domain).attrs in - () + let check_initialized access_trace addr astate = + let attrs = (astate.post :> base_domain).attrs in + let+ () = BaseAddressAttributes.check_initialized addr attrs in + let is_written_to = + Option.exists (BaseAddressAttributes.find_opt addr attrs) ~f:(fun attrs -> + Attribute.Attributes.get_written_to attrs |> Option.is_some ) + in + if is_written_to then astate else abduce_attribute addr (MustBeInitialized access_trace) astate (** [astate] with [astate.post.attrs = f astate.post.attrs] *) @@ -258,7 +265,8 @@ module AddressAttributes = struct let astate = if Attribute.is_suitable_for_pre attr then abduce_attribute value attr astate else astate in - add_one value attr astate ) + let astate = add_one value attr astate in + match attr with Attribute.WrittenTo _ -> initialize value astate | _ -> astate ) let find_opt address astate = @@ -577,8 +585,6 @@ let get_pre {pre} = (pre :> BaseDomain.t) let get_post {post} = (post :> BaseDomain.t) -let initialize address x = {x with post= PostDomain.initialize address x.post} - (* re-exported for mli *) let incorporate_new_eqs astate (phi, new_eqs) = if PathCondition.is_unsat_cheap phi then phi diff --git a/infer/src/pulse/PulseAbductiveDomain.mli b/infer/src/pulse/PulseAbductiveDomain.mli index c35684fad..c405687ae 100644 --- a/infer/src/pulse/PulseAbductiveDomain.mli +++ b/infer/src/pulse/PulseAbductiveDomain.mli @@ -126,7 +126,7 @@ module AddressAttributes : sig val check_valid : Trace.t -> AbstractValue.t -> t -> (t, Invalidation.t * Trace.t) result - val check_initialized : AbstractValue.t -> t -> (unit, Trace.t) result + val check_initialized : Trace.t -> AbstractValue.t -> t -> (t, Trace.t) result val invalidate : AbstractValue.t * ValueHistory.t -> Invalidation.t -> Location.t -> t -> t diff --git a/infer/src/pulse/PulseAttribute.ml b/infer/src/pulse/PulseAttribute.ml index a258e7064..e4bd88fb2 100644 --- a/infer/src/pulse/PulseAttribute.ml +++ b/infer/src/pulse/PulseAttribute.ml @@ -32,6 +32,7 @@ module Attribute = struct | DynamicType of Typ.Name.t | EndOfCollection | Invalid of Invalidation.t * Trace.t + | MustBeInitialized of Trace.t | MustBeValid of Trace.t | StdVectorReserve | Uninitialized of Trace.t @@ -73,6 +74,8 @@ module Attribute = struct let uninitialized_rank = Variants.to_rank (Uninitialized dummy_trace) + let must_be_initialized_rank = Variants.to_rank (MustBeInitialized dummy_trace) + let pp f attribute = let pp_string_if_debug string fmt = if Config.debug_level_analysis >= 3 then F.pp_print_string fmt string @@ -97,6 +100,10 @@ module Attribute = struct F.fprintf f "Invalid %a" (Trace.pp ~pp_immediate:(fun fmt -> Invalidation.pp fmt invalidation)) trace + | MustBeInitialized trace -> + F.fprintf f "MustBeInitialized %a" + (Trace.pp ~pp_immediate:(pp_string_if_debug "read")) + trace | MustBeValid trace -> F.fprintf f "MustBeValid %a" (Trace.pp ~pp_immediate:(pp_string_if_debug "access")) trace | StdVectorReserve -> @@ -181,13 +188,20 @@ module Attributes = struct trace ) + let get_must_be_initialized attrs = + Set.find_rank attrs Attribute.must_be_initialized_rank + |> Option.map ~f:(fun attr -> + let[@warning "-8"] (Attribute.MustBeInitialized trace) = attr in + trace ) + + include Set end include Attribute let is_suitable_for_pre = function - | MustBeValid _ -> + | MustBeValid _ | MustBeInitialized _ -> true | AddressOfCppTemporary _ | AddressOfStackVariable _ @@ -213,6 +227,8 @@ let map_trace ~f = function Uninitialized (f trace) | WrittenTo trace -> WrittenTo (f trace) + | MustBeInitialized trace -> + MustBeInitialized (f trace) | ( AddressOfCppTemporary _ | AddressOfStackVariable _ | Closure _ diff --git a/infer/src/pulse/PulseAttribute.mli b/infer/src/pulse/PulseAttribute.mli index df4715635..d6d3cf071 100644 --- a/infer/src/pulse/PulseAttribute.mli +++ b/infer/src/pulse/PulseAttribute.mli @@ -19,6 +19,7 @@ type t = | DynamicType of Typ.Name.t | EndOfCollection | Invalid of Invalidation.t * Trace.t + | MustBeInitialized of Trace.t | MustBeValid of Trace.t | StdVectorReserve | Uninitialized of Trace.t @@ -56,4 +57,6 @@ module Attributes : sig val is_std_vector_reserved : t -> bool val get_uninitialized : t -> Trace.t option + + val get_must_be_initialized : t -> Trace.t option end diff --git a/infer/src/pulse/PulseBaseAddressAttributes.ml b/infer/src/pulse/PulseBaseAddressAttributes.ml index 8adc58335..7ef3783be 100644 --- a/infer/src/pulse/PulseBaseAddressAttributes.ml +++ b/infer/src/pulse/PulseBaseAddressAttributes.ml @@ -119,6 +119,8 @@ let get_closure_proc_name = get_attribute Attributes.get_closure_proc_name let get_must_be_valid = get_attribute Attributes.get_must_be_valid +let get_must_be_initialized = get_attribute Attributes.get_must_be_initialized + let std_vector_reserve address memory = add_one address Attribute.StdVectorReserve memory let is_end_of_collection address attrs = diff --git a/infer/src/pulse/PulseBaseAddressAttributes.mli b/infer/src/pulse/PulseBaseAddressAttributes.mli index e040d3b9c..4d15d9ec2 100644 --- a/infer/src/pulse/PulseBaseAddressAttributes.mli +++ b/infer/src/pulse/PulseBaseAddressAttributes.mli @@ -39,6 +39,8 @@ val get_closure_proc_name : AbstractValue.t -> t -> Procname.t option val get_must_be_valid : AbstractValue.t -> t -> Trace.t option +val get_must_be_initialized : AbstractValue.t -> t -> Trace.t option + val std_vector_reserve : AbstractValue.t -> t -> t val is_std_vector_reserved : AbstractValue.t -> t -> bool diff --git a/infer/src/pulse/PulseInterproc.ml b/infer/src/pulse/PulseInterproc.ml index 2fdd9d4e7..223fa84f3 100644 --- a/infer/src/pulse/PulseInterproc.ml +++ b/infer/src/pulse/PulseInterproc.ml @@ -515,27 +515,42 @@ let apply_post callee_proc_name call_location pre_post ~captured_vars_with_actua let check_all_valid callee_proc_name call_location {AbductiveDomain.pre; _} call_state = AddressMap.fold (fun addr_pre (addr_caller, hist_caller) astate_result -> - match astate_result with - | Error _ -> - astate_result - | Ok astate -> ( + let mk_access_trace callee_access_trace = + Trace.ViaCall + { in_call= callee_access_trace + ; f= Call callee_proc_name + ; location= call_location + ; history= hist_caller } + in + let open IResult.Let_syntax in + let* astate = astate_result in + let* astate = match BaseAddressAttributes.get_must_be_valid addr_pre (pre :> BaseDomain.t).attrs with | None -> astate_result | Some callee_access_trace -> - let access_trace = - Trace.ViaCall - { in_call= callee_access_trace - ; f= Call callee_proc_name - ; location= call_location - ; history= hist_caller } - in + let access_trace = mk_access_trace callee_access_trace in AddressAttributes.check_valid access_trace addr_caller astate |> Result.map_error ~f:(fun (invalidation, invalidation_trace) -> L.d_printfln "ERROR: caller's %a invalid!" AbstractValue.pp addr_caller ; ( Diagnostic.AccessToInvalidAddress {calling_context= []; invalidation; invalidation_trace; access_trace} - , astate ) ) ) ) + , astate ) ) + in + match BaseAddressAttributes.get_must_be_initialized addr_pre (pre :> BaseDomain.t).attrs with + | None -> + astate_result + | Some callee_access_trace -> + let access_trace = mk_access_trace callee_access_trace in + AddressAttributes.check_initialized access_trace addr_caller astate + |> Result.map_error ~f:(fun invalidation_trace -> + L.d_printfln "ERROR: caller's %a is uninitialized!" AbstractValue.pp addr_caller ; + ( Diagnostic.AccessToInvalidAddress + { calling_context= [] + ; invalidation= Uninitialized + ; invalidation_trace + ; access_trace } + , astate ) ) ) call_state.subst (Ok call_state.astate) diff --git a/infer/src/pulse/PulseOperations.ml b/infer/src/pulse/PulseOperations.ml index 246f898d4..7306596cc 100644 --- a/infer/src/pulse/PulseOperations.ml +++ b/infer/src/pulse/PulseOperations.ml @@ -27,8 +27,8 @@ let check_addr_access access_mode location (address, history) astate = in match access_mode with | Read -> - let+ () = - AddressAttributes.check_initialized address astate + let+ astate = + AddressAttributes.check_initialized access_trace address astate |> Result.map_error ~f:(fun invalidation_trace -> ( Diagnostic.AccessToInvalidAddress { calling_context= [] @@ -533,10 +533,24 @@ let get_captured_actuals location ~captured_vars ~actual_closure astate = (astate, captured_vars_with_actuals) +let conservatively_initialize_args arg_values ({AbductiveDomain.post} as astate) = + let reachable_values = BaseDomain.reachable_addresses_from arg_values (post :> BaseDomain.t) in + AbstractValue.Set.fold AbductiveDomain.initialize reachable_values astate + + let call ~caller_proc_desc err_log ~(callee_data : (Procdesc.t * PulseSummary.t) option) call_loc callee_pname ~ret ~actuals ~formals_opt (astate : AbductiveDomain.t) = + let get_arg_values () = List.map actuals ~f:(fun ((value, _), _) -> value) in match callee_data with | Some (callee_proc_desc, exec_states) -> + let astate = + (* NOTE: This conservatively initializes all reachable addresses from captured variables + when calling ObjC blocks, because the captured variables with call-by-reference in ObjC + are incorrectly translated in the frontend. See T80743637. *) + if Procname.is_objc_block callee_pname then + conservatively_initialize_args (get_arg_values ()) astate + else astate + in let formals = Procdesc.get_formals callee_proc_desc |> List.map ~f:(fun (mangled, _) -> Pvar.mk mangled callee_pname |> Var.of_pvar) @@ -586,5 +600,6 @@ let call ~caller_proc_desc err_log ~(callee_data : (Procdesc.t * PulseSummary.t) | None -> (* no spec found for some reason (unknown function, ...) *) L.d_printfln "No spec found for %a@\n" Procname.pp callee_pname ; + let astate = conservatively_initialize_args (get_arg_values ()) astate in unknown_call call_loc (SkippedKnownCall callee_pname) ~ret ~actuals ~formals_opt astate |> fun astate -> Ok [ExecutionDomain.ContinueProgram astate] diff --git a/infer/src/pulse/PulseOperations.mli b/infer/src/pulse/PulseOperations.mli index 215b9a207..181883fa3 100644 --- a/infer/src/pulse/PulseOperations.mli +++ b/infer/src/pulse/PulseOperations.mli @@ -156,3 +156,5 @@ val unknown_call : -> t (** performs a call to a function with no summary by optimistically havoc'ing the by-ref actuals and the return value as appropriate *) + +val conservatively_initialize_args : AbstractValue.t list -> t -> t diff --git a/infer/tests/codetoanalyze/c/pulse/issues.exp b/infer/tests/codetoanalyze/c/pulse/issues.exp index 70131dbdc..27c6794bd 100644 --- a/infer/tests/codetoanalyze/c/pulse/issues.exp +++ b/infer/tests/codetoanalyze/c/pulse/issues.exp @@ -5,5 +5,8 @@ codetoanalyze/c/pulse/nullptr.c, malloc_no_check_bad, 2, NULLPTR_DEREFERENCE, no codetoanalyze/c/pulse/nullptr.c, nullptr_deref_young_bad, 5, NULLPTR_DEREFERENCE, no_bucket, ERROR, [invalidation part of the trace starts here,assigned,is the null pointer,use-after-lifetime part of the trace starts here,assigned,invalid access occurs here] codetoanalyze/c/pulse/uninit.c, call_to_use_and_mayinit_bad, 2, PULSE_UNINITIALIZED_VALUE, no_bucket, ERROR, [invalidation part of the trace starts here,variable `x` declared here,was uninitialized,use-after-lifetime part of the trace starts here,variable `x` declared here,invalid access occurs here] codetoanalyze/c/pulse/uninit.c, dereference_bad, 2, PULSE_UNINITIALIZED_VALUE, no_bucket, ERROR, [invalidation part of the trace starts here,variable `p` declared here,was uninitialized,use-after-lifetime part of the trace starts here,variable `p` declared here,invalid access occurs here] +codetoanalyze/c/pulse/uninit.c, interprocedural_nop_in_callee_bad, 3, PULSE_UNINITIALIZED_VALUE, no_bucket, ERROR, [invalidation part of the trace starts here,variable `x` declared here,was uninitialized,use-after-lifetime part of the trace starts here,variable `x` declared here,invalid access occurs here] +codetoanalyze/c/pulse/uninit.c, interprocedural_read_in_callee_bad, 2, PULSE_UNINITIALIZED_VALUE, no_bucket, ERROR, [invalidation part of the trace starts here,variable `x` declared here,was uninitialized,use-after-lifetime part of the trace starts here,variable `x` declared here,when calling `read_int_ref` here,parameter `p` of read_int_ref,invalid access occurs here] +codetoanalyze/c/pulse/uninit.c, interprocedural_uninit_in_callee_bad, 3, PULSE_UNINITIALIZED_VALUE, no_bucket, ERROR, [invalidation part of the trace starts here,when calling `uninit` here,allocated by call to `malloc` (modelled),was uninitialized,use-after-lifetime part of the trace starts here,passed as argument to `uninit`,return from call to `uninit`,assigned,invalid access occurs here] codetoanalyze/c/pulse/uninit.c, malloc_bad, 3, PULSE_UNINITIALIZED_VALUE, no_bucket, ERROR, [invalidation part of the trace starts here,allocated by call to `malloc` (modelled),was uninitialized,use-after-lifetime part of the trace starts here,allocated by call to `malloc` (modelled),assigned,invalid access occurs here] codetoanalyze/c/pulse/uninit.c, self_assign_bad, 2, PULSE_UNINITIALIZED_VALUE, no_bucket, ERROR, [invalidation part of the trace starts here,variable `x` declared here,was uninitialized,use-after-lifetime part of the trace starts here,variable `x` declared here,invalid access occurs here] diff --git a/infer/tests/codetoanalyze/c/pulse/uninit.c b/infer/tests/codetoanalyze/c/pulse/uninit.c index 6bd13eb34..83ea6e2b5 100644 --- a/infer/tests/codetoanalyze/c/pulse/uninit.c +++ b/infer/tests/codetoanalyze/c/pulse/uninit.c @@ -41,20 +41,36 @@ void malloc_bad() { void init_int_ref(int* p) { *p = 5; } -void nop(int* p) {} - -void interprocedural_good() { +void interprocedural_init_in_callee_good() { int x; init_int_ref(&x); int y = x; } -void interprocedural_bad_FN() { +void nop(int* p) {} + +void interprocedural_nop_in_callee_bad() { int x; nop(&x); int y = x; } +void read_int_ref(int* p) { int x = *p; } + +void interprocedural_read_in_callee_bad() { + int x; + read_int_ref(&x); +} + +int* uninit() { return (int*)malloc(sizeof(int)); } + +void interprocedural_uninit_in_callee_bad() { + int* p = uninit(); + if (p) { + int x = *p; + } +} + struct uninit_s { int f1; int f2; diff --git a/infer/tests/codetoanalyze/objc/pulse/issues.exp b/infer/tests/codetoanalyze/objc/pulse/issues.exp index b0ed542c1..10037a065 100644 --- a/infer/tests/codetoanalyze/objc/pulse/issues.exp +++ b/infer/tests/codetoanalyze/objc/pulse/issues.exp @@ -11,5 +11,6 @@ codetoanalyze/objc/pulse/NPEBlocks.m, Singleton.dispatch_once_no_npe_good_FP, 6, codetoanalyze/objc/pulse/NPEBlocks.m, captured_npe_bad, 5, NULLPTR_DEREFERENCE, no_bucket, ERROR, [invalidation part of the trace starts here,assigned,is the null pointer,use-after-lifetime part of the trace starts here,assigned,when calling `objc_blockcaptured_npe_bad_3` here,parameter `x` of objc_blockcaptured_npe_bad_3,invalid access occurs here] codetoanalyze/objc/pulse/NPEBlocks.m, captured_npe_ok_FP, 6, NULLPTR_DEREFERENCE, no_bucket, ERROR, [invalidation part of the trace starts here,assigned,is the null pointer,use-after-lifetime part of the trace starts here,assigned,invalid access occurs here] codetoanalyze/objc/pulse/uninit.m, Uninit.capture_in_closure_bad, 7, PULSE_UNINITIALIZED_VALUE, no_bucket, ERROR, [invalidation part of the trace starts here,variable `x` declared here,was uninitialized,use-after-lifetime part of the trace starts here,variable `x` declared here,invalid access occurs here] +codetoanalyze/objc/pulse/uninit.m, Uninit.not_set_in_closure_bad, 7, PULSE_UNINITIALIZED_VALUE, no_bucket, ERROR, [invalidation part of the trace starts here,variable `x` declared here,was uninitialized,use-after-lifetime part of the trace starts here,variable `x` declared here,invalid access occurs here] codetoanalyze/objc/pulse/use_after_free.m, PulseTest.use_after_free_simple_in_objc_method_bad:, 2, USE_AFTER_FREE, no_bucket, ERROR, [invalidation part of the trace starts here,parameter `x` of PulseTest.use_after_free_simple_in_objc_method_bad:,was invalidated by call to `free()`,use-after-lifetime part of the trace starts here,parameter `x` of PulseTest.use_after_free_simple_in_objc_method_bad:,invalid access occurs here] codetoanalyze/objc/pulse/use_after_free.m, use_after_free_simple_bad, 2, USE_AFTER_FREE, no_bucket, ERROR, [invalidation part of the trace starts here,parameter `x` of use_after_free_simple_bad,was invalidated by call to `free()`,use-after-lifetime part of the trace starts here,parameter `x` of use_after_free_simple_bad,invalid access occurs here] diff --git a/infer/tests/codetoanalyze/objc/pulse/uninit.m b/infer/tests/codetoanalyze/objc/pulse/uninit.m index 68ef90ae3..9b8c1520e 100644 --- a/infer/tests/codetoanalyze/objc/pulse/uninit.m +++ b/infer/tests/codetoanalyze/objc/pulse/uninit.m @@ -40,4 +40,24 @@ return x; } +- (BOOL)not_set_in_closure_bad { + __block BOOL x; + + void (^block)() = ^() { + }; + + block(); + return x; +} + +- (BOOL)use_in_closure_bad_FN { + __block BOOL x; + + void (^block)() = ^() { + BOOL y = x; + }; + + block(); +} + @end