Add Clippy and Rust fmt to CI-pipeline (#75)

.travis_run_tests.sh

 #!/bin/bash
-docker run --rm -t cwe-checker cargo test && docker run --rm -t cwe-checker dune runtest && pytest
+docker run --rm -t cwe-checker make codestyle-check \
+&& docker run --rm -t cwe-checker cargo test \
+&& docker run --rm -t cwe-checker dune runtest \
+&& pytest

Makefile

@@ -18,6 +18,10 @@ test:
 	cd test/artificial_samples; scons; cd ../..
 	pytest -v --ignore=_build
 
+codestyle-check:
+	cargo fmt -- --check
+	cargo clippy -- -D clippy::all
+
 clean:
 	cargo clean
 	rm -f src/libcwe_checker_rs.a

cwe_checker_rs/Cargo.toml

@@ -14,6 +14,7 @@ petgraph = { version = "0.5", features = ["default", "serde-1"] }
 fnv = "1.0" # a faster hash function for small keys like integers
 anyhow = "1.0" # for easy error types
 crossbeam-channel = "0.4"
+derive_more = "0.99"
 
 [lib]
 name = "cwe_checker_rs"

cwe_checker_rs/src/analysis/graph.rs

@@ -95,7 +95,7 @@ impl<'a> GraphBuilder<'a> {
     /// add all subs to the jump targets so that call instructions can be linked to the starting block of the corresponding sub.
     fn add_subs_to_jump_targets(&mut self) {
         for sub in self.program.term.subs.iter() {
-            if sub.term.blocks.len() > 0 {
+            if !sub.term.blocks.is_empty() {
                 let start_block = &sub.term.blocks[0];
                 let target_index = self.jump_targets[&start_block.tid];
                 self.jump_targets.insert(sub.tid.clone(), target_index);
@@ -140,7 +140,7 @@ impl<'a> GraphBuilder<'a> {
                             self.return_addresses
                                 .entry(target_tid.clone())
                                 .and_modify(|vec| vec.push((source, return_index)))
-                                .or_insert(vec![(source, return_index)]);
+                                .or_insert_with(|| vec![(source, return_index)]);
                         }
                         // TODO: Non-returning calls and tail calls both have no return target in BAP.
                         // Thus we need to distinguish them somehow to correctly handle tail calls.
@@ -189,8 +189,7 @@ impl<'a> GraphBuilder<'a> {
                 .term
                 .jmps
                 .iter()
-                .filter(|jump| matches!(jump.term.kind, JmpKind::Call(_)))
-                .next()
+                .find(|jump| matches!(jump.term.kind, JmpKind::Call(_)))
                 .unwrap();
             let cr_combine_node = self.graph.add_node(Node::CallReturn(call_block));
             self.graph
@@ -210,8 +209,7 @@ impl<'a> GraphBuilder<'a> {
                     .term
                     .jmps
                     .iter()
-                    .find(|jmp| matches!(jmp.term.kind, JmpKind::Return(_)))
-                    .is_some()
+                    .any(|jmp| matches!(jmp.term.kind, JmpKind::Return(_)))
                 {
                     let return_from_node = self.jump_targets[&block.tid].1;
                     self.add_call_return_node_and_edges(sub, return_from_node);
@@ -242,7 +240,7 @@ impl<'a> GraphBuilder<'a> {
 /// This function builds the interprocedural control flow graph for a program term.
 pub fn get_program_cfg(program: &Term<Program>, extern_subs: HashSet<Tid>) -> Graph {
     let builder = GraphBuilder::new(program, extern_subs);
-    return builder.build();
+    builder.build()
 }
 
 /// For a given set of block TIDs generate a map from the TIDs to the indices of the BlkStart and BlkEnd nodes
@@ -255,17 +253,14 @@ pub fn get_indices_of_block_nodes<'a, I: Iterator<Item = &'a Tid>>(
     let mut tid_to_indices_map = HashMap::new();
     for node_index in graph.node_indices() {
         if let Some(tid) = tids.get(&graph[node_index].get_block().tid) {
-            match graph[node_index] {
-                Node::BlkStart(_block_term) => {
-                    let start_index = node_index;
-                    let end_index = graph.neighbors(start_index).next().unwrap();
-                    tid_to_indices_map.insert(tid.clone(), (start_index, end_index));
-                }
-                _ => (),
+            if let Node::BlkStart(_block_term) = graph[node_index] {
+                let start_index = node_index;
+                let end_index = graph.neighbors(start_index).next().unwrap();
+                tid_to_indices_map.insert(tid.clone(), (start_index, end_index));
             }
         }
     }
-    return tid_to_indices_map;
+    tid_to_indices_map
 }
 
 #[cfg(test)]

cwe_checker_rs/src/analysis/interprocedural_fixpoint.rs

@@ -172,11 +172,11 @@ impl<'a, T: Problem<'a>> GeneralFPProblem for GeneralizedProblem<'a, T> {
             Edge::ExternCallStub(call) => self
                 .problem
                 .update_call_stub(node_value.unwrap_value(), call)
-                .map(|val| NodeValue::Value(val)),
+                .map(NodeValue::Value),
             Edge::Jump(jump, untaken_conditional) => self
                 .problem
                 .update_jump(node_value.unwrap_value(), jump, *untaken_conditional)
-                .map(|val| NodeValue::Value(val)),
+                .map(NodeValue::Value),
         }
     }
 }
@@ -192,7 +192,7 @@ impl<'a, T: Problem<'a>> Computation<'a, T> {
         let generalized_problem = GeneralizedProblem::new(problem);
         let computation = super::fixpoint::Computation::new(
             generalized_problem,
-            default_value.map(|val| NodeValue::Value(val)),
+            default_value.map(NodeValue::Value),
         );
         Computation {
             generalized_computation: computation,

cwe_checker_rs/src/analysis/mem_region.rs

@@ -21,8 +21,10 @@ Implementation needs is_top() to be a member function of the ValueDomain trait.
 use super::abstract_domain::*;
 use crate::bil::{BitSize, Bitvector};
 use apint::{Int, Width};
+use derive_more::Deref;
 use serde::{Deserialize, Serialize};
 use std::collections::BTreeMap;
+use std::ops::DerefMut;
 use std::sync::Arc;
 
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone)]
@@ -31,21 +33,17 @@ struct Element<T> {
     value: T,
 }
 
-#[derive(Serialize, Deserialize, Debug, Hash, Clone)]
+#[derive(Serialize, Deserialize, Debug, Hash, Clone, PartialEq, Eq, Deref)]
+#[deref(forward)]
 pub struct MemRegion<T: AbstractDomain + ValueDomain + std::fmt::Debug>(Arc<MemRegionData<T>>);
 
-impl<T: AbstractDomain + ValueDomain + std::fmt::Debug> PartialEq for MemRegion<T> {
-    fn eq(&self, other: &Self) -> bool {
-        if Arc::ptr_eq(&self.0, &other.0) {
-            true
-        } else {
-            self.0 == other.0
-        }
+impl<T: AbstractDomain + ValueDomain + std::fmt::Debug> DerefMut for MemRegion<T> {
+    fn deref_mut(&mut self) -> &mut MemRegionData<T> {
+        Arc::make_mut(&mut self.0)
     }
 }
 
-impl<T: AbstractDomain + ValueDomain + std::fmt::Debug> Eq for MemRegion<T> {}
-
+// TODO: most of the functions in this impl block should be moved to MemRegionData (or removed, if they are only thin wrappers).
 impl<T: AbstractDomain + ValueDomain + std::fmt::Debug> MemRegion<T> {
     pub fn new(address_bitsize: BitSize) -> Self {
         MemRegion(Arc::new(MemRegionData::new(address_bitsize)))
@@ -90,7 +88,7 @@ impl<T: AbstractDomain + ValueDomain + std::fmt::Debug> MemRegion<T> {
 
 /// An abstract domain representing a continuous region of memory. See the module level description for more.
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone)]
-struct MemRegionData<T: AbstractDomain + ValueDomain + std::fmt::Debug> {
+pub struct MemRegionData<T: AbstractDomain + ValueDomain + std::fmt::Debug> {
     address_bitsize: BitSize,
     values: BTreeMap<i64, T>,
 }
@@ -162,7 +160,7 @@ impl<T: AbstractDomain + ValueDomain + std::fmt::Debug> MemRegionData<T> {
             }
         }
         let bitsize = 8 * size as u16;
-        return T::new_top(bitsize);
+        T::new_top(bitsize)
     }
 
     /// Remove all elements intersecting the provided interval.

cwe_checker_rs/src/analysis/pointer_inference/context.rs

@@ -166,7 +166,6 @@ impl<'a> crate::analysis::interprocedural_fixpoint::Problem<'a> for Context<'a>
         } else {
             panic!("Malformed control flow graph: Encountered call edge with a non-call jump term.")
         };
-        let stack_offset_domain = self.get_current_stack_offset(state);
 
         if let Label::Direct(ref callee_tid) = call.target {
             let callee_stack_id = AbstractIdentifier::new(
@@ -177,7 +176,7 @@ impl<'a> crate::analysis::interprocedural_fixpoint::Problem<'a> for Context<'a>
                 call_term.tid.clone(),
                 AbstractLocation::from_var(&self.project.stack_pointer_register).unwrap(),
             );
-            let stack_offset_adjustment = stack_offset_domain.clone();
+            let stack_offset_adjustment = self.get_current_stack_offset(state);
             let address_bitsize = self.project.stack_pointer_register.bitsize().unwrap();
 
             let mut callee_state = state.clone();
@@ -214,9 +213,7 @@ impl<'a> crate::analysis::interprocedural_fixpoint::Problem<'a> for Context<'a>
             );
             // set the list of caller stack ids to only this caller id
             callee_state.caller_stack_ids = BTreeSet::new();
-            callee_state
-                .caller_stack_ids
-                .insert(new_caller_stack_id.clone());
+            callee_state.caller_stack_ids.insert(new_caller_stack_id);
             // Remove non-referenced objects and objects, only the caller knows about, from the state.
             callee_state.ids_known_to_caller = BTreeSet::new();
             callee_state.remove_unreferenced_objects();
@@ -224,7 +221,7 @@ impl<'a> crate::analysis::interprocedural_fixpoint::Problem<'a> for Context<'a>
             callee_state.ids_known_to_caller = callee_state.memory.get_all_object_ids();
             callee_state.ids_known_to_caller.remove(&callee_stack_id);
 
-            return callee_state;
+            callee_state
         } else {
             panic!("Indirect call edges not yet supported.")
             // TODO: Support indirect call edges!
@@ -277,7 +274,7 @@ impl<'a> crate::analysis::interprocedural_fixpoint::Problem<'a> for Context<'a>
         state_after_return.merge_callee_stack_to_caller_stack(
             callee_stack_id,
             original_caller_stack_id,
-            &(-stack_offset_on_call.clone()),
+            &(-stack_offset_on_call),
         );
         state_after_return.stack_id = original_caller_stack_id.clone();
         state_after_return.caller_stack_ids = state_before_call.caller_stack_ids.clone();
@@ -382,11 +379,11 @@ impl<'a> crate::analysis::interprocedural_fixpoint::Problem<'a> for Context<'a>
                                     new_state.set_register(return_register, pointer.into()),
                                     Some(&call.tid),
                                 );
-                                return Some(new_state);
+                                Some(new_state)
                             } else {
                                 // We cannot track the new object, since we do not know where to store the pointer to it.
                                 // TODO: Return a diagnostics message to the user here.
-                                return Some(new_state);
+                                Some(new_state)
                             }
                         }
                         "free" => {
@@ -412,16 +409,16 @@ impl<'a> crate::analysis::interprocedural_fixpoint::Problem<'a> for Context<'a>
                                             }
                                         } // TODO: add diagnostics for else case
                                         new_state.remove_unreferenced_objects();
-                                        return Some(new_state);
+                                        Some(new_state)
                                     } else {
                                         // TODO: add diagnostics message for the user here
-                                        return Some(new_state);
+                                        Some(new_state)
                                     }
                                 }
                                 Err(err) => {
                                     // We do not know which memory object to free
                                     self.log_debug(Err(err), Some(&call.tid));
-                                    return Some(new_state);
+                                    Some(new_state)
                                 }
                             }
                         }
