Merge 221bf97 into 79c5282

Zilliqa · Oct 6, 2022 · 0161780 · 0161780
2 parents 79c5282 + 221bf97
commit 0161780
Show file tree

Hide file tree

Showing 8 changed files with 593 additions and 6 deletions.
diff --git a/src/base/DeadCodeDetector.ml b/src/base/DeadCodeDetector.ml
@@ -26,6 +26,7 @@
   * Let bindings
   * Library imports
   * User-defined ADTs and their constructors
+  * Fields in contract address types
 
   An ADT is considered as used if it one of:
   * Occurs in type annotations of the source code elements after typechecking
@@ -83,6 +84,7 @@ module DeadCodeDetector (SR : Rep) (ER : Rep) = struct
 
   let emp_idset = SCIdentifierSet.empty
   let emp_erset = ERSet.empty
+  let emp_idsmap = Map.empty (module SCIdentifierComp)
 
   (** Warning level for dead code detection *)
   let warning_level_dead_code = 3
@@ -194,9 +196,7 @@ module DeadCodeDetector (SR : Rep) (ER : Rep) = struct
 
   (** Collects a mapping from ADTs to their constructor definitions. *)
   let collect_adts_to_ctrs lentries =
-    List.fold_left lentries
-      ~init:(Map.empty (module SCIdentifierComp))
-      ~f:(fun m lentry ->
+    List.fold_left lentries ~init:emp_idsmap ~f:(fun m lentry ->
         match lentry with
         | LibTyp (id, ctr_defs) ->
             Map.set m ~key:(SCIdentifier.get_id id) ~data:ctr_defs
@@ -531,6 +531,120 @@ module DeadCodeDetector (SR : Rep) (ER : Rep) = struct
           | CompTrans -> SCIdentifierSet.union param_adts res_param_adts
           | CompProc -> res_param_adts ))
 
