Move indirect flag into ValueLocation

Author: D0ntPanic

arch/x86/arch_x86.cpp

@@ -3869,12 +3869,9 @@ class X86SystemVCallingConvention: public X86BaseCallingConvention
 	{
 		if (!returnValue.has_value())
 			return 0;
-		for (auto& component: returnValue->components)
-		{
-			// Indirect return values have the pointer popped off the stack by the called function
-			if (component.indirect)
-				return 4;
-		}
+		// Indirect return values have the pointer popped off the stack by the called function
+		if (returnValue->indirect)
+			return 4;
 		return 0;
 	}
 };
@@ -4738,9 +4735,7 @@ class X64SystemVCallingConvention: public X64BaseCallingConvention
 		if (!components.has_value())
 		{
 			// Value doesn't work in a register, return through an indirect pointer
-			ValueLocationComponent indirect(
-				GetIndirectReturnValueLocation(), 0, type->GetWidth(), true, GetReturnedIndirectReturnValuePointer());
-			return ValueLocation({indirect});
+			return ValueLocation({GetIndirectReturnValueLocation()}, true, GetReturnedIndirectReturnValuePointer());
 		}
 
 		ValueLocation result;
@@ -4822,9 +4817,7 @@ class X64SystemVCallingConvention: public X64BaseCallingConvention
 			return result;
 
 		// Value doesn't work in a register, return through an indirect pointer
-		ValueLocationComponent indirect(
-			GetIndirectReturnValueLocation(), 0, type->GetWidth(), true, GetReturnedIndirectReturnValuePointer());
-		return ValueLocation({indirect});
+		return ValueLocation({GetIndirectReturnValueLocation()}, true, GetReturnedIndirectReturnValuePointer());
 	}
 
 	Variable GetIndirectReturnValueLocation() override { return Variable::Register(XED_REG_RDI); }
@@ -4845,15 +4838,12 @@ class X64SystemVCallingConvention: public X64BaseCallingConvention
 		size_t addrSize = GetArchitecture()->GetAddressSize();
 		int64_t stackOffset = addrSize;
 