@@ -431,7 +428,7 @@ impl<'a> crate::analysis::interprocedural_fixpoint::Problem<'a> for Context<'a>
                                 Some(&call.tid),
                             );
                             let mut possible_referenced_ids = BTreeSet::new();
-                            if extern_symbol.arguments.len() == 0 {
+                            if extern_symbol.arguments.is_empty() {
                                 // TODO: We assume here that we do not know the parameters and approximate them by all parameter registers.
                                 // This approximation is wrong if the function is known but has neither parameters nor return values.
                                 // We need to somehow distinguish these two cases.
@@ -465,7 +462,7 @@ impl<'a> crate::analysis::interprocedural_fixpoint::Problem<'a> for Context<'a>
                                     .memory
                                     .mark_mem_object_as_untracked(id, &possible_referenced_ids);
                             }
-                            return Some(new_state);
+                            Some(new_state)
                         }
                     }
                 } else {

cwe_checker_rs/src/analysis/pointer_inference/data.rs

@@ -54,7 +54,7 @@ impl Data {
                     }
                 })
                 .collect();
-            if remaining_targets.len() == 0 {
+            if remaining_targets.is_empty() {
                 *self = Data::new_top(self.bitsize());
             } else {
                 *self = Data::Pointer(PointerDomain::with_targets(remaining_targets));

cwe_checker_rs/src/analysis/pointer_inference/identifier.rs

 use crate::bil::variable::*;
 use crate::prelude::*;
-use crate::utils::fast_cmp_arc::FastCmpArc;
+use derive_more::Deref;
 use std::sync::Arc;
 
 // TODO: Right now abstract locations are used as giving the location where a pointer to an object is located.
@@ -14,8 +14,10 @@ use std::sync::Arc;
 /// The time identifier is given by a `Tid`.
 /// If it is the Tid of a basic block, then it describes the point in time *before* execution of the first instruction in the block.
 /// If it is the Tid of a Def or Jmp, then it describes the point in time *after* the execution of the Def or Jmp.
-#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone, PartialOrd, Ord)]
-pub struct AbstractIdentifier(FastCmpArc<AbstractIdentifierData>);
+
+#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone, PartialOrd, Ord, Deref)]
+#[deref(forward)]
+pub struct AbstractIdentifier(Arc<AbstractIdentifierData>);
 
 /// The data contained in an abstract identifier
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone, PartialOrd, Ord)]
@@ -27,10 +29,7 @@ pub struct AbstractIdentifierData {
 impl AbstractIdentifier {
     /// create a new abstract identifier
     pub fn new(time: Tid, location: AbstractLocation) -> AbstractIdentifier {
-        AbstractIdentifier(FastCmpArc(Arc::new(AbstractIdentifierData {
-            time,
-            location,
-        })))
+        AbstractIdentifier(Arc::new(AbstractIdentifierData { time, location }))
     }
 }
 

cwe_checker_rs/src/analysis/pointer_inference/mod.rs

@@ -55,7 +55,7 @@ impl<'a> PointerInference<'a> {
                 if let Some((start_node_index, _end_node_index)) =
                     tid_to_graph_indices_map.get(&block_tid)
                 {
-                    Some((sub_tid.clone(), start_node_index.clone()))
+                    Some((sub_tid.clone(), *start_node_index))
                 } else {
                     None
                 }
@@ -147,8 +147,7 @@ impl<'a> PointerInference<'a> {
                 .term
                 .extern_symbols
                 .iter()
-                .find(|symbol| symbol.tid == sub.tid)
-                .is_some()
+                .any(|symbol| symbol.tid == sub.tid)
             {
                 continue; // We ignore functions marked as extern symbols.
             }
@@ -160,19 +159,15 @@ impl<'a> PointerInference<'a> {
         let mut new_entry_points = Vec::new();
         for (node_id, node) in graph.node_references() {
             if let Node::BlkStart(block) = node {
-                if start_block_to_sub_map.get(&block.tid).is_some()
-                    && self.computation.get_node_value(node_id).is_none()
-                {
-                    if only_cfg_roots
+                if !(start_block_to_sub_map.get(&block.tid).is_none()
+                    || self.computation.get_node_value(node_id).is_some()
+                    || only_cfg_roots
                         && graph
                             .neighbors_directed(node_id, Direction::Incoming)
                             .next()
-                            .is_none()
-                    {
-                        new_entry_points.push(node_id);
-                    } else if !only_cfg_roots {
-                        new_entry_points.push(node_id);
-                    }
+                            .is_some())
+                {
+                    new_entry_points.push(node_id);
                 }
             }
         }

cwe_checker_rs/src/analysis/pointer_inference/object.rs

@@ -210,7 +210,7 @@ impl AbstractObjectInfo {
         } else {
             self.memory = MemRegion::new(self.memory.get_address_bitsize());
         }
-        return Ok(());
+        Ok(())
     }
 
     fn get_all_possible_pointer_targets(&self) -> BTreeSet<AbstractIdentifier> {
@@ -222,7 +222,7 @@ impl AbstractObjectInfo {
                 }
             };
         }
-        return targets;
+        targets
     }
 
     /// For pointer values replace an abstract identifier with another one and add the offset_adjustment to the pointer offsets.
@@ -245,11 +245,9 @@ impl AbstractObjectInfo {
     pub fn set_state(&mut self, new_state: Option<ObjectState>) {
         if self.is_unique {
             self.state = new_state;
-        } else {
-            if self.state != new_state {
-                self.state = None;
-            } // else don't change the state
-        }
+        } else if self.state != new_state {
+            self.state = None;
+        } // else don't change the state
     }
 
     /// Remove the provided IDs from the target lists of all pointers in the memory object.

cwe_checker_rs/src/analysis/pointer_inference/object_list.rs

@@ -55,7 +55,7 @@ impl AbstractObjectList {
                 }
             }
         }
