Fix IL lookups that used the wrong kind of index

mediumlevelilinstruction.cpp

@@ -1317,7 +1317,10 @@ bool MediumLevelILInstructionBase::HasLowLevelIL() const
 	Ref<LowLevelILFunction> func = function->GetLowLevelIL();
 	if (!func)
 		return false;
-	return GetLowLevelILExprIndex() < func->GetExprCount();
+	Ref<LowLevelILFunction> ssa = func->GetSSAForm();
+	if (!ssa)
+		return false;
+	return GetLowLevelILExprIndex() < ssa->GetExprCount();
 }
 
 
@@ -1326,10 +1329,13 @@ LowLevelILInstruction MediumLevelILInstructionBase::GetLowLevelIL() const
 	Ref<LowLevelILFunction> func = function->GetLowLevelIL();
 	if (!func)
 		throw LowLevelILInstructionAccessException();
+	Ref<LowLevelILFunction> ssa = func->GetSSAForm();
+	if (!ssa)
+		throw LowLevelILInstructionAccessException();
 	size_t expr = GetLowLevelILExprIndex();
-	if (GetLowLevelILExprIndex() >= func->GetExprCount())
+	if (expr >= ssa->GetExprCount())
 		throw LowLevelILInstructionAccessException();
-	return func->GetExpr(expr);
+	return ssa->GetExpr(expr).GetNonSSAForm();
 }
 
 

plugins/efi_resolver/src/Resolver.cpp

@@ -712,7 +712,7 @@ bool Resolver::resolveGuidInterfaceAtCallsite(const GuidInterfaceCallsite& calls
 				continue;
 			if (ssa.GetSSAVariable().version != 0)
 			{
-				auto incomming_def = hlil->GetSSAVarDefinition(ssa.GetSSAVariable());
+				auto incomming_def = hlilSsa->GetSSAVarDefinition(ssa.GetSSAVariable());
 				if (incomming_def >= hlilSsa->GetExprCount())
 					continue;
 				auto incomming_def_ssa = hlilSsa->GetExpr(incomming_def);

rust/src/high_level_il/function.rs

@@ -145,12 +145,12 @@ impl HighLevelILFunction {
                 variable.version,
             )
         };
-        self.instruction_from_index(HighLevelInstructionIndex(index))
+        self.instruction_from_expr_index(HighLevelExpressionIndex(index))
     }
 
     pub fn ssa_memory_definition(&self, version: usize) -> Option<HighLevelILInstruction> {
         let index = unsafe { BNGetHighLevelILSSAMemoryDefinition(self.handle, version) };
-        self.instruction_from_index(HighLevelInstructionIndex(index))
+        self.instruction_from_expr_index(HighLevelExpressionIndex(index))
     }
 
     /// Gets all the instructions that use the given SSA variable.
@@ -193,7 +193,7 @@ impl HighLevelILFunction {
                 self.handle,
                 &variable.variable.into(),
                 variable.version,
-                instr.expr_index.0,
+                instr.instr_index.0,
             )
         }
     }