-		if (returnValue.has_value())
+		if (returnValue.has_value() && returnValue->indirect)
 		{
 			// If the return value is stored as an indirect location parameter, ensure that the normal parameters
 			// don't overlap with it.
 			for (auto& component : returnValue->components)
 			{
-				if (!component.indirect)
-					continue;
-
 				if (component.variable.type == RegisterVariableSourceType)
 				{
 					if (intArgIter != intArgs.end() && *intArgIter == component.variable.storage)
@@ -4879,9 +4869,6 @@ class X64SystemVCallingConvention: public X64BaseCallingConvention
 				result.push_back(param.location);
 				for (auto& component : param.location.components)
 				{
-					if (component.indirect)
-						continue;
-
 					if (component.variable.type == RegisterVariableSourceType)
 					{
 						// If the non-default location matches the next register in the register parameter
@@ -4958,19 +4945,19 @@ class X64SystemVCallingConvention: public X64BaseCallingConvention
 						}
 					}
 
-					// Single component values shouldn't have the size set, otherwise heuristically determined locations
-					// will not match.
-					if (!indirect && location.components.size() == 1)
-						location.components[0].size.reset();
-
 					if (indirect)
 					{
+						location.indirect = true;
 						std::ranges::for_each(location.components, [&](auto& component) {
-							component.indirect = true;
 							component.size = param.type->GetWidth();
 						});
 					}
 
+					// Single component values shouldn't have the size set, otherwise heuristically determined locations
+					// will not match.
+					if (location.components.size() == 1)
+						location.components[0].size.reset();
+
 					if (valid)
 					{
 						// Value fit in registers

binaryninjaapi.h

@@ -10546,13 +10546,10 @@ namespace BinaryNinja {
 		Variable variable;
 		int64_t offset = 0;
 		std::optional<uint64_t> size;
-		bool indirect = false;
-		std::optional<Variable> returnedPointer;
 
 		ValueLocationComponent() = default;
-		ValueLocationComponent(Variable var, int64_t ofs = 0, std::optional<uint64_t> sz = std::nullopt,
-			bool indir = false, std::optional<Variable> retPtr = std::nullopt)
-			: variable(var), offset(ofs), size(sz), indirect(indir), returnedPointer(retPtr)
+		ValueLocationComponent(Variable var, int64_t ofs = 0, std::optional<uint64_t> sz = std::nullopt) :
+			variable(var), offset(ofs), size(sz)
 		{}
 
 		ValueLocationComponent RemapVariables(const std::function<Variable(Variable)>& remap) const;
@@ -10569,15 +10566,20 @@ namespace BinaryNinja {
 	struct ValueLocation
 	{
 		std::vector<ValueLocationComponent> components;
+		bool indirect = false;
+		std::optional<Variable> returnedPointer;
 
 		ValueLocation() {}
-		ValueLocation(Variable var) : components {var} {}
-		ValueLocation(const std::vector<ValueLocationComponent>& components) : components(components) {}
-		ValueLocation(std::vector<ValueLocationComponent>&& components) : components(std::move(components)) {}
-
-		ValueLocation(BNVariableSourceType type, uint64_t storage) : components {Variable(type, storage)} {}
-		ValueLocation(BNVariableSourceType type, uint32_t index, uint64_t storage) :
-			components {Variable(type, index, storage)}
+		ValueLocation(Variable var, bool indir = false, std::optional<Variable> retPtr = std::nullopt) :
+			components {var}, indirect(indir), returnedPointer(retPtr)
+		{}
+		ValueLocation(const std::vector<ValueLocationComponent>& components, bool indir = false,
+			std::optional<Variable> retPtr = std::nullopt) :
+			components(components), indirect(indir), returnedPointer(retPtr)
+		{}
+		ValueLocation(std::vector<ValueLocationComponent>&& components, bool indir = false,
+			std::optional<Variable> retPtr = std::nullopt) :
+			components(std::move(components)), indirect(indir), returnedPointer(retPtr)
 		{}
 
 		std::optional<Variable> GetVariableForReturnValue() const;

binaryninjacore.h

@@ -2611,15 +2611,15 @@ extern "C"
 		int64_t offset;
 		bool sizeValid;
 		uint64_t size;
-		bool indirect;
-		bool returnedPointerValid;
-		BNVariable returnedPointer;
 	} BNValueLocationComponent;
 
 	typedef struct BNValueLocation
 	{
 		size_t count;
 		BNValueLocationComponent* components;
+		bool indirect;
+		bool returnedPointerValid;
+		BNVariable returnedPointer;
 	} BNValueLocation;
 
 	typedef struct BNValueLocationWithConfidence

plugins/pdb-ng/src/type_parser.rs

@@ -2423,11 +2423,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
         let default_return_location = convention
             .contents
             .return_value_location(self.bv, return_value.clone());
-        let default_return_indrect = default_return_location
-            .components
-            .iter()
-            .any(|c| c.indirect);
-        if default_return_indrect {
+        if default_return_location.indirect {
             None
         } else {
             let variable = if let Some(reg) = convention.contents.int_arg_registers().get(0) {
@@ -2449,13 +2445,15 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
                         variable,
                         offset: 0,
                         size: Some(return_value.ty.contents.width()),
-                        indirect: true,
-                        returned_pointer: convention.contents.return_int_reg().map(|reg| Variable::new(
+                    }],
+                    indirect: true,
+                    returned_pointer: convention.contents.return_int_reg().map(|reg| {
+                        Variable::new(
                             VariableSourceType::RegisterVariableSourceType,
                             0,
                             reg.0 as i64,
-                        )),
-                    }],
+                        )
+                    }),
                 },
                 MAX_CONFIDENCE,
             ))

python/types.py

@@ -443,8 +443,6 @@ class ValueLocationComponent:
 	var: 'variable.CoreVariable'
 	offset: int = 0
 	size: Optional[int] = None
-	indirect: bool = False
-	returned_pointer: Optional['variable.CoreVariable'] = None
 
 	@staticmethod
 	def _from_core_struct(struct: core.BNValueLocationComponent, arch: Optional['architecture.Architecture'] = None):
@@ -456,14 +454,7 @@ def _from_core_struct(struct: core.BNValueLocationComponent, arch: Optional['arc
 		size = None
 		if struct.sizeValid:
 			size = struct.size
-		indirect = struct.indirect
-		returned_pointer = None
-		if struct.returnedPointerValid:
-			if arch is None:
-				returned_pointer = variable.CoreVariable.from_BNVariable(struct.returnedPointer)
-			else:
-				returned_pointer = variable.ArchitectureVariable.from_BNVariable(arch, struct.returnedPointer)
-		return ValueLocationComponent(var, offset, size, indirect, returned_pointer)
+		return ValueLocationComponent(var, offset, size)
 
 	def _to_core_struct(self) -> core.BNValueLocationComponent:
 		struct = core.BNValueLocationComponent()
@@ -472,10 +463,6 @@ def _to_core_struct(self) -> core.BNValueLocationComponent:
 		struct.sizeValid = self.size is not None
 		if self.size is not None:
 			struct.size = self.size
-		struct.indirect = self.indirect
-		struct.returnedPointerValid = self.returned_pointer is not None
-		if self.returned_pointer is not None:
-			struct.returnedPointer = self.returned_pointer.to_BNVariable()
 		return struct
 
 	def to_string(self, arch: Optional['architecture.Architecture']):
@@ -507,13 +494,22 @@ def __repr__(self):
 @dataclass
 class ValueLocation:
 	components: List['ValueLocationComponent']
+	indirect: bool = False
+	returned_pointer: Optional['variable.CoreVariable'] = None
 
 	@staticmethod
 	def _from_core_struct(struct: core.BNValueLocation, arch: Optional['architecture.Architecture'] = None):
 		components = []
 		for i in range(struct.count):
 			components.append(ValueLocationComponent._from_core_struct(struct.components[i], arch))
-		return ValueLocation(components)
+		indirect = struct.indirect
+		returned_pointer = None
+		if struct.returnedPointerValid:
+			if arch is None:
+				returned_pointer = variable.CoreVariable.from_BNVariable(struct.returnedPointer)
+			else:
+				returned_pointer = variable.ArchitectureVariable.from_BNVariable(arch, struct.returnedPointer)
+		return ValueLocation(components, indirect, returned_pointer)
 
 	def _to_core_struct(self) -> core.BNValueLocation:
 		struct = core.BNValueLocation()
@@ -522,6 +518,10 @@ def _to_core_struct(self) -> core.BNValueLocation:
 		for i in range(len(self.components)):
 			components[i] = self.components[i]._to_core_struct()
 		struct.components = components
+		struct.indirect = self.indirect
+		struct.returnedPointerValid = self.returned_pointer is not None
+		if self.returned_pointer is not None:
+			struct.returnedPointer = self.returned_pointer.to_BNVariable()
 		return struct
 
 	def with_confidence(self, confidence: int) -> 'ValueLocationWithConfidence':

rust/src/types.rs

@@ -1173,8 +1173,6 @@ pub struct ValueLocationComponent {
     pub variable: Variable,
     pub offset: i64,
     pub size: Option<u64>,
-    pub indirect: bool,
-    pub returned_pointer: Option<Variable>,
 }
 
 impl ValueLocationComponent {
@@ -1189,12 +1187,6 @@ impl ValueLocationComponent {
             variable,
             offset: value.offset,
             size,
-            indirect: value.indirect,
-            returned_pointer: if value.returnedPointerValid {
-                Some(Variable::from(&value.returnedPointer))
-            } else {
-                None
-            },
         }
     }
 
@@ -1204,20 +1196,15 @@ impl ValueLocationComponent {
             offset: value.offset,
             sizeValid: value.size.is_some(),
             size: value.size.unwrap_or(0),
-            indirect: value.indirect,
-            returnedPointerValid: value.returned_pointer.is_some(),
-            returnedPointer: if let Some(ptr) = value.returned_pointer {
-                ptr.into()
-            } else {
-                Variable::new(VariableSourceType::RegisterVariableSourceType, 0, 0).into()
-            },
         }
     }
 }
 
 #[derive(Debug, Clone, Hash, PartialEq, Eq)]
 pub struct ValueLocation {
     pub components: Vec<ValueLocationComponent>,
+    pub indirect: bool,
+    pub returned_pointer: Option<Variable>,
 }
 
 impl ValueLocation {
@@ -1227,9 +1214,9 @@ impl ValueLocation {
                 variable: var,
                 offset: 0,
                 size: None,
-                indirect: false,
-                returned_pointer: None,
             }],
+            indirect: false,
+            returned_pointer: None,
         }
     }
 