+  let merge_id_maps m1 m2 =
+    Map.merge m1 m2 ~f:(fun ~key:_ -> function
+      | `Both (s1, s2) -> Some (Set.union s1 s2) | `Left s | `Right s -> Some s)
+
+  (** Returns a list of fields with the contract address type from
+      [address_params] that are used in [s]. *)
+  let rec get_used_address_fields address_params (s, _annot) =
+    match s with
+    | RemoteLoad (_, addr, field) | RemoteMapGet (_, addr, field, _, _) ->
+        Map.set emp_idsmap ~key:(get_id addr)
+          ~data:(SCIdentifierSet.singleton (get_id field))
+    | MatchStmt (_id, arms) ->
+        List.fold_left arms ~init:emp_idsmap ~f:(fun m (_pattern, stmts) ->
+            List.fold_left stmts ~init:emp_idsmap ~f:(fun m sa ->
+                get_used_address_fields address_params sa |> merge_id_maps m)
+            |> merge_id_maps m)
+    | Bind _ | Load _ | Store _ | MapUpdate _ | MapGet _ | ReadFromBC _
+    | TypeCast _ | AcceptPayment | Iterate _ | SendMsgs _ | CreateEvnt _
+    | CallProc _ | Throw _ | GasStmt _ ->
+        emp_idsmap
+
+  (** Updates a map of identifiers [m] iff [ty] has contract address type.
+      [m] has the following structure: [id |-> F] where [F] is a set of field
+      names used in the contract address type. *)
+  let update_contract_params_map m id ty =
+    match ty with
+    | SType.Address (ContrAddr addr) ->
+        let data =
+          List.fold_left (SType.IdLoc_Comp.Map.keys addr) ~init:emp_idset
+            ~f:(fun s id -> SCIdentifierSet.add s (get_id id))
+        in
+        Map.set m ~key:(SCIdentifier.get_id id) ~data
+    | _ -> m
+
+  (** Checks for unused fields in contract address types of [comp]'s
+      parameters.
+      Returns a set of ids of contract parameters used in this [comp].  *)
+  let check_contract_address_types_in_params contract_params
+      used_contract_params comp =
+    let address_params =
+      List.fold_left comp.comp_params ~init:contract_params
+        ~f:(fun m (id, ty) -> update_contract_params_map m id ty)
+    in
+    let used_addresses =
+      (* addr |-> set of used fields *)
+      List.fold_left comp.comp_body ~init:emp_idsmap ~f:(fun m s ->
+          get_used_address_fields address_params s |> merge_id_maps m)
+    in
+    (* Generate warnings for unused fields in component arguments. *)
+    List.iter comp.comp_params ~f:(fun (id, ty) ->
+        let name = get_id id in
+        match (Map.find address_params name, Map.find used_addresses name) with
+        | Some fields, Some used_fields
+          when not @@ phys_equal (Set.length fields) (Set.length used_fields)
+          -> (
+            match ty with
+            | SType.Address (ContrAddr m) ->
+                List.iter (SType.IdLoc_Comp.Map.keys m) ~f:(fun id ->
+                    let name = get_id id in
+                    if Set.mem fields name && (not @@ Set.mem used_fields name)
+                    then
+                      warn1
+                        ("Unused field in the contract address type: "
+                       ^ as_error_string id)
+                        warning_level_dead_code
+                        (SType.TIdentifier.get_rep id))
+            | _ -> ())
+        | _ -> ());
+    (* Collect fields of the [cmod] contract with contract address types used
+       in this component. *)
+    Map.keys used_addresses
+    |> List.fold_left ~init:used_contract_params ~f:(fun used_ids_map used_id ->
+           match Map.find used_ids_map used_id with
+           | Some used_fields ->
+               let data =
+                 Map.find_exn used_addresses used_id |> Set.union used_fields
+               in
+               Map.set used_ids_map ~key:used_id ~data
+           | None ->
+               let data = Map.find_exn used_addresses used_id in
+               Map.set used_ids_map ~key:used_id ~data)
+
+  (** Checks for unused fields in contract address types. *)
+  let check_contract_address_types cmod =
+    let contract_params =
+      List.fold_left cmod.contr.cparams ~init:emp_idsmap ~f:(fun m (id, ty) ->
+          update_contract_params_map m id ty)
+    in
+    let used_contract_params =
+      List.fold_left cmod.contr.ccomps ~init:emp_idsmap ~f:(fun used c ->
+          check_contract_address_types_in_params contract_params used c)
+    in
+    (* Report unused contract parameters with contract address types. *)
+    Map.keys contract_params
+    |> List.iter ~f:(fun param ->
+           match Map.find used_contract_params param with
+           | Some used_fields ->
+               let all_fields = Map.find_exn contract_params param in
+               let unused_fields = Set.diff all_fields used_fields in
+               if not @@ Set.is_empty unused_fields then
+                 List.iter cmod.contr.cparams ~f:(fun (id, _ty) ->
+                     if SCIdentifier.Name.equal param (SCIdentifier.get_id id)
+                     then
+                       Set.iter unused_fields ~f:(fun f ->
+                           warn1
+                             ("Unused field in the contract address type: "
+                             ^ SCIdentifier.Name.as_string f)
+                             warning_level_dead_code
+                             (ER.get_loc (SCIdentifier.get_rep id))))
+           | None ->
+               (* All the fields of [param] are unused, so we don't report it.
+                  It is an unused contract parameter.*)
+               ())
+
   (** Checks for dead code in the fields and constraints of a contract
       @return Live variables, ADTs and constructors, including ones found in
               fields and constraints. *)
@@ -676,16 +790,18 @@ module DeadCodeDetector (SR : Rep) (ER : Rep) = struct
   let dc_cmod (cmod : cmodule) (elibs : libtree list) =
     let fields_state = FieldsState.mk cmod in
     let adts_to_ctrs =
-      Option.value_map cmod.libs
-        ~default:(Map.empty (module SCIdentifierComp))
-        ~f:(fun l -> collect_adts_to_ctrs l.lentries)
+      Option.value_map cmod.libs ~default:emp_idsmap ~f:(fun l ->
+          collect_adts_to_ctrs l.lentries)
     in
 
     (* Check components *)
     let lv_comps, comps_adts, comps_ctrs, comp_param_adts =
       check_comps cmod fields_state
     in
 
+    (* Check for unused fields in contract address types *)
+    check_contract_address_types cmod;
+
     (* Check fields and constraints *)
     let lv_fields, adts_fields, ctrs_fields =
       check_fields_and_constraints cmod lv_comps comps_adts comps_ctrs

diff --git a/tests/checker/good/Good.ml b/tests/checker/good/Good.ml
@@ -92,6 +92,9 @@ module Tests = Scilla_test.Util.DiffBasedTests (struct
       "unbox_result3.scilla";
       "unbox_result4.scilla";
       "unbox_result5.scilla";
+      "dead_code_test15.scilla";
+      "dead_code_test16.scilla";
+      "dead_code_test17.scilla";
       "simple-dex-remote-reads.scilla";
       "type_casts.scilla";
       "timestamp.scilla";

diff --git a/tests/checker/good/gold/dead_code_test15.scilla.gold b/tests/checker/good/gold/dead_code_test15.scilla.gold
@@ -0,0 +1,192 @@
+{
+  "cashflow_tags": {
+    "State variables": [ { "field": "param_contr", "tag": "NoInfo" } ],
+    "ADT constructors": []
+  },
+  "contract_info": {
+    "scilla_major_version": "0",
+    "vname": "Dead15",
+    "params": [
+      {
+        "vname": "param_contr",
+        "type": "ByStr20 with contract field param_unused : ByStr20, field param_used : ByStr20 end"
+      }
+    ],
+    "fields": [],
+    "transitions": [
+      {
+        "vname": "tr1",
+        "params": [
+          {
+            "vname": "contract_address",
+            "type": "ByStr20 with contract field field1 : ByStr20, field field2 : Uint128, field unused_field : Map (Uint256) (ByStr20) end"
+          }
+        ]
+      },
+      {
+        "vname": "tr2",
+        "params": [
+          {
+            "vname": "contract_address_unused",
+            "type": "ByStr20 with contract field field1 : ByStr20, field field2 : Uint128 end"
+          },
+          {
+            "vname": "contract_address",
+            "type": "ByStr20 with contract field field3 : Uint64, field field4 : Uint64 end"
+          }
+        ]
+      },
+      { "vname": "tr3", "params": [] }
+    ],
+    "procedures": [
+      {
+        "vname": "do_accept",
+        "params": [ { "vname": "a", "type": "ByStr20" } ]
+      }
+    ],
+    "events": [],
+    "ADTs": [
+      {
+        "tname": "Option",
+        "tparams": [ "'A" ],
+        "tmap": [
+          { "cname": "Some", "argtypes": [ "'A" ] },
+          { "cname": "None", "argtypes": [] }
+        ]
+      },
+      {
+        "tname": "Bool",
+        "tparams": [],
+        "tmap": [
+          { "cname": "True", "argtypes": [] },
+          { "cname": "False", "argtypes": [] }
+        ]
+      },
+      {
+        "tname": "Nat",
+        "tparams": [],
+        "tmap": [
+          { "cname": "Zero", "argtypes": [] },
+          { "cname": "Succ", "argtypes": [ "Nat" ] }
+        ]
+      },
+      {
+        "tname": "List",
+        "tparams": [ "'A" ],
+        "tmap": [
+          { "cname": "Cons", "argtypes": [ "'A", "List ('A)" ] },
+          { "cname": "Nil", "argtypes": [] }
+        ]
+      },
+      {
+        "tname": "Pair",
+        "tparams": [ "'A", "'B" ],
+        "tmap": [ { "cname": "Pair", "argtypes": [ "'A", "'B" ] } ]
+      }
+    ]
+  },
+  "warnings": [
+    {
+      "warning_message": "Unused field in the contract address type: param_unused",
+      "start_location": {
+        "file": "contracts/dead_code_test15.scilla",
+        "line": 10,
+        "column": 5
+      },
+      "end_location": { "file": "", "line": 0, "column": 0 },
+      "warning_id": 3
+    },
+    {
+      "warning_message": "Unused field in the contract address type: field4",
+      "start_location": {
+        "file": "contracts/dead_code_test15.scilla",
+        "line": 35,
+        "column": 13
+      },
+      "end_location": { "file": "", "line": 0, "column": 0 },
+      "warning_id": 3
+    },
+    {
+      "warning_message": "Unused field in the contract address type: unused_field",
+      "start_location": {
+        "file": "contracts/dead_code_test15.scilla",
+        "line": 22,
+        "column": 11
+      },
+      "end_location": { "file": "", "line": 0, "column": 0 },
+      "warning_id": 3
+    },
+    {
+      "warning_message": "Unused transition parameter: contract_address",
+      "start_location": {
+        "file": "contracts/dead_code_test15.scilla",
+        "line": 33,
+        "column": 5
+      },
+      "end_location": { "file": "", "line": 0, "column": 0 },
+      "warning_id": 3
+    },
+    {
+      "warning_message": "Unused transition parameter: contract_address_unused",
+      "start_location": {
+        "file": "contracts/dead_code_test15.scilla",
+        "line": 29,
+        "column": 5
+      },
+      "end_location": { "file": "", "line": 0, "column": 0 },
+      "warning_id": 3
+    },
+    {
+      "warning_message": "Unused remote load statement to: a",
+      "start_location": {
+        "file": "contracts/dead_code_test15.scilla",
+        "line": 37,
+        "column": 3
+      },
+      "end_location": { "file": "", "line": 0, "column": 0 },
+      "warning_id": 3
+    },
+    {
+      "warning_message": "Unused transition parameter: contract_address",
+      "start_location": {
+        "file": "contracts/dead_code_test15.scilla",
+        "line": 19,
+        "column": 16
+      },
+      "end_location": { "file": "", "line": 0, "column": 0 },
+      "warning_id": 3
+    },
+    {
+      "warning_message": "Unused remote load statement to: a",
+      "start_location": {
+        "file": "contracts/dead_code_test15.scilla",
+        "line": 23,
+        "column": 3
+      },
+      "end_location": { "file": "", "line": 0, "column": 0 },
+      "warning_id": 3
+    },
+    {
+      "warning_message": "Unused remote load statement to: b",
+      "start_location": {
+        "file": "contracts/dead_code_test15.scilla",
+        "line": 24,
+        "column": 3
+      },
+      "end_location": { "file": "", "line": 0, "column": 0 },
+      "warning_id": 3
+    },
+    {
+      "warning_message": "Unused procedure parameter: a",
+      "start_location": {
+        "file": "contracts/dead_code_test15.scilla",
+        "line": 15,
+        "column": 21
+      },
+      "end_location": { "file": "", "line": 0, "column": 0 },
+      "warning_id": 3
+    }
+  ],
+  "gas_remaining": "7999"
+}
+