@@ -217,7 +217,7 @@ impl HighLevelILFunction {
 
     /// Determines if `variable` is live at a given point in the function
     pub fn is_variable_live_at(&self, variable: Variable, instr: &HighLevelILInstruction) -> bool {
-        unsafe { BNIsHighLevelILVarLiveAt(self.handle, &variable.into(), instr.expr_index.0) }
+        unsafe { BNIsHighLevelILVarLiveAt(self.handle, &variable.into(), instr.instr_index.0) }
     }
 
     /// This gets just the HLIL variables - you may be interested in the union

rust/src/high_level_il/instruction.rs

@@ -1077,7 +1077,10 @@ impl HighLevelILInstruction {
     /// Version of active memory contents in SSA form for this instruction
     pub fn ssa_memory_version(&self) -> usize {
         unsafe {
-            BNGetHighLevelILSSAMemoryVersionAtILInstruction(self.function.handle, self.expr_index.0)
+            BNGetHighLevelILSSAMemoryVersionAtILInstruction(
+                self.function.handle,
+                self.instr_index.0,
+            )
         }
     }
 
@@ -1086,7 +1089,7 @@ impl HighLevelILInstruction {
             BNGetHighLevelILSSAVarVersionAtILInstruction(
                 self.function.handle,
                 &variable.into(),
-                self.expr_index.0,
+                self.instr_index.0,
             )
         };
         SSAVariable::new(variable, version)

rust/src/medium_level_il/function.rs

@@ -360,13 +360,11 @@ impl MediumLevelILFunction {
         let result = unsafe {
             BNGetMediumLevelILSSAVarDefinition(self.handle, &raw_var, ssa_variable.version)
         };
-        // TODO: Does this return the expression or instruction index? Also we dont diff and this prob doesnt work.
         self.instruction_from_index(MediumLevelInstructionIndex(result))
     }
 
     pub fn ssa_memory_definition(&self, version: usize) -> Option<MediumLevelILInstruction> {
         let result = unsafe { BNGetMediumLevelILSSAMemoryDefinition(self.handle, version) };
-        // TODO: Does this return the expression or instruction index? Also we dont diff and this prob doesnt work.
         self.instruction_from_index(MediumLevelInstructionIndex(result))
     }
 
@@ -525,7 +523,7 @@ pub struct ILReferenceSource {
     pub arch: CoreArchitecture,
     pub addr: u64,
     pub graph_type: FunctionGraphType,
-    pub expr_idx: MediumLevelExpressionIndex,
+    pub instr_idx: MediumLevelInstructionIndex,
 }
 
 impl From<BNILReferenceSource> for ILReferenceSource {
@@ -535,7 +533,7 @@ impl From<BNILReferenceSource> for ILReferenceSource {
             arch: unsafe { CoreArchitecture::from_raw(value.arch) },
             addr: value.addr,
             graph_type: value.type_,
-            expr_idx: MediumLevelExpressionIndex(value.exprId),
+            instr_idx: MediumLevelInstructionIndex(value.exprId),
         }
     }
 }
@@ -547,7 +545,7 @@ impl From<&BNILReferenceSource> for ILReferenceSource {
             arch: unsafe { CoreArchitecture::from_raw(value.arch) },
             addr: value.addr,
             graph_type: value.type_,
-            expr_idx: MediumLevelExpressionIndex(value.exprId),
+            instr_idx: MediumLevelInstructionIndex(value.exprId),
         }
     }
 }

rust/src/medium_level_il/instruction.rs

