Small code refactorings (#396)

src/caller/src/main.rs

@@ -130,14 +130,8 @@ fn run_with_ghidra(args: &CmdlineArgs) -> Result<(), Error> {
         disassemble_binary(&binary_file_path, bare_metal_config_opt, args.verbose)?;
 
     // Generate the control flow graph of the program
-    let extern_sub_tids = project
-        .program
-        .term
-        .extern_symbols
-        .keys()
-        .cloned()
-        .collect();
-    let control_flow_graph = graph::get_program_cfg(&project.program, extern_sub_tids);
+    let (control_flow_graph, mut logs_graph) = graph::get_program_cfg_with_logs(&project.program);
+    all_logs.append(&mut logs_graph);
 
     let analysis_results = AnalysisResults::new(&binary, &control_flow_graph, &project);
 
@@ -204,6 +198,7 @@ fn run_with_ghidra(args: &CmdlineArgs) -> Result<(), Error> {
         all_logs.append(&mut logs);
         all_cwes.append(&mut cwes);
     }
+    all_cwes.sort();
 
     // Print the results of the modules.
     if args.quiet {

src/cwe_checker_lib/src/analysis/backward_interprocedural_fixpoint/mock_context.rs

 use super::*;
 use crate::analysis::graph::Graph;
 use petgraph::graph::NodeIndex;
-use std::collections::{HashMap, HashSet};
+use std::collections::HashMap;
 
 /// Identifier for BlkStart and BlkEnd nodes
 #[derive(Clone, PartialEq, Eq, Hash)]
@@ -19,7 +19,7 @@ pub struct Context<'a> {
 
 impl<'a> Context<'a> {
     pub fn new(project: &'a Project) -> Self {
-        let mut graph = crate::analysis::graph::get_program_cfg(&project.program, HashSet::new());
+        let mut graph = crate::analysis::graph::get_program_cfg(&project.program);
         graph.reverse();
         let mut tid_to_node_index: HashMap<(Tid, Tid, StartEnd), NodeIndex> = HashMap::new();
         for node in graph.node_indices() {

src/cwe_checker_lib/src/analysis/dead_variable_elimination/mod.rs

@@ -14,14 +14,7 @@ use alive_vars_computation::*;
 /// Returns a map that assigns to each basic block `Tid` the set of all variables
 /// that are alive at the end of the basic block.
 pub fn compute_alive_vars(project: &Project) -> HashMap<Tid, BTreeSet<Variable>> {
-    let extern_subs = project
-        .program
-        .term
-        .extern_symbols
-        .keys()
-        .cloned()
-        .collect();
-    let mut graph = crate::analysis::graph::get_program_cfg(&project.program, extern_subs);
+    let mut graph = crate::analysis::graph::get_program_cfg(&project.program);
     graph.reverse();
     let context = Context::new(project, &graph);
     let all_physical_registers = context.all_physical_registers.clone();

src/cwe_checker_lib/src/analysis/expression_propagation/mod.rs

@@ -330,16 +330,9 @@ pub fn merge_def_assignments_to_same_var(blk: &mut Term<Blk>) {
 ///
 /// This is performed by a fixpoint computation and might panic, if it does not stabilize.
 pub fn propagate_input_expression(project: &mut Project) {
-    let extern_subs = project
-        .program
-        .term
-        .extern_symbols
-        .keys()
-        .cloned()
-        .collect();
     merge_same_var_assignments(project);
 
-    let graph = crate::analysis::graph::get_program_cfg(&project.program, extern_subs);
+    let graph = crate::analysis::graph::get_program_cfg(&project.program);
     let computation = compute_expression_propagation(&graph);
     let results = extract_results(&graph, computation);
     insert_expressions(results, &mut project.program.term);

src/cwe_checker_lib/src/analysis/expression_propagation/tests.rs

@@ -176,15 +176,7 @@ fn no_propagation_on_calls() {
 /// variable-expressions pairs.
 fn insertion_table_update() {
     let project = &mock_project();
-    let extern_subs = project
-        .program
-        .term
-        .extern_symbols
-        .keys()
-        .cloned()
-        .collect();
-
-    let graph = crate::analysis::graph::get_program_cfg(&project.program, extern_subs);
+    let graph = crate::analysis::graph::get_program_cfg(&project.program);
     let context = Context::new(&graph);
 
     let blk = get_mock_entry_block().term;

src/cwe_checker_lib/src/analysis/forward_interprocedural_fixpoint.rs

@@ -336,7 +336,7 @@ mod tests {
         expr,
         intermediate_representation::*,
     };
-    use std::collections::{BTreeMap, HashMap, HashSet};
+    use std::collections::{BTreeMap, HashMap};
 
     fn new_block(name: &str) -> Term<Blk> {
         Term {
@@ -403,9 +403,7 @@ mod tests {
     /// Checks if the nodes corresponding to the callee function are first in the worklist.
     fn check_bottom_up_worklist() {
         let project = mock_project();
-        let extern_subs = HashSet::new();
-
-        let graph = crate::analysis::graph::get_program_cfg(&project.program, extern_subs);
+        let graph = crate::analysis::graph::get_program_cfg(&project.program);
         let context = Context::new(&graph);
         let comp = create_computation_with_bottom_up_worklist_order(context, Some(HashMap::new()));
         // The last two nodes should belong to the callee
@@ -423,9 +421,7 @@ mod tests {
     #[test]
     fn check_top_down_worklist() {
         let project = mock_project();
-        let extern_subs = HashSet::new();
-
-        let graph = crate::analysis::graph::get_program_cfg(&project.program, extern_subs);
+        let graph = crate::analysis::graph::get_program_cfg(&project.program);
         let context = Context::new(&graph);
         let comp = create_computation_with_top_down_worklist_order(context, Some(HashMap::new()));
         // The first two nodes should belong to the callee

src/cwe_checker_lib/src/analysis/function_signature/context/tests.rs

 use super::*;
 use crate::{bitvec, variable};
-use std::collections::HashSet;
 
 #[test]
 fn test_compute_return_values_of_call() {
     let project = Project::mock_x64();
-    let graph = crate::analysis::graph::get_program_cfg(&project.program, HashSet::new());
+    let graph = crate::analysis::graph::get_program_cfg(&project.program);
 
     let context = Context::new(&project, &graph);
 
@@ -54,7 +53,7 @@ fn test_compute_return_values_of_call() {
 #[test]
 fn test_call_stub_handling() {
     let project = Project::mock_arm32();
-    let graph = crate::analysis::graph::get_program_cfg(&project.program, HashSet::new());
+    let graph = crate::analysis::graph::get_program_cfg(&project.program);
 
     let context = Context::new(&project, &graph);
 
@@ -117,7 +116,7 @@ fn test_call_stub_handling() {
 #[test]
 fn test_get_global_mem_address() {
     let project = Project::mock_arm32();
-    let graph = crate::analysis::graph::get_program_cfg(&project.program, HashSet::new());
+    let graph = crate::analysis::graph::get_program_cfg(&project.program);
 
     let context = Context::new(&project, &graph);
     // Check global address from abstract ID

src/cwe_checker_lib/src/analysis/graph.rs

@@ -44,6 +44,7 @@
 
 use crate::intermediate_representation::*;
 use crate::prelude::*;
+use crate::utils::log::LogMessage;
 use petgraph::graph::{DiGraph, NodeIndex};
 use std::collections::{HashMap, HashSet};
 
@@ -188,6 +189,8 @@ struct GraphBuilder<'a> {
     return_addresses: HashMap<Tid, Vec<(NodeIndex, NodeIndex)>>,
     /// A list of `BlkEnd` nodes for which outgoing edges still have to be added to the graph.
     block_worklist: Vec<NodeIndex>,
+    /// List of `LogMessage` generated by `build` function.
+    log_messages: Vec<LogMessage>,
 }
 
 impl<'a> GraphBuilder<'a> {
@@ -201,6 +204,7 @@ impl<'a> GraphBuilder<'a> {
             jump_targets: HashMap::new(),
             return_addresses: HashMap::new(),
             block_worklist: Vec::new(),
+            log_messages: Vec::new(),
         }
     }
 
@@ -237,8 +241,12 @@ impl<'a> GraphBuilder<'a> {
                 let start_block = &sub.term.blocks[0];
                 let target_index = self.jump_targets[&(start_block.tid.clone(), sub.tid.clone())];
                 self.call_targets.insert(sub.tid.clone(), target_index);
+            } else {
+                self.log_messages.push(LogMessage::new_info(format!(
+                    "{} contains no blocks",
+                    sub.tid
+                )))
             }
-            // TODO: Generate Log-Message for Subs without blocks.
         }
     }
 
@@ -479,19 +487,25 @@ impl<'a> GraphBuilder<'a> {
     }
 
     /// Build the interprocedural control flow graph.
-    pub fn build(mut self) -> Graph<'a> {
+    pub fn build(&mut self) -> Graph<'a> {
         self.add_program_blocks();
         self.add_subs_to_call_targets();
         self.add_jump_and_call_edges();
         self.add_return_edges();
-        self.graph
+        self.graph.clone()
     }
 }
 
 /// Build 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);
-    builder.build()
+pub fn get_program_cfg(program: &Term<Program>) -> Graph {
+    get_program_cfg_with_logs(program).0
+}
+
+/// Build the interprocedural control flow graph for a program term with log messages created by building.
+pub fn get_program_cfg_with_logs(program: &Term<Program>) -> (Graph, Vec<LogMessage>) {
+    let extern_subs = program.term.extern_symbols.keys().cloned().collect();
+    let mut builder = GraphBuilder::new(program, extern_subs);
+    (builder.build(), builder.log_messages)
 }
 
 /// Returns a map from function TIDs to the node index of the `BlkStart` node of the first block in the function.
@@ -610,7 +624,7 @@ mod tests {
     #[test]
     fn create_program_cfg() {
         let program = mock_program();
-        let graph = get_program_cfg(&program, HashSet::new());
+        let graph = get_program_cfg(&program);
         println!("{}", serde_json::to_string_pretty(&graph).unwrap());
         assert_eq!(graph.node_count(), 16);
         assert_eq!(graph.edge_count(), 20);
@@ -646,7 +660,7 @@ mod tests {
             tid: Tid::new("program".to_string()),
             term: program,
         };
-        let graph = get_program_cfg(&program_term, HashSet::new());
+        let graph = get_program_cfg(&program_term);
         assert_eq!(graph.node_count(), 2);
         assert_eq!(graph.edge_count(), 2);
     }

src/cwe_checker_lib/src/checkers/cwe_119/mod.rs

@@ -89,7 +89,6 @@ pub fn check_cwe(
 
     fixpoint_computation.compute_with_max_steps(100);
 
-    let (logs, mut cwe_warnings) = log_thread.collect();
-    cwe_warnings.sort();
+    let (logs, cwe_warnings) = log_thread.collect();
     (logs, cwe_warnings)
 }

src/cwe_checker_lib/src/checkers/cwe_134.rs

@@ -105,7 +105,6 @@ pub fn check_cwe(
         }
     }
 
-    cwe_warnings.sort();
     (Vec::new(), cwe_warnings)
 }
 
@@ -176,8 +175,6 @@ fn generate_cwe_warning(
 
 #[cfg(test)]
 pub mod tests {
-    use std::collections::HashSet;
-
     use crate::analysis::pointer_inference::PointerInference as PointerInferenceComputation;
     use crate::{defs, intermediate_representation::*};
 
@@ -209,7 +206,7 @@ pub mod tests {
     fn test_locate_format_string() {
         let sprintf_symbol = ExternSymbol::mock_sprintf_x64();
         let project = mock_project();
-        let graph = crate::analysis::graph::get_program_cfg(&project.program, HashSet::new());
+        let graph = crate::analysis::graph::get_program_cfg(&project.program);
         let mut pi_results = PointerInferenceComputation::mock(&project);
         pi_results.compute(false);
         let mut format_string_index: HashMap<String, usize> = HashMap::new();

src/cwe_checker_lib/src/checkers/cwe_190.rs

@@ -175,6 +175,5 @@ pub fn check_cwe(
         }
     }
 
-    cwe_warnings.sort();
     (Vec::new(), cwe_warnings)
 }

src/cwe_checker_lib/src/checkers/cwe_243.rs

@@ -175,6 +175,5 @@ pub fn check_cwe(
         }
     }
 
-    cwe_warnings.sort();
     (Vec::new(), cwe_warnings)
 }

src/cwe_checker_lib/src/checkers/cwe_332.rs

@@ -68,6 +68,5 @@ pub fn check_cwe(
             cwe_warnings.push(generate_cwe_warning(secure_initializer_func, rand_func));
         }
     }
-    cwe_warnings.sort();
     (Vec::new(), cwe_warnings)
 }

src/cwe_checker_lib/src/checkers/cwe_367.rs

@@ -119,6 +119,5 @@ pub fn check_cwe(
         }
     }
 
-    cwe_warnings.sort();
     (Vec::new(), cwe_warnings)
 }

src/cwe_checker_lib/src/checkers/cwe_416/mod.rs

@@ -78,7 +78,6 @@ pub fn check_cwe(
     }
 
     fixpoint_computation.compute_with_max_steps(100);
-    let (logs, mut cwe_warnings) = log_thread.collect();
-    cwe_warnings.sort();
+    let (logs, cwe_warnings) = log_thread.collect();
     (logs, cwe_warnings)
 }

src/cwe_checker_lib/src/checkers/cwe_426.rs

@@ -96,6 +96,5 @@ pub fn check_cwe(
             }
         }
     }
-    cwe_warnings.sort();
     (Vec::new(), cwe_warnings)
 }

src/cwe_checker_lib/src/checkers/cwe_467.rs

@@ -120,6 +120,5 @@ pub fn check_cwe(
             }
         }
     }
-    cwe_warnings.sort();
     (Vec::new(), cwe_warnings)
 }

src/cwe_checker_lib/src/checkers/cwe_560.rs

@@ -133,6 +133,5 @@ pub fn check_cwe(
         }
     }
 
-    cwes.sort();
     (log_messages, cwes)
 }

src/cwe_checker_lib/src/checkers/cwe_676.rs

@@ -84,7 +84,6 @@ pub fn generate_cwe_warnings<'a>(
         cwe_warnings.push(cwe_warning);
     }
 
-    cwe_warnings.sort();
     cwe_warnings
 }
 

src/cwe_checker_lib/src/checkers/cwe_782.rs

@@ -80,6 +80,6 @@ pub fn check_cwe(
             .values()
             .for_each(|sub| warnings.append(&mut handle_sub(sub, symbol)));
     }
-    warnings.sort();
+
     (vec![], warnings)
 }

