Merge remote-tracking branch

'origin/GP-6776_ghidracadabra_PR-9101_vigneshakaviki_fix_varargs-filter-firstVarArgSlot'
(Closes #9101, Closes #9091)

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/stack/SymPcodeExecutor.java

@@ -327,7 +327,8 @@ public class SymPcodeExecutor extends PcodeExecutor<Sym> {
 		for (int i = 0; i < arguments.length; i++) {
 			types[i + 1] = arguments[0].getDataType();
 		}
-		VariableStorage[] vsLocs = convention.getStorageLocations(program, types, false);
+		VariableStorage[] vsLocs =
+			convention.getStorageLocations(program, types, false, sig.hasVarArgs());
 		Address min = null;
 		Address max = null; // Exclusive
 		for (VariableStorage vs : vsLocs) {

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionData.java

@@ -193,10 +193,11 @@ class FunctionData extends FunctionDataView {
 
 	void setVarArgs(boolean enable) {
 		this.hasVarArgs = enable;
+		updateParameterAndReturnStorage();
 	}
 
 	/**
-	 * Update dynamic storage and auto-params when custom storage is disasbled.
+	 * Update dynamic storage and auto-params when custom storage is disabled.
 	 * Returns immediately if custom storage is enabled.
 	 */
 	void updateParameterAndReturnStorage() {
@@ -222,7 +223,8 @@ class FunctionData extends FunctionDataView {
 		}
 
 		VariableStorage[] paramStorage =
-			effectiveCallingConvention.getStorageLocations(getProgram(), dataTypes, true);
+			effectiveCallingConvention.getStorageLocations(getProgram(), dataTypes, true,
+				hasVarArgs);
 
 		returnInfo.setStorage(paramStorage[0]);
 

Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/util/FillOutStructureHelper.java

@@ -4,9 +4,9 @@
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *      http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -234,7 +234,7 @@ public class FillOutStructureHelper {
 			typeList[i] = inputs[i].getHigh().getDataType();
 		}
 		VariableStorage[] storageLocations =
-			model.getStorageLocations(currentProgram, typeList, false);
+			model.getStorageLocations(currentProgram, typeList, false, function.hasVarArgs());
 		return storageLocations[slot].getMinAddress();
 	}
 

Ghidra/Features/SystemEmulation/src/main/java/ghidra/pcode/emu/sys/UseropEmuSyscallDefinition.java

@@ -86,7 +86,7 @@ public class UseropEmuSyscallDefinition<T> implements EmuSyscallDefinition<T> {
 		for (int i = 0; i < locs.length; i++) {
 			locs[i] = dtMachineWord;
 		}
-		VariableStorage[] vss = convention.getStorageLocations(program, locs, false);
+		VariableStorage[] vss = convention.getStorageLocations(program, locs, false, false);
 
 		outVar = getSingleVnStorage(vss[0]);
 		inVars = Arrays.asList(new Varnode[inputCount]);

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/function/FunctionManagerDB.java

@@ -615,12 +615,14 @@ public class FunctionManagerDB implements FunctionManager {
 	 * @param returnDataType
 	 * @param currentParams
 	 * @param paramOffset offset within currentParams for first parameter
+	 * @param hasVarArgs if true, any storage rules specific to varargs function will be applied
 	 * @return return variable storage if dynamic storage does not match current custom storage -
 	 * this is done so that this storage can be assigned if currently <UNASSIGNED>.  If dynamic
 	 * storage matches null will be returned.
 	 */
 	private VariableStorage checkDynamicStorageConversion(DataType returnDataType,
-			Parameter[] currentParams, int paramOffset, PrototypeModel callingConvention) {
+			Parameter[] currentParams, int paramOffset, PrototypeModel callingConvention,
+			boolean hasVarArgs) {
 		DataType types[] = new DataType[currentParams.length - paramOffset + 1];
 		types[0] = returnDataType;
 		int index = 1;
@@ -629,7 +631,7 @@ public class FunctionManagerDB implements FunctionManager {
 		}
 
 		VariableStorage[] paramStorage =
-			callingConvention.getStorageLocations(program, types, true);
+			callingConvention.getStorageLocations(program, types, true, hasVarArgs);
 // TODO: Only handles a single auto-param insertion (could be more auto-params)
 		index = (paramStorage.length == types.length) ? 1 : 2; // May have inserted extra parameter
 		if ((paramStorage.length - 1) != types.length) {
@@ -681,7 +683,8 @@ public class FunctionManagerDB implements FunctionManager {
 			if (CompilerSpec.CALLING_CONVENTION_thiscall.equals(func.getCallingConventionName())) {
 				if (params.length != 0 && isLikelyThisParam(params[0])) {
 					returnStorage =
-						checkDynamicStorageConversion(returnDataType, params, 1, callingConvention);
+						checkDynamicStorageConversion(returnDataType, params, 1, callingConvention,
+							func.hasVarArgs());
 					if (returnStorage == null) {
 						useDynamic = true;
 						func.removeVariable(params[0]);
@@ -690,7 +693,8 @@ public class FunctionManagerDB implements FunctionManager {
 			}
 			else {
 				returnStorage =
-					checkDynamicStorageConversion(returnDataType, params, 0, callingConvention);
+					checkDynamicStorageConversion(returnDataType, params, 0, callingConvention,
+						func.hasVarArgs());
 				useDynamic = (returnStorage == null);
 			}
 

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/function/FunctionVariables.java

@@ -141,7 +141,8 @@ class FunctionVariables {
 		}
 
 		VariableStorage[] variableStorage =
-			callingConvention.getStorageLocations(function.getProgram(), dataTypes, true);
+			callingConvention.getStorageLocations(function.getProgram(), dataTypes, true,
+				function.hasVarArgs());
 		returnParam.setDynamicStorage(variableStorage[0]);
 
 		int autoIndex = 0;

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/lang/PrototypeModel.java

@@ -248,6 +248,7 @@ public class PrototypeModel {
 	 * @return return location or {@link VariableStorage#UNASSIGNED_STORAGE} if
 	 * unable to determine suitable location
 	 */
+	@Deprecated
 	public VariableStorage getReturnLocation(DataType dataType, Program program) {
 		DataType clone = dataType.clone(program.getDataTypeManager());
 		PrototypePieces proto = new PrototypePieces(this, clone);
@@ -270,7 +271,7 @@ public class PrototypeModel {
 	 * Warning: The use of this method with a null {@code params} argument, or incorrect
 	 * datatypes, is highly discouraged since it will produce inaccurate results.
 	 * It is recommended that a complete function signature be used in
-	 * conjunction with the {@link #getStorageLocations(Program, DataType[], boolean)}
+	 * conjunction with the {@link #getStorageLocations(Program, DataType[], boolean, boolean)}
 	 * method.  Parameter storage allocation may be affected by the return datatype
 	 * specified (e.g., hidden return storage parameter).
 	 *  
@@ -282,7 +283,10 @@ public class PrototypeModel {
 	 * @param program is the Program
 	 * @return next parameter location or {@link VariableStorage#UNASSIGNED_STORAGE} if
 	 * unable to determine suitable location
+	 * @deprecated This method does not apply any storage rules specific to varags functions.
+	 * Use {@link #getStorageLocations(Program,DataType[],boolean,boolean)} instead.
 	 */
+	@Deprecated
 	public VariableStorage getNextArgLocation(Parameter[] params, DataType dataType,
 			Program program) {
 		return getArgLocation(params != null ? params.length : 0, params, dataType, program);
@@ -315,10 +319,12 @@ public class PrototypeModel {
 	 * @param program is the Program
 	 * @return parameter location or {@link VariableStorage#UNASSIGNED_STORAGE} if
 	 * unable to determine suitable location
+	 * @deprecated This method does not apply any storage rules specific to varags functions. 
+	 * Use {@link #getStorageLocations(Program,DataType[],boolean,boolean)} instead.
 	 */
+	@Deprecated
 	public VariableStorage getArgLocation(int argIndex, Parameter[] params, DataType dataType,
 			Program program) {
-
 		if (dataType != null) {
 			dataType = dataType.clone(program.getDataTypeManager());
 			// Identify next arg index based upon number of storage varnodes 
@@ -338,7 +344,7 @@ public class PrototypeModel {
 		}
 		arr[argIndex + 1] = dataType;
 
-		VariableStorage res[] = getStorageLocations(program, arr, false);
+		VariableStorage res[] = getStorageLocations(program, arr, false, false);
 		return res[res.length - 1];
 	}
 
@@ -378,12 +384,13 @@ public class PrototypeModel {
 	}
 
 	/**
-	 * Compute the variable storage for a given array of return/parameter datatypes.  The first array element
-	 * is the return datatype, which is followed by any input parameter datatypes in order.
-	 * If addAutoParams is true, pointer datatypes will automatically be inserted for "this" or "hidden return"
-	 * input parameters, if needed.  In this case, the dataTypes array should not include explicit entries for
-	 * these parameters.  If addAutoParams is false, the dataTypes array is assumed to already contain explicit
-	 * entries for any of these parameters.
+	 * Compute the variable storage for a given array of return/parameter datatypes.  
+	 * The first array element is the return datatype, which is followed by any input parameter 
+	 * datatypes in order. If addAutoParams is true, pointer datatypes will automatically be 
+	 * inserted for "this" or "hidden return"input parameters, if needed.  In this case, the 
+	 * dataTypes array should not include explicit entries for these parameters.  If addAutoParams 
+	 * is false, the dataTypes array is assumed to already contain explicit entries for any of 
+	 * these parameters.
 	 * <br>
 	 * Note: storage will not be assigned to the {@link DataType#DEFAULT default undefined} datatype
 	 * or zero-length datatypes or any subsequent parameter following such a parameter.
@@ -395,9 +402,40 @@ public class PrototypeModel {
 	 * @return dynamic storage locations orders by ordinal where first element corresponds to
 	 * return storage. The returned array may also include additional auto-parameter storage 
 	 * locations. 
+	 * @deprecated This method does not apply any storage rules specific to varags functions.
+	 * Use {@link #getStorageLocations(Program,DataType[],boolean,boolean)} instead.
 	 */
+	@Deprecated
 	public VariableStorage[] getStorageLocations(Program program, DataType[] dataTypes,
 			boolean addAutoParams) {
+		return getStorageLocations(program, dataTypes, addAutoParams, false);
+	}
+
+	/**
+	 * Compute the variable storage for a given array of return/parameter datatypes.  
+	 * The first array element is the return datatype, which is followed by any input parameter 
+	 * datatypes in order. If addAutoParams is true, pointer datatypes will automatically be 
+	 * inserted for "this" or "hidden return"input parameters, if needed.  In this case, the 
+	 * dataTypes array should not include explicit entries for these parameters.  If addAutoParams 
+	 * is false, the dataTypes array is assumed to already contain explicit entries for any of 
+	 * these parameters.  
+	 * <br>
+	 * Note: storage will not be assigned to the {@link DataType#DEFAULT default undefined} datatype
+	 * or zero-length datatypes or any subsequent parameter following such a parameter.
+	 * 
+	 * @param program is the Program
+	 * @param dataTypes return/parameter datatypes (first element is always the return datatype, 
+	 * i.e., minimum array length is 1)
+	 * @param addAutoParams true if auto-parameter storage locations can be generated
+	 * @param isVarArgs If true, the last input parameter will be treated as the last fixed argument
+	 * of a varargs function (via {@link PrototypePieces#firstVarArgSlot}).
+	 * @return dynamic storage locations orders by ordinal where first element corresponds to
+	 * return storage. The returned array may also include additional auto-parameter storage 
+	 * locations. 
+	 * 
+	 */
+	public VariableStorage[] getStorageLocations(Program program, DataType[] dataTypes,
+			boolean addAutoParams, boolean isVarArgs) {
 
 		DataType injectedThis = null;
 		if (addAutoParams && hasThis) {
@@ -406,6 +444,10 @@ public class PrototypeModel {
 			injectedThis = new PointerDataType(program.getDataTypeManager());
 		}
 		PrototypePieces proto = new PrototypePieces(this, dataTypes, injectedThis);
+		if (isVarArgs) {
+			// All supplied parameters are non-optional; varargs begin immediately after them.
+			proto.firstVarArgSlot = proto.intypes.size();
+		}
 
 		ArrayList<ParameterPieces> res = new ArrayList<>();
 		assignParameterStorage(proto, program.getDataTypeManager(), res, addAutoParams);

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/VariableUtilities.java

@@ -4,9 +4,9 @@
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *      http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -752,7 +752,8 @@ public class VariableUtilities {
 			arr[0] = DataType.VOID;
 			arr[1] = dt;
 			VariableStorage thisStorage =
-				convention.getStorageLocations(function.getProgram(), arr, true)[1];
+				convention.getStorageLocations(function.getProgram(), arr, true,
+					function.hasVarArgs())[1];
 			try {
 				return new ParameterImpl("this", 0, dt, thisStorage, false, function.getProgram(),
 					SourceType.ANALYSIS);

Ghidra/Processors/TI_MSP430/data/languages/TI_MSP430.cspec

@@ -52,6 +52,10 @@
           <datatype name="struct" minsize="1"/>
           <convert_to_ptr/>
         </rule>
+        <rule>
+          <datatype name="union" minsize="1"/>
+          <convert_to_ptr/>
+        </rule>
         <rule>
           <datatype name="any" maxsize="4"/>
           <join stackspill="true"/>		
@@ -78,6 +82,10 @@
           <datatype name="struct"/>
           <hidden_return/>
         </rule>
+        <rule>
+          <datatype name="union"/>
+          <hidden_return/>
+        </rule>
         <rule>
           <datatype name="any" maxsize="8"/>
           <join/>

Ghidra/Processors/TI_MSP430/src/test.processors/java/ghidra/test/processors/cspec/MSP430_CSpecTest.java

@@ -25,37 +25,6 @@ public class MSP430_CSpecTest extends CSpecPrototypeTest {
 
 	private static final String[] EXPECTED_PROTOTYPE_ERRORS = {
 		"paramsVariadic_I_I_C_C_S_S_I_I_L_L_50",
-		"paramsUnion_56",
-		"paramsUnion_57",
-		"paramsUnion_58",
-		"paramsUnion_59",
-		"paramsUnion_60",
-		"paramsUnion_61",
-		"paramsUnion_62",
-		"paramsUnion_63",
-		"paramsUnion_64",
-		"paramsUnion_65",
-		"paramsUnion_66",
-		"paramsUnion_67",
-		"paramsUnion_68",
-		"paramsUnion_69",
-		"paramsUnion_70",
-		"paramsUnion_71",
-		"paramsUnion_72",
-		"paramsUnion_73",
-		"paramsUnion_74",
-		"paramsUnion_75",
-		"paramsUnion_76",
-		"returnUnion_82",
-		"returnUnion_83",
-		"returnUnion_89",
-		"returnUnion_90",
-		"returnUnion_96",
-		"returnUnion_102",
-		"returnUnion_104",
-		"returnUnion_112",
-		"returnUnion_121",
-		"returnUnion_130",
 	};
 
 	public MSP430_CSpecTest() throws Exception {

Ghidra/Test/IntegrationTest/src/test.slow/java/ghidra/program/model/lang/VarargsStorageTest.java

@@ -0,0 +1,139 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package ghidra.program.model.lang;
+
+import static org.junit.Assert.*;
+
+import org.junit.Test;
+
+import generic.test.AbstractGenericTest;
+import ghidra.app.plugin.processors.sleigh.SleighLanguageProvider;
+import ghidra.program.database.ProgramBuilder;
+import ghidra.program.model.data.*;
+import ghidra.program.model.listing.Program;
+import ghidra.program.model.listing.VariableStorage;
+
+public class VarargsStorageTest extends AbstractGenericTest {
+
+	private Program program;
+
+	@Test
+	public void test_MSP430_stdcall() throws Exception {
+
+		Language language = SleighLanguageProvider.getSleighLanguageProvider()
+				.getLanguage(new LanguageID("TI_MSP430:LE:16:default"));
+		ProgramBuilder builder = new ProgramBuilder("test", language);
+
+		program = builder.getProgram();
+		PrototypeModel model = program.getCompilerSpec().getCallingConvention("__stdcall");
+		DataType intType = new UnsignedIntegerDataType(program.getDataTypeManager());
+		DataType voidType = new VoidDataType(program.getDataTypeManager());
+		assertNotNull(model);
+		DataType[] inputs = new DataType[4];
+		inputs[0] = voidType;
+		inputs[1] = intType;
+		inputs[2] = intType;
+		inputs[3] = intType;
+		VariableStorage[] storage = model.getStorageLocations(program, inputs, false, false);
+		assertEquals(1, storage[1].getVarnodeCount());
+		assertEquals("R12", storage[1].getFirstVarnode().toString(language));
+		assertEquals(1, storage[2].getVarnodeCount());
+		assertEquals("R13", storage[2].getFirstVarnode().toString(language));
+		assertEquals(1, storage[3].getVarnodeCount());
+		assertEquals("R14", storage[3].getFirstVarnode().toString(language));
+
+		// TI_MSP430 __stdcall passes the last fixed argument of a variadic function
+		// on the stack, so storage[3] should be a stack location instead of R14
+		storage = model.getStorageLocations(program, inputs, false, true);
+		assertEquals(1, storage[1].getVarnodeCount());
+		assertEquals("R12", storage[1].getFirstVarnode().toString(language));
+		assertEquals(1, storage[2].getVarnodeCount());
+		assertEquals("R13", storage[2].getFirstVarnode().toString(language));
+		assertEquals(1, storage[3].getVarnodeCount());
+		assertTrue(storage[3].isStackStorage());
+	}
+
+	@Test
+	public void test_avr8__stdcall() throws Exception {
+
+		Language language = SleighLanguageProvider.getSleighLanguageProvider()
+				.getLanguage(new LanguageID("avr8:LE:16:default"));
+		ProgramBuilder builder = new ProgramBuilder("test", language);
+
+		program = builder.getProgram();
+		PrototypeModel model = program.getCompilerSpec().getCallingConvention("__stdcall");
+		DataType intType = new UnsignedIntegerDataType(program.getDataTypeManager());
+		DataType voidType = new VoidDataType(program.getDataTypeManager());
+		assertNotNull(model);
+		DataType[] inputs = new DataType[4];
+		inputs[0] = voidType;
+		inputs[1] = intType;
+		inputs[2] = intType;
+		inputs[3] = intType;
+		VariableStorage[] storage = model.getStorageLocations(program, inputs, false, false);
+		assertEquals(1, storage[1].getVarnodeCount());
+		assertEquals("R25R24", storage[1].getFirstVarnode().toString(language));
+		assertEquals(1, storage[2].getVarnodeCount());
+		assertEquals("R23R22", storage[2].getFirstVarnode().toString(language));
+		assertEquals(1, storage[3].getVarnodeCount());
+		assertEquals("R21R20", storage[3].getFirstVarnode().toString(language));
+
+		// avr8 __stdcall passes all parameters of a variadic function on the stack
+		storage = model.getStorageLocations(program, inputs, false, true);
+		assertEquals(1, storage[1].getVarnodeCount());
+		assertTrue(storage[1].isStackStorage());
+		assertEquals(1, storage[2].getVarnodeCount());
+		assertTrue(storage[2].isStackStorage());
+		assertEquals(1, storage[3].getVarnodeCount());
+		assertTrue(storage[3].isStackStorage());
+	}
+
+	@Test
+	public void test_x64__stdcall() throws Exception {
+
+		Language language = SleighLanguageProvider.getSleighLanguageProvider()
+				.getLanguage(new LanguageID("x86:LE:64:default"));
+		ProgramBuilder builder = new ProgramBuilder("test", "x86:LE:64:default", "gcc", null);
+
+		program = builder.getProgram();
+		PrototypeModel model = program.getCompilerSpec().getCallingConvention("__stdcall");
+		DataType longType = new UnsignedLongDataType(program.getDataTypeManager());
+		DataType voidType = new VoidDataType(program.getDataTypeManager());
+		assertNotNull(model);
+		DataType[] inputs = new DataType[4];
+		inputs[0] = voidType;
+		inputs[1] = longType;
+		inputs[2] = longType;
+		inputs[3] = longType;
+		VariableStorage[] storage = model.getStorageLocations(program, inputs, false, false);
+		assertEquals(1, storage[1].getVarnodeCount());
+		assertEquals("RDI", storage[1].getFirstVarnode().toString(language));
+		assertEquals(1, storage[2].getVarnodeCount());
+		assertEquals("RSI", storage[2].getFirstVarnode().toString(language));
+		assertEquals(1, storage[3].getVarnodeCount());
+		assertEquals("RDX", storage[3].getFirstVarnode().toString(language));
+
+		// x64 gcc __stdcall does not assign parameters differently for variadic functions
+		storage = model.getStorageLocations(program, inputs, false, true);
+		assertEquals(1, storage[1].getVarnodeCount());
+		assertEquals("RDI", storage[1].getFirstVarnode().toString(language));
+		assertEquals(1, storage[2].getVarnodeCount());
+		assertEquals("RSI", storage[2].getFirstVarnode().toString(language));
+		assertEquals(1, storage[3].getVarnodeCount());
+		assertEquals("RDX", storage[3].getFirstVarnode().toString(language));
+	}
+
+}