@@ -1274,20 +1261,20 @@ impl ValueLocation {
         }
     }
 
-    pub(crate) fn from_raw(components: &BNValueLocation) -> Self {
-        if components.count == 0 {
-            return Self {
-                components: Vec::new(),
-            };
-        }
-
+    pub(crate) fn from_raw(loc: &BNValueLocation) -> Self {
         let components_raw: &[BNValueLocationComponent] =
-            unsafe { std::slice::from_raw_parts(components.components, components.count) };
+            unsafe { crate::ffi::slice_from_raw_parts(loc.components, loc.count) };
         Self {
             components: components_raw
                 .iter()
                 .map(|component| ValueLocationComponent::from_raw(component))
                 .collect(),
+            indirect: loc.indirect,
+            returned_pointer: if loc.returnedPointerValid {
+                Some(Variable::from(&loc.returnedPointer))
+            } else {
+                None
+            },
         }
     }
 
@@ -1300,6 +1287,13 @@ impl ValueLocation {
         BNValueLocation {
             count: components.len(),
             components: Box::leak(components).as_mut_ptr(),
+            indirect: value.indirect,
+            returnedPointerValid: value.returned_pointer.is_some(),
+            returnedPointer: if let Some(ptr) = value.returned_pointer {
+                ptr.into()
+            } else {
+                Variable::new(VariableSourceType::RegisterVariableSourceType, 0, 0).into()
+            },
         }
     }
 
@@ -1318,9 +1312,9 @@ impl Into<ValueLocation> for Variable {
                 variable: self,
                 offset: 0,
                 size: None,
-                indirect: false,
-                returned_pointer: None,
             }],
+            indirect: false,
+            returned_pointer: None,
         }
     }
 }
@@ -1367,6 +1361,8 @@ impl ReturnValue {
                     .map(|v| &v.contents)
                     .unwrap_or(&ValueLocation {
                         components: Vec::new(),
+                        indirect: false,
+                        returned_pointer: None,
                     }),
             ),
             locationConfidence: value.location.as_ref().map(|v| v.confidence).unwrap_or(0),
@@ -1476,6 +1472,8 @@ impl FunctionParameter {
             defaultLocation: value.location.is_none(),
             location: ValueLocation::into_rust_raw(&value.location.unwrap_or(ValueLocation {
                 components: Vec::new(),
+                indirect: false,
+                returned_pointer: None,
             })),
         }
     }