MIPS: prevent gp register value loss (#213)

src/cwe_checker_lib/src/analysis/pointer_inference/context/mod.rs

@@ -64,6 +64,26 @@ impl<'a> Context<'a> {
         }
     }
 
+    /// Return `true` if the all of the following properties hold:
+    /// * The CPU architecture is a MIPS variant and `var` is the MIPS global pointer register `gp`
+    /// * Loading the value at `address` into the register `var` would overwrite the value of `var` with a `Top` value.
+    fn is_mips_gp_load_to_top_value(
+        &self,
+        state: &State,
+        var: &Variable,
+        address: &Expression,
+    ) -> bool {
+        if self.project.cpu_architecture.contains("MIPS") && var.name == "gp" {
+            if let Ok(gp_val) = state.load_value(address, var.size, self.runtime_memory_image) {
+                gp_val.is_top()
+            } else {
+                true
+            }
+        } else {
+            false
+        }
+    }
+
     /// If `result` is an `Err`, log the error message as a debug message through the `log_collector` channel.
     pub fn log_debug(&self, result: Result<(), Error>, location: Option<&Tid>) {
         if let Err(err) = result {

src/cwe_checker_lib/src/analysis/pointer_inference/context/trait_impls.rs

@@ -76,10 +76,16 @@ impl<'a> crate::analysis::forward_interprocedural_fixpoint::Context<'a> for Cont
                 Some(new_state)
             }
             Def::Load { var, address } => {
-                self.log_debug(
-                    new_state.handle_load(var, address, self.runtime_memory_image),
-                    Some(&def.tid),
-                );
+                if !self.is_mips_gp_load_to_top_value(state, var, address) {
+                    self.log_debug(
+                        new_state.handle_load(var, address, self.runtime_memory_image),
+                        Some(&def.tid),
+                    );
+                }
+                // Else we ignore the load and hope that the value still contained in the gp register is still correct.
+                // This only works because gp is (incorrectly) marked as a callee-saved register.
+                // FIXME: If the rest of the analysis becomes good enough so that this case is not common anymore,
+                // we should log it.
                 Some(new_state)
             }
         }
@@ -163,6 +169,11 @@ impl<'a> crate::analysis::forward_interprocedural_fixpoint::Context<'a> for Cont
                     Bitvector::zero(apint::BitWidth::from(address_bytesize)).into(),
                 ),
             );
+            // For MIPS architecture only: Ensure that the t9 register contains the address of the called function
+            if self.project.cpu_architecture.contains("MIPS") {
+                let _ = callee_state
+                    .set_mips_link_register(callee_tid, self.project.stack_pointer_register.size);
+            }
             // 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);

src/cwe_checker_lib/src/analysis/pointer_inference/mod.rs

@@ -157,12 +157,14 @@ impl<'a> PointerInference<'a> {
             ))));
         }
         for (sub_tid, start_node_index) in entry_sub_to_entry_node_map.into_iter() {
+            let mut fn_entry_state = State::new(&project.stack_pointer_register, sub_tid.clone());
+            if project.cpu_architecture.contains("MIPS") {
+                let _ = fn_entry_state
+                    .set_mips_link_register(&sub_tid, project.stack_pointer_register.size);
+            }
             fixpoint_computation.set_node_value(
                 start_node_index,
-                super::interprocedural_fixpoint_generic::NodeValue::Value(State::new(
-                    &project.stack_pointer_register,
-                    sub_tid,
-                )),
+                super::interprocedural_fixpoint_generic::NodeValue::Value(fn_entry_state),
             );
         }
         PointerInference {
@@ -292,12 +294,14 @@ impl<'a> PointerInference<'a> {
                 [&self.computation.get_graph()[entry].get_block().tid]
                 .tid
                 .clone();
+            let mut fn_entry_state = State::new(&project.stack_pointer_register, sub_tid.clone());
+            if project.cpu_architecture.contains("MIPS") {
+                let _ = fn_entry_state
+                    .set_mips_link_register(&sub_tid, project.stack_pointer_register.size);
+            }
             self.computation.set_node_value(
                 entry,
-                super::interprocedural_fixpoint_generic::NodeValue::Value(State::new(
-                    &project.stack_pointer_register,
-                    sub_tid,
-                )),
+                super::interprocedural_fixpoint_generic::NodeValue::Value(fn_entry_state),
             );
         }
     }

src/cwe_checker_lib/src/analysis/pointer_inference/state/mod.rs

@@ -62,6 +62,34 @@ impl State {
         }
     }
 
+    /// Set the MIPS link register `t9` to the address of the callee TID.
+    ///
+    /// According to the System V ABI for MIPS the caller has to save the callee address in register `t9`
+    /// on a function call to position-independent code.
+    /// This function manually sets `t9` to the correct value
+    /// to mitigate cases where `t9` could not be correctly computed due to previous analysis errors.
+    ///
+    /// Returns an error if the callee address could not be parsed (e.g. for `UNKNOWN` addresses).
+    pub fn set_mips_link_register(
+        &mut self,
+        callee_tid: &Tid,
+        generic_pointer_size: ByteSize,
+    ) -> Result<(), Error> {
+        let link_register = Variable {
+            name: "t9".into(),
+            size: generic_pointer_size,
+            is_temp: false,
+        };
+        let address = Bitvector::from_u64(u64::from_str_radix(&callee_tid.address, 16)?)
+            .into_resize_unsigned(generic_pointer_size);
+        // FIXME: A better way would be to test whether the link register contains the correct value
+        // and only fix and log cases where it doesn't contain the correct value.
+        // Right now this is unfortunately the common case,
+        // so logging every case would generate too many log messages.
+        self.set_register(&link_register, address.into());
+        Ok(())
+    }
+
     /// Clear all non-callee-saved registers from the state.
     /// This automatically also removes all virtual registers.
     /// The parameter is a list of callee-saved register names.