diff --git a/infer/documentation/issues/PULSE_UNNECESSARY_COPY_THRIFT_ASSIGNMENT.md b/infer/documentation/issues/PULSE_UNNECESSARY_COPY_THRIFT_ASSIGNMENT.md new file mode 100644 index 00000000000..6aad76c7874 --- /dev/null +++ b/infer/documentation/issues/PULSE_UNNECESSARY_COPY_THRIFT_ASSIGNMENT.md @@ -0,0 +1,2 @@ +This is similar to [PULSE_UNNECESSARY_COPY_ASSIGNMENT](#pulse_unnecessary_copy_assignment), but is +reported when copied into thrift fields. diff --git a/infer/man/man1/infer-full.txt b/infer/man/man1/infer-full.txt index 1f9b96c09b8..c1cd30714da 100644 --- a/infer/man/man1/infer-full.txt +++ b/infer/man/man1/infer-full.txt @@ -716,6 +716,7 @@ OPTIONS PULSE_UNNECESSARY_COPY_OPTIONAL (enabled by default), PULSE_UNNECESSARY_COPY_OPTIONAL_CONST (disabled by default), PULSE_UNNECESSARY_COPY_RETURN (disabled by default), + PULSE_UNNECESSARY_COPY_THRIFT_ASSIGNMENT (enabled by default), PURE_FUNCTION (enabled by default), REGEX_OP_ON_UI_THREAD (enabled by default), RESOURCE_LEAK (enabled by default), diff --git a/infer/man/man1/infer-report.txt b/infer/man/man1/infer-report.txt index 746f50e7749..779e0f33558 100644 --- a/infer/man/man1/infer-report.txt +++ b/infer/man/man1/infer-report.txt @@ -246,6 +246,7 @@ OPTIONS PULSE_UNNECESSARY_COPY_OPTIONAL (enabled by default), PULSE_UNNECESSARY_COPY_OPTIONAL_CONST (disabled by default), PULSE_UNNECESSARY_COPY_RETURN (disabled by default), + PULSE_UNNECESSARY_COPY_THRIFT_ASSIGNMENT (enabled by default), PURE_FUNCTION (enabled by default), REGEX_OP_ON_UI_THREAD (enabled by default), RESOURCE_LEAK (enabled by default), diff --git a/infer/man/man1/infer.txt b/infer/man/man1/infer.txt index ada11eb30f5..130605681b2 100644 --- a/infer/man/man1/infer.txt +++ b/infer/man/man1/infer.txt @@ -716,6 +716,7 @@ OPTIONS PULSE_UNNECESSARY_COPY_OPTIONAL (enabled by default), PULSE_UNNECESSARY_COPY_OPTIONAL_CONST (disabled by default), PULSE_UNNECESSARY_COPY_RETURN (disabled by default), + PULSE_UNNECESSARY_COPY_THRIFT_ASSIGNMENT (enabled by default), PURE_FUNCTION (enabled by default), REGEX_OP_ON_UI_THREAD (enabled by default), RESOURCE_LEAK (enabled by default), diff --git a/infer/src/base/IssueType.ml b/infer/src/base/IssueType.ml index 78ac6ec63ff..cc269a0c12f 100644 --- a/infer/src/base/IssueType.ml +++ b/infer/src/base/IssueType.ml @@ -1042,6 +1042,12 @@ let unnecessary_copy_return_pulse = ~user_documentation:[%blob "./documentation/issues/PULSE_UNNECESSARY_COPY_RETURN.md"] +let unnecessary_copy_thrift_assignment_pulse = + register ~category:PerfRegression ~id:"PULSE_UNNECESSARY_COPY_THRIFT_ASSIGNMENT" Error Pulse + ~hum:"Unnecessary Copy Assignment into Thrift" + ~user_documentation:[%blob "./documentation/issues/PULSE_UNNECESSARY_COPY_THRIFT_ASSIGNMENT.md"] + + let unreachable_code_after = register_hidden ~id:"UNREACHABLE_CODE" Error BufferOverrunChecker let use_after_delete = diff --git a/infer/src/base/IssueType.mli b/infer/src/base/IssueType.mli index 8f9f17a7506..e56287539c7 100644 --- a/infer/src/base/IssueType.mli +++ b/infer/src/base/IssueType.mli @@ -372,6 +372,8 @@ val unnecessary_copy_optional_const_pulse : t val unnecessary_copy_return_pulse : t +val unnecessary_copy_thrift_assignment_pulse : t + val unreachable_code_after : t val use_after_delete : latent:bool -> t diff --git a/infer/src/pulse/Pulse.ml b/infer/src/pulse/Pulse.ml index 4aa29562c12..dd17f393a27 100644 --- a/infer/src/pulse/Pulse.ml +++ b/infer/src/pulse/Pulse.ml @@ -101,7 +101,7 @@ let is_copy_cted_into_var from copied_into = match (from : Attribute.CopyOrigin.t) with | CopyInGetDefault | CopyCtor -> true - | CopyAssignment | CopyToOptional -> + | CopyAssignment _ | CopyToOptional -> false diff --git a/infer/src/pulse/PulseAttribute.ml b/infer/src/pulse/PulseAttribute.ml index 59c09c755e0..778d7dc6620 100644 --- a/infer/src/pulse/PulseAttribute.ml +++ b/infer/src/pulse/PulseAttribute.ml @@ -112,13 +112,15 @@ module Attribute = struct [@@deriving compare, equal, show {with_path= false}] module CopyOrigin = struct - type t = CopyCtor | CopyAssignment | CopyToOptional | CopyInGetDefault + type assign_t = Normal | Thrift [@@deriving compare, equal] + + type t = CopyCtor | CopyAssignment of assign_t | CopyToOptional | CopyInGetDefault [@@deriving compare, equal] let pp fmt = function | CopyCtor -> F.fprintf fmt "copied" - | CopyAssignment -> + | CopyAssignment (Normal | Thrift) -> F.fprintf fmt "copy assigned" | CopyToOptional -> F.fprintf fmt "copied by Optional value construction" diff --git a/infer/src/pulse/PulseAttribute.mli b/infer/src/pulse/PulseAttribute.mli index 61489503b97..065952eb691 100644 --- a/infer/src/pulse/PulseAttribute.mli +++ b/infer/src/pulse/PulseAttribute.mli @@ -70,7 +70,9 @@ type taint_propagation_reason = InternalModel | UnknownCall | UserConfig val pp_taint_propagation_reason : F.formatter -> taint_propagation_reason -> unit module CopyOrigin : sig - type t = CopyCtor | CopyAssignment | CopyToOptional | CopyInGetDefault + type assign_t = Normal | Thrift [@@deriving compare, equal] + + type t = CopyCtor | CopyAssignment of assign_t | CopyToOptional | CopyInGetDefault [@@deriving compare, equal] val pp : Formatter.t -> t -> unit diff --git a/infer/src/pulse/PulseDiagnostic.ml b/infer/src/pulse/PulseDiagnostic.ml index 6a04b2c1241..c6d8dfb9500 100644 --- a/infer/src/pulse/PulseDiagnostic.ml +++ b/infer/src/pulse/PulseDiagnostic.ml @@ -1219,7 +1219,7 @@ let get_issue_type ~latent issue_type = IssueType.pulse_transitive_access | UnnecessaryCopy {copied_location= Some _}, false -> IssueType.unnecessary_copy_return_pulse - | UnnecessaryCopy {copied_into= IntoField _; source_typ; from= CopyAssignment}, false + | UnnecessaryCopy {copied_into= IntoField _; source_typ; from= CopyAssignment Normal}, false when Option.exists ~f:Typ.is_rvalue_reference source_typ -> IssueType.unnecessary_copy_assignment_movable_pulse | UnnecessaryCopy {copied_into= IntoField _; source_typ; from= CopyCtor}, false @@ -1235,9 +1235,11 @@ let get_issue_type ~latent issue_type = IssueType.unnecessary_copy_intermediate_pulse | UnnecessaryCopy {copied_into= IntoVar _; from= CopyCtor | CopyInGetDefault}, false -> IssueType.unnecessary_copy_pulse - | UnnecessaryCopy {from= CopyAssignment; source_typ}, false -> + | UnnecessaryCopy {from= CopyAssignment Normal; source_typ}, false -> if is_from_const source_typ then IssueType.unnecessary_copy_assignment_const_pulse else IssueType.unnecessary_copy_assignment_pulse + | UnnecessaryCopy {from= CopyAssignment Thrift}, false -> + IssueType.unnecessary_copy_thrift_assignment_pulse | UnnecessaryCopy {source_typ; from= CopyToOptional}, false -> if is_from_const source_typ then IssueType.unnecessary_copy_optional_const_pulse else IssueType.unnecessary_copy_optional_pulse diff --git a/infer/src/pulse/PulseNonDisjunctiveOperations.ml b/infer/src/pulse/PulseNonDisjunctiveOperations.ml index c1c172d35b1..cb0a59d3eb8 100644 --- a/infer/src/pulse/PulseNonDisjunctiveOperations.ml +++ b/infer/src/pulse/PulseNonDisjunctiveOperations.ml @@ -52,9 +52,10 @@ let get_copy_origin pname actuals = let open IOption.Let_syntax in let* attrs = IRAttributes.load pname in if attrs.ProcAttributes.is_cpp_copy_ctor then Some Attribute.CopyOrigin.CopyCtor - else if attrs.ProcAttributes.is_cpp_copy_assignment then Some Attribute.CopyOrigin.CopyAssignment + else if attrs.ProcAttributes.is_cpp_copy_assignment then + Some (Attribute.CopyOrigin.CopyAssignment Normal) else if is_thrift_field_copy_assignment pname actuals then - Some Attribute.CopyOrigin.CopyAssignment + Some (Attribute.CopyOrigin.CopyAssignment Thrift) else None @@ -286,8 +287,12 @@ let is_address_reachable_from_unowned source_addr ~astates_before proc_lvalue_re let is_copied_from_address_reachable_from_unowned ~is_captured_by_ref ~is_intermediate ~from source_addr_typ_opt proc_lvalue_ref_parameters ~astates_before = ( is_intermediate - || Attribute.CopyOrigin.equal from CopyAssignment - || Attribute.CopyOrigin.equal from CopyToOptional ) + || + match (from : Attribute.CopyOrigin.t) with + | CopyAssignment _ | CopyToOptional -> + true + | CopyCtor | CopyInGetDefault -> + false ) && Option.exists source_addr_typ_opt ~f:(fun (source_addr, source_expr, _) -> ( match source_expr with | DecompilerExpr.SourceExpr ((PVar pvar, _), _) ->