src/cwe_checker_lib/src/intermediate_representation/project/propagate_control_flow.rs

@@ -16,14 +16,7 @@ use std::collections::{BTreeSet, HashMap, HashSet};
 /// For such a sequence we then retarget the destination of the first jump to the final jump destination of the sequence.
 /// Lastly, the newly bypassed blocks are considered dead code and are removed.
 pub fn propagate_control_flow(project: &mut Project) {
-    let extern_subs: HashSet<Tid> = project
-        .program
-        .term
-        .extern_symbols
-        .keys()
-        .cloned()
-        .collect();
-    let cfg = crate::analysis::graph::get_program_cfg(&project.program, extern_subs.clone());
+    let cfg = crate::analysis::graph::get_program_cfg(&project.program);
     let nodes_without_incoming_edges_at_beginning = get_nodes_without_incoming_edge(&cfg);
 
     let mut jmps_to_retarget = HashMap::new();
@@ -75,7 +68,7 @@ pub fn propagate_control_flow(project: &mut Project) {
     }
     retarget_jumps(project, jmps_to_retarget);
 
-    let cfg = crate::analysis::graph::get_program_cfg(&project.program, extern_subs);
+    let cfg = crate::analysis::graph::get_program_cfg(&project.program);
     let nodes_without_incoming_edges_at_end = get_nodes_without_incoming_edge(&cfg);
 
     remove_new_orphaned_blocks(

src/cwe_checker_lib/src/pipeline/results.rs

@@ -123,15 +123,12 @@ impl<'a> AnalysisResults<'a> {
 mod tests {
     use super::*;
     use crate::analysis::graph::get_program_cfg;
-    use std::collections::HashSet;
 
     impl<'a> AnalysisResults<'a> {
         /// Mocks the `AnalysisResults` struct with a given project.
         /// Note that the function leaks memory!
         pub fn mock_from_project(project: &'a Project) -> AnalysisResults<'a> {
-            let extern_subs =
-                HashSet::from_iter(project.program.term.extern_symbols.keys().cloned());
-            let graph = Box::new(get_program_cfg(&project.program, extern_subs));
+            let graph = Box::new(get_program_cfg(&project.program));
             let graph: &'a Graph = Box::leak(graph);
             let binary: &'a Vec<u8> = Box::leak(Box::new(Vec::new()));
             let analysis_results = AnalysisResults::new(binary, graph, project);