-        return false;
+        false
     }
 
     /// Get the value at a given address.
@@ -87,7 +87,7 @@ impl AbstractObjectList {
                         break;
                     }
                 }
-                merged_value.ok_or(anyhow!("Pointer without targets encountered."))
+                merged_value.ok_or_else(|| anyhow!("Pointer without targets encountered."))
             }
         }
     }
@@ -102,7 +102,7 @@ impl AbstractObjectList {
         for (id, _offset) in pointer.iter_targets() {
             target_object_set.insert(self.ids.get(id).unwrap().0);
         }
-        if target_object_set.len() == 0 {
+        if target_object_set.is_empty() {
             return Err(anyhow!("Pointer without targets encountered"));
         }
         if target_object_set.len() == 1 {
@@ -170,7 +170,7 @@ impl AbstractObjectList {
         let mut merged_objects = self.objects.clone();
         let mut merged_ids = self.ids.clone();
         for (other_id, (other_index, other_offset)) in other.ids.iter() {
-            if let Some((index, offset)) = merged_ids.get(&other_id).clone() {
+            if let Some((index, offset)) = merged_ids.get(&other_id) {
                 let (index, offset) = (*index, offset.clone());
                 merged_ids.insert(other_id.clone(), (index, offset.merge(&other_offset)));
                 if index < self.objects.len() {
@@ -322,24 +322,22 @@ impl AbstractObjectList {
                 }
                 Arc::make_mut(object).set_state(None);
             }
-        } else {
-            if let Some(id) = ids.iter().next() {
-                let object = &mut self.objects[self.ids[&id].0];
-                if let AbstractObject::Memory(tracked_mem) = Arc::deref(object) {
-                    if tracked_mem.state != Some(ObjectState::Alive) {
-                        // Possible double free detected
-                        // TODO: Check rate of false positives.
-                        // If too high, only mark those with explicit dangling state.
-                        possible_double_free_ids.push(id.clone());
-                    }
+        } else if let Some(id) = ids.iter().next() {
+            let object = &mut self.objects[self.ids[&id].0];
+            if let AbstractObject::Memory(tracked_mem) = Arc::deref(object) {
+                if tracked_mem.state != Some(ObjectState::Alive) {
+                    // Possible double free detected
+                    // TODO: Check rate of false positives.
+                    // If too high, only mark those with explicit dangling state.
+                    possible_double_free_ids.push(id.clone());
                 }
-                Arc::make_mut(object).set_state(Some(ObjectState::Dangling));
             }
+            Arc::make_mut(object).set_state(Some(ObjectState::Dangling));
         }
         if possible_double_free_ids.is_empty() {
-            return Ok(());
+            Ok(())
         } else {
-            return Err(possible_double_free_ids);
+            Err(possible_double_free_ids)
         }
     }
 
@@ -380,11 +378,10 @@ impl AbstractObjectList {
             }
         }
         let mut old_to_new_index_map: BTreeMap<usize, usize> = BTreeMap::new();
-        for old_index in 0..other_object_list.objects.len() {
-            if objects_already_known[old_index] == false {
+        for (old_index, old_object) in other_object_list.objects.iter().enumerate() {
+            if !objects_already_known[old_index] {
                 old_to_new_index_map.insert(old_index, self.objects.len());
-                self.objects
-                    .push(other_object_list.objects[old_index].clone());
+                self.objects.push(old_object.clone());
             }
         }
         for (id, (old_index, offset)) in other_object_list.ids.iter() {

cwe_checker_rs/src/analysis/pointer_inference/state.rs

@@ -98,7 +98,7 @@ impl State {
             }
             Ok(())
         } else {
-            return Err(anyhow!("Variable is not a register type"));
+            Err(anyhow!("Variable is not a register type"))
         }
     }
 
@@ -138,8 +138,7 @@ impl State {
             .filter_map(|(register, value)| {
                 if callee_saved_register_names
                     .iter()
-                    .find(|reg_name| **reg_name == register.name)
-                    .is_some()
+                    .any(|reg_name| **reg_name == register.name)
                 {
                     Some((register.clone(), value.clone()))
                 } else {
@@ -284,11 +283,11 @@ impl State {
             // A more precise solution would write to every caller memory region separately,
             // but would also need to check first whether the target memory region is unique or not.
             self.memory.set_value(pointer, value.clone())?;
-            return Ok(());
+            Ok(())
         } else {
             // TODO: Implement recognition of stores to global memory.
             // Needs implementation of reads from global data first.
-            return Err(anyhow!("Memory write to non-pointer data"));
+            Err(anyhow!("Memory write to non-pointer data"))
         }
     }
 
@@ -313,10 +312,10 @@ impl State {
             size,
         } = store_exp
         {
-            let data = self.eval(value).unwrap_or(Data::new_top(*size));
+            let data = self.eval(value).unwrap_or_else(|_| Data::new_top(*size));
             assert_eq!(data.bitsize(), *size);
             // TODO: At the moment, both memory and endianness are ignored. Change that!
-            return self.write_to_address(address, &data);
+            self.write_to_address(address, &data)
         } else {
             panic!("Expected store expression")
         }
@@ -343,7 +342,7 @@ impl State {
             }
         }
         // We only return the last error encountered.
-        return result_log;
+        result_log
     }
 
     /// merge two states
@@ -353,7 +352,7 @@ impl State {
         for (register, other_value) in other.register.iter() {
             if let Some(value) = self.register.get(register) {
                 let merged_value = value.merge(other_value);
-                if merged_value.is_top() == false {
+                if !merged_value.is_top() {
                     // We only have to keep non-top elements.
                     merged_register.insert(register.clone(), merged_value);
                 }
@@ -393,7 +392,7 @@ impl State {
                     match offset {
                         BitvectorDomain::Value(offset_val) => {
                             if offset_val.try_to_i64().unwrap() >= 0
-                                && self.caller_stack_ids.len() > 0
+                                && !self.caller_stack_ids.is_empty()
                             {
                                 for caller_id in self.caller_stack_ids.iter() {
                                     new_targets.add_target(caller_id.clone(), offset.clone());
@@ -415,9 +414,9 @@ impl State {
                     new_targets.add_target(id.clone(), offset.clone());
                 }
             }
-            return Data::Pointer(new_targets);
+            Data::Pointer(new_targets)
         } else {
-            return address.clone();
+            address.clone()
         }
     }
 
@@ -486,7 +485,7 @@ impl State {
                 }
             }
         }
-        return ids;
+        ids
     }
 
     /// Merge the callee stack with the caller stack.
@@ -531,7 +530,7 @@ impl State {
             .register
             .clone()
             .into_iter()
-            .filter(|(register, _value)| register.is_temp == false)
+            .filter(|(register, _value)| !register.is_temp)
             .collect();
     }
 

cwe_checker_rs/src/ffi/analysis.rs

@@ -15,12 +15,12 @@ fn run_pointer_inference(program_jsonbuilder_val: ocaml::Value) -> (Vec<CweWarni
 }
 
 caml!(rs_run_pointer_inference(program_jsonbuilder_val) {
-    return failwith_on_panic( || {
+    failwith_on_panic( || {
         let cwe_warnings_and_log = run_pointer_inference(program_jsonbuilder_val);
         let cwe_warnings_and_log_json = serde_json::to_string(&cwe_warnings_and_log).unwrap();
         let ocaml_string = ocaml::Str::from(&cwe_warnings_and_log_json as &str);
         ocaml::Value::from(ocaml_string)
-    });
+    })
 });
 
 fn run_pointer_inference_and_print_debug(program_jsonbuilder_val: ocaml::Value) {
@@ -33,8 +33,8 @@ fn run_pointer_inference_and_print_debug(program_jsonbuilder_val: ocaml::Value)
 }
 
 caml!(rs_run_pointer_inference_and_print_debug(program_jsonbuilder_val) {
-    return failwith_on_panic( || {
+    failwith_on_panic( || {
         run_pointer_inference_and_print_debug(program_jsonbuilder_val);
         ocaml::Value::unit()
-    });
+    })
 });

cwe_checker_rs/src/ffi/serde.rs

@@ -73,10 +73,10 @@ impl From<&JsonBuilder> for serde_json::Value {
 }
 
 caml!(rs_finalize_json_builder(builder_val) {
-    return failwith_on_panic( || {
+    failwith_on_panic( || {
         JsonBuilder::ocaml_finalize(builder_val);
         ocaml::Value::unit()
-    });
+    })
 });
 
 /// Build JsonBuilder::Null as Ocaml value
@@ -85,9 +85,9 @@ fn build_serde_null() -> ocaml::Value {
 }
 
 caml!(rs_build_serde_null(_unit) {
-    return failwith_on_panic( || {
+    failwith_on_panic( || {
         build_serde_null()
-    });
+    })
 });
 
 /// Build JsonBuilder::Bool as Ocaml value
@@ -97,9 +97,9 @@ fn build_serde_bool(bool_val: ocaml::Value) -> ocaml::Value {
 }
 
 caml!(rs_build_serde_bool(bool_val) {
-    return failwith_on_panic( || {
+    failwith_on_panic( || {
         build_serde_bool(bool_val)
-    });
+    })
 });
 
 /// Build JsonBuilder::Number as Ocaml value
@@ -109,9 +109,9 @@ fn build_serde_number(num: ocaml::Value) -> ocaml::Value {
 }
 
 caml!(rs_build_serde_number(number) {
-    return failwith_on_panic( || {
+    failwith_on_panic( || {
         build_serde_number(number)
-    });
+    })
 });
 
 /// Build JsonBuilder::Object representing a bitvector from a string generated by `Bitvector.to_string` in Ocaml
@@ -127,7 +127,7 @@ fn build_serde_bitvector(bitvector_string_val: ocaml::Value) -> ocaml::Value {
     if number_slice.starts_with("0x") {
         number_slice = &number_slice[2..];
     }
-    while number_slice.len() > 0 {
+    while !number_slice.is_empty() {
         if number_slice.len() > 16 {
             let digit = u64::from_str_radix(&number_slice[(number_slice.len() - 16)..], 16)
                 .expect("Bitvector value parsing failed");
@@ -151,18 +151,14 @@ fn build_serde_bitvector(bitvector_string_val: ocaml::Value) -> ocaml::Value {
         ("digits".to_string(), Rc::new(JsonBuilder::Array(num_list))),
         ("width".to_string(), Rc::new(JsonBuilder::Array(width_list))),
     ]);
-    // TODO: remove deserialization check
-    let check_serde = serde_json::to_string(&serde_json::Value::from(&result)).unwrap();
-    let _bitv: apint::ApInt = serde_json::from_str(&check_serde)
-        .expect(&format!("Invalid value generated: {}", check_serde));
 
     result.to_ocaml()
 }
 
 caml!(rs_build_serde_bitvector(bitvector_string) {
-    return failwith_on_panic( || {
+    failwith_on_panic( || {
         build_serde_bitvector(bitvector_string)
-    });
+    })
 });
 
 /// Build JsonBuilder::String as Ocaml value
@@ -172,9 +168,9 @@ fn build_serde_string(string_val: ocaml::Value) -> ocaml::Value {
 }
 
 caml!(rs_build_serde_string(string_val) {
-    return failwith_on_panic( || {
+    failwith_on_panic( || {
         build_serde_string(string_val)
-    });
+    })
 });
 
 /// Build JsonBuilder::Array as Ocaml value from an Ocaml list
@@ -189,9 +185,9 @@ fn build_serde_array_from_list(list_val: ocaml::Value) -> ocaml::Value {
 }
 
 caml!(rs_build_serde_array_from_list(list_val) {
-    return failwith_on_panic( || {
+    failwith_on_panic( || {
         build_serde_array_from_list(list_val)
-    });
+    })
 });
 
 /// Build JsonBuilder::Object as Ocaml value from an Ocaml list of tuples
@@ -217,9 +213,9 @@ fn build_serde_object(tuple_list_val: ocaml::Value) -> ocaml::Value {
 }
 
 caml!(rs_build_serde_object(tuple_list_val) {
-    return failwith_on_panic( || {
+    failwith_on_panic( || {
         build_serde_object(tuple_list_val)
-    });
+    })
 });
 
 /// Get the Json string corresponding to a JsonBuilder object and return it as an Ocaml value.
@@ -230,7 +226,7 @@ fn get_json_string(builder_val: ocaml::Value) -> ocaml::Value {
 }
 
 caml!(rs_convert_json_to_string(builder_val) {
-    return failwith_on_panic( || {
+    failwith_on_panic( || {
         get_json_string(builder_val)
-    });
+    })
 });

cwe_checker_rs/src/term/symbol.rs

@@ -29,7 +29,7 @@ impl ExternSymbol {
         }
         match &return_args[0].location {
             Expression::Var(var) => Ok(var),
-            _ => Err(anyhow!("Return location is not a register"))?,
+            _ => Err(anyhow!("Return location is not a register")),
         }
     }
 

cwe_checker_rs/src/utils/fast_cmp_arc.rs

-use crate::analysis::abstract_domain::AbstractDomain;
-use crate::prelude::*;
-use std::ops::{Deref, DerefMut};
-use std::sync::Arc;
-
-// TODO: This is a helper not only for abstract domains! It needs its own source file!
-#[derive(Serialize, Deserialize, Debug, Hash, Clone)]
-pub struct FastCmpArc<T>(pub Arc<T>);
-
-impl<T: PartialEq + Eq> PartialEq for FastCmpArc<T> {
-    fn eq(&self, other: &Self) -> bool {
-        if Arc::ptr_eq(&self.0, &other.0) {
-            true
-        } else {
-            self.0.eq(&other.0)
-        }
-    }
-}
-
-impl<T: Eq> Eq for FastCmpArc<T> {}
-
-impl<T: AbstractDomain + Clone> AbstractDomain for FastCmpArc<T> {
-    fn top(&self) -> Self {
-        FastCmpArc(Arc::new(self.0.top()))
-    }
-
-    fn merge(&self, other: &Self) -> Self {
-        if Arc::ptr_eq(&self.0, &other.0) {
-            self.clone()
-        } else {
-            FastCmpArc(Arc::new(self.0.merge(&other.0)))
-        }
-    }
-}
-
-impl<T: PartialOrd + Ord> PartialOrd for FastCmpArc<T> {
-    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
-        Some(self.cmp(other))
-    }
-}
-
-impl<T: PartialOrd + Ord> Ord for FastCmpArc<T> {
-    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
-        if Arc::ptr_eq(&self.0, &other.0) {
-            std::cmp::Ordering::Equal
-        } else {
-            self.0.cmp(&other.0)
-        }
-    }
-}
-
-impl<T> Deref for FastCmpArc<T> {
-    type Target = T;
-    fn deref(&self) -> &T {
-        &self.0
-    }
-}
-
-impl<T: Clone> DerefMut for FastCmpArc<T> {
-    fn deref_mut(&mut self) -> &mut T {
-        Arc::make_mut(&mut self.0)
-    }
-}

cwe_checker_rs/src/utils/mod.rs

-pub mod fast_cmp_arc;
 pub mod log;