@@ -1311,7 +1311,7 @@ impl MediumLevelILInstruction {
         let value = unsafe {
             BNGetMediumLevelILPossibleExprValues(
                 self.function.handle,
-                self.instr_index.0,
+                self.expr_index.0,
                 options.as_ptr() as *mut _,
                 options.len(),
             )
@@ -2068,9 +2068,9 @@ unsafe impl CoreArrayProviderInner for BranchDependence {
 
     unsafe fn wrap_raw<'a>(raw: &'a Self::Raw, context: &'a Self::Context) -> Self::Wrapped<'a> {
         Self {
-            instruction: MediumLevelILInstruction::from_expr_index(
+            instruction: MediumLevelILInstruction::from_instr_index(
                 context.clone(),
-                MediumLevelExpressionIndex(raw.branch),
+                MediumLevelInstructionIndex(raw.branch),
             ),
             dependence: raw.dependence,
         }

rust/tests/binary_view.rs

@@ -145,10 +145,10 @@ fn test_binary_view_search() {
 
     // Test constant search.
     let constant = 0x80000000;
-    let const_2607b = view
+    let const_26080 = view
         .find_next_constant(0, view.end(), constant, FunctionViewType::MediumLevelIL)
         .expect("Failed to find constant");
-    assert_eq!(const_2607b, image_base + 0x2607b);
+    assert_eq!(const_26080, image_base + 0x26080);
 
     // Test binary search.
     let query = SearchQuery::new("42 2e 64 65 ?? 75 67 24");

rust/tests/high_level_il.rs

@@ -26,12 +26,12 @@ fn test_hlil_info() {
     // 00025f10        )
     let instr_0 = hlil_instr_iter.next().unwrap();
     assert_eq!(instr_0.instr_index, HighLevelInstructionIndex(0));
-    assert_eq!(instr_0.expr_index, HighLevelExpressionIndex(5));
+    assert_eq!(instr_0.expr_index, HighLevelExpressionIndex(3));
     assert_eq!(instr_0.address, image_base + 0x00025f22);
     println!("{:?}", instr_0.kind);
     match instr_0.kind {
         HighLevelILInstructionKind::Ret(op) => {
-            assert_eq!(op.first_src, 4);
+            assert_eq!(op.first_src, 1);
             assert_eq!(op.num_srcs, 1);
         }
         _ => panic!("Expected Ret"),
@@ -61,3 +61,55 @@ fn test_hlil_var_info() {
         }
     }
 }
+
+#[test]
+fn test_hlil_ssa_variable_definition() {
+    let _session = Session::new().expect("Failed to initialize session");
+    let out_dir = env!("OUT_DIR").parse::<PathBuf>().unwrap();
+    let view = binaryninja::load(out_dir.join("atox.obj")).expect("Failed to create view");
+
+    let mut var_inits = 0;
+    for function in view.functions().iter() {
+        let hlil_function = function.high_level_il(false).unwrap();
+        let ssa_function = hlil_function.ssa_form();
+        for v in hlil_function.variables().iter() {
+            for ssa_var in hlil_function.ssa_variables(&v).iter() {
+                if ssa_var.version == 0 {
+                    continue;
+                }
+                assert!(hlil_function.ssa_variable_definition(ssa_var).is_some());
+                let def = ssa_function
+                    .ssa_variable_definition(ssa_var)
+                    .expect("Versioned SSA variable should have a definition");
+                if let HighLevelILInstructionKind::VarInitSsa(op) = def.kind {
+                    assert_eq!(op.dest, ssa_var);
+                    var_inits += 1;
+                }
+            }
+        }
+    }
+    assert!(var_inits > 0);
+}
+
+#[test]
+fn test_hlil_variable_live_at_uses() {
+    let _session = Session::new().expect("Failed to initialize session");
+    let out_dir = env!("OUT_DIR").parse::<PathBuf>().unwrap();
+    let view = binaryninja::load(out_dir.join("atox.obj")).expect("Failed to create view");
+
+    let mut uses = 0;
+    for function in view.functions().iter() {
+        let hlil_function = function.high_level_il(false).unwrap();
+        for v in hlil_function.variables().iter() {
+            // Only parameters can be live without a definition.
+            if hlil_function.variable_definitions(v).is_empty() {
+                continue;
+            }
+            for use_instr in hlil_function.variable_uses(v).iter() {
+                assert!(hlil_function.is_variable_live_at(v, &use_instr));
+                uses += 1;
+            }
+        }
+    }
+    assert!(uses > 0);
+}

rust/tests/medium_level_il.rs

@@ -62,22 +62,22 @@ fn test_mlil_info() {
     // 3 @ 00025f19  (MLIL_CALL eax_1 = (MLIL_CONST_PTR.d __crt_interlocked_read_32)((MLIL_VAR.d var_8)))
     let instr_3 = mlil_instr_iter.next().unwrap();
     assert_eq!(instr_3.instr_index, MediumLevelInstructionIndex(3));
-    assert_eq!(instr_3.expr_index, MediumLevelExpressionIndex(10));
+    assert_eq!(instr_3.expr_index, MediumLevelExpressionIndex(9));
     assert_eq!(instr_3.address, image_base + 0x00025f19);
     println!("{:?}", instr_3.kind);
     match instr_3.kind {
         MediumLevelILInstructionKind::Call(op) => {
-            assert_eq!(op.first_output, 8);
+            assert_eq!(op.first_output, 0);
             assert_eq!(op.num_outputs, 1);
-            assert_eq!(op.dest, MediumLevelExpressionIndex(7));
-            assert_eq!(op.first_param, 9);
+            assert_eq!(op.dest, MediumLevelExpressionIndex(8));
+            assert_eq!(op.first_param, 1);
             assert_eq!(op.num_params, 1);
         }
         _ => panic!("Expected Call"),
     }
     match instr_3.lift().kind {
         MediumLevelILLiftedInstructionKind::Call(lifted_call) => {
-            assert_eq!(lifted_call.dest.expr_index, MediumLevelExpressionIndex(7));
+            assert_eq!(lifted_call.dest.expr_index, MediumLevelExpressionIndex(8));
             assert_eq!(lifted_call.output.len(), 1);
             assert_eq!(lifted_call.params.len(), 1);
         }
@@ -86,12 +86,12 @@ fn test_mlil_info() {
     // 4 @ 00025f22  (MLIL_RET return (MLIL_VAR.d eax_1))
     let instr_4 = mlil_instr_iter.next().unwrap();
     assert_eq!(instr_4.instr_index, MediumLevelInstructionIndex(4));
-    assert_eq!(instr_4.expr_index, MediumLevelExpressionIndex(13));
+    assert_eq!(instr_4.expr_index, MediumLevelExpressionIndex(11));
     assert_eq!(instr_4.address, image_base + 0x00025f22);
     println!("{:?}", instr_4.kind);
     match instr_4.kind {
         MediumLevelILInstructionKind::Ret(op) => {
-            assert_eq!(op.first_operand, 12);
+            assert_eq!(op.first_operand, 2);
             assert_eq!(op.num_operands, 1);
         }
         _ => panic!("Expected Ret"),
@@ -193,3 +193,83 @@ fn test_mlil_possible_values() {
         _ => panic!("Expected Ret"),
     }
 }
+
+#[test]
+fn test_mlil_const_possible_values() {
+    let _session = Session::new().expect("Failed to initialize session");
+    let out_dir = env!("OUT_DIR").parse::<PathBuf>().unwrap();
+    let view = binaryninja::load(out_dir.join("atox.obj")).expect("Failed to create view");
+
+    // Only constants whose expression index differs from their instruction index can tell the
+    // two kinds of index apart.
+    let mut sub_expression_consts = 0;
+    for function in view.functions().iter() {
+        let mlil_function = function.medium_level_il().unwrap();
+        for expr_index in 0..mlil_function.expression_count() {
+            let expr = mlil_function
+                .instruction_from_expr_index(MediumLevelExpressionIndex(expr_index))
+                .unwrap();
+            let MediumLevelILInstructionKind::Const(op) = expr.kind else {
+                continue;
+            };
+            if expr.expr_index.0 == expr.instr_index.0 {
+                continue;
+            }
+            assert_eq!(
+                expr.possible_values(),
+                PossibleValueSet::ConstantValue {
+                    value: op.constant as i64
+                }
+            );
+            sub_expression_consts += 1;
+        }
+    }
+    assert!(sub_expression_consts > 0);
+}
+
+#[test]
+fn test_mlil_branch_dependencies() {
+    let _session = Session::new().expect("Failed to initialize session");
+    let out_dir = env!("OUT_DIR").parse::<PathBuf>().unwrap();
+    let view = binaryninja::load(out_dir.join("atox.obj")).expect("Failed to create view");
+
+    let mut dependencies = 0;
+    for function in view.functions().iter() {
+        let mlil_function = function.medium_level_il().unwrap();
+        for instr_index in 0..mlil_function.instruction_count() {
+            let instr = mlil_function
+                .instruction_from_index(MediumLevelInstructionIndex(instr_index))
+                .unwrap();
+            for dependence in instr.branch_dependencies().iter() {
+                assert!(matches!(
+                    dependence.instruction.kind,
+                    MediumLevelILInstructionKind::If(_)
+                ));
+                dependencies += 1;
+            }
+        }
+    }
+    assert!(dependencies > 0);
+}
+
+#[test]
+fn test_mlil_var_refs() {
+    let _session = Session::new().expect("Failed to initialize session");
+    let out_dir = env!("OUT_DIR").parse::<PathBuf>().unwrap();
+    let view = binaryninja::load(out_dir.join("atox.obj")).expect("Failed to create view");
+
+    let mut refs = 0;
+    for function in view.functions().iter() {
+        let mlil_function = function.medium_level_il().unwrap();
+        for var in mlil_function.variables().iter() {
+            for var_ref in mlil_function.var_refs(&var).iter() {
+                let instr = mlil_function
+                    .instruction_from_index(var_ref.instr_idx)
+                    .expect("Reference should name a valid instruction");
+                assert_eq!(instr.address, var_ref.addr);
+                refs += 1;
+            }
+        }
+    }
+    assert!(refs > 0);
+}