Skip to content

Commit 5904b7d

Browse files
ghidra1ghidra1
committed
GT-3214 corrected function parameter ordinal numbering when more than
one auto-parameter is present
1 parent 0077735 commit 5904b7d

File tree

2 files changed

+106
-31
lines changed
  • Ghidra
    • Features/Base/src/test.slow/java/ghidra/program/database/function
    • Framework/SoftwareModeling/src/main/java/ghidra/program/database/function

2 files changed

+106
-31
lines changed

‎Ghidra/Features/Base/src/test.slow/java/ghidra/program/database/function/FunctionDBTest.java

Lines changed: 105 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@
4545
/**
4646
*
4747
*/
48-
public class FunctionDBTest extends AbstractGhidraHeadedIntegrationTest implements DomainObjectListener {
48+
public class FunctionDBTest extends AbstractGhidraHeadedIntegrationTest
49+
implements DomainObjectListener {
4950

5051
private ProgramDB program;
5152
private AddressSpace space;
@@ -609,7 +610,7 @@ private Function createTestFunction() throws Exception {
609610
return f;
610611
}
611612

612-
private void assertParameters(Parameter p1, Parameter p2, boolean checkStorage) {
613+
private void assertParameter(Parameter p1, Parameter p2, boolean checkStorage) {
613614
assertEquals(p1.getName(), p2.getName());
614615
if (!p1.getDataType().isEquivalent(p2.getDataType())) {
615616
Assert.fail("Expected " + p1.getDataType().getName() + " but parameter has type " +
@@ -816,15 +817,15 @@ public void testUpdateFunctionCustomStorage() throws Exception {
816817
assertEquals("__stdcall", f.getCallingConventionName());
817818

818819
Parameter return1 = f.getReturn();
819-
assertParameters(returnVar, return1, true);
820+
assertParameter(returnVar, return1, true);
820821
assertEquals("r6:4", return1.getVariableStorage().toString());
821822

822823
Parameter[] parameters = f.getParameters();
823824
assertEquals(3, parameters.length);
824825

825-
assertParameters(p1, parameters[0], true);
826-
assertParameters(p2, parameters[1], true);
827-
assertParameters(p3, parameters[2], true);
826+
assertParameter(p1, parameters[0], true);
827+
assertParameter(p2, parameters[1], true);
828+
assertParameter(p3, parameters[2], true);
828829

829830
}
830831

@@ -835,38 +836,104 @@ public void testUpdateFunctionDynamicStorage() throws Exception {
835836

836837
Structure bigStruct = new StructureDataType("bigStruct", 20);
837838

839+
ReturnParameterImpl returnVar =
840+
new ReturnParameterImpl(bigStruct, VariableStorage.UNASSIGNED_STORAGE, program);
841+
842+
ParameterImpl p1 =
843+
new ParameterImpl(Function.RETURN_PTR_PARAM_NAME, new PointerDataType(bigStruct),
844+
new VariableStorage(program, program.getRegister("r7")), program);
845+
Structure classStruct = VariableUtilities.findOrCreateClassStruct(f);
846+
ParameterImpl p2 =
847+
new ParameterImpl(Function.THIS_PARAM_NAME, new PointerDataType(classStruct),
848+
new VariableStorage(program, program.getRegister("r8")), program);
849+
ParameterImpl p3 = new ParameterImpl("m2", LongLongDataType.dataType,
850+
new VariableStorage(program, program.getRegister("r12"), program.getRegister("r11")),
851+
program);
852+
ParameterImpl p4 = new ParameterImpl("m3", ByteDataType.dataType,
853+
new VariableStorage(program, program.getRegister("r9")), program);
854+
855+
// function updated with formal signature
856+
f.updateFunction("__thiscall", returnVar, FunctionUpdateType.DYNAMIC_STORAGE_ALL_PARAMS,
857+
true, SourceType.USER_DEFINED, p3, p4);
858+
859+
assertTrue(!f.hasCustomVariableStorage());
860+
assertEquals("__thiscall", f.getCallingConventionName());
861+
862+
Parameter return1 = f.getReturn();
863+
assertTrue(return1.isForcedIndirect());
864+
assertTrue(bigStruct.isEquivalent(return1.getFormalDataType()));
865+
assertTrue(returnVar.getDataType().isEquivalent(returnVar.getDataType()));
866+
assertEquals("r12:4 (ptr)", return1.getVariableStorage().toString());
867+
868+
Parameter[] parameters = f.getParameters();
869+
assertEquals(4, parameters.length);
870+
871+
assertParameter(p1, parameters[0], false);
872+
assertEquals(0, parameters[0].getOrdinal());
873+
assertEquals("r12:4 (auto)", parameters[0].getVariableStorage().toString());
874+
assertParameter(p2, parameters[1], false);
875+
assertEquals(1, parameters[1].getOrdinal());
876+
assertEquals("r11:4 (auto)", parameters[1].getVariableStorage().toString());
877+
assertParameter(p3, parameters[2], false);
878+
assertEquals(2, parameters[2].getOrdinal());
879+
assertEquals("Stack[0x0]:8", parameters[2].getVariableStorage().toString());
880+
assertParameter(p4, parameters[3], false);
881+
assertEquals(3, parameters[3].getOrdinal());
882+
assertEquals("r10:1", parameters[3].getVariableStorage().toString());
883+
884+
}
885+
886+
@Test
887+
public void testUpdateFunctionDynamicStorage1() throws Exception {
888+
889+
Function f = createTestFunction();
890+
891+
Structure bigStruct = new StructureDataType("bigStruct", 20);
892+
838893
ReturnParameterImpl returnVar = new ReturnParameterImpl(new PointerDataType(bigStruct),
839894
VariableStorage.UNASSIGNED_STORAGE, program);
840895

841896
ParameterImpl p1 =
842897
new ParameterImpl(Function.RETURN_PTR_PARAM_NAME, new PointerDataType(bigStruct),
843898
new VariableStorage(program, program.getRegister("r7")), program);
844-
ParameterImpl p2 = new ParameterImpl("m2", LongLongDataType.dataType,
899+
Structure classStruct = VariableUtilities.findOrCreateClassStruct(f);
900+
ParameterImpl p2 =
901+
new ParameterImpl(Function.THIS_PARAM_NAME, new PointerDataType(classStruct),
902+
new VariableStorage(program, program.getRegister("r8")), program);
903+
ParameterImpl p3 = new ParameterImpl("m2", LongLongDataType.dataType,
845904
new VariableStorage(program, program.getRegister("r12"), program.getRegister("r11")),
846905
program);
847-
ParameterImpl p3 = new ParameterImpl("m3", ByteDataType.dataType,
906+
ParameterImpl p4 = new ParameterImpl("m3", ByteDataType.dataType,
848907
new VariableStorage(program, program.getRegister("r9")), program);
849908

850-
f.updateFunction("__stdcall", returnVar, FunctionUpdateType.DYNAMIC_STORAGE_ALL_PARAMS,
851-
true, SourceType.USER_DEFINED, p1, p2, p3);
909+
// function updated with auto parameters
910+
f.updateFunction("__thiscall", returnVar, FunctionUpdateType.DYNAMIC_STORAGE_ALL_PARAMS,
911+
true, SourceType.USER_DEFINED, p1, p2, p3, p4);
852912

853913
assertTrue(!f.hasCustomVariableStorage());
854-
assertEquals("__stdcall", f.getCallingConventionName());
914+
assertEquals("__thiscall", f.getCallingConventionName());
855915

856916
Parameter return1 = f.getReturn();
917+
assertTrue(return1.isForcedIndirect());
857918
assertTrue(bigStruct.isEquivalent(return1.getFormalDataType()));
858919
assertTrue(returnVar.getDataType().isEquivalent(returnVar.getDataType()));
859920
assertEquals("r12:4 (ptr)", return1.getVariableStorage().toString());
860921

861922
Parameter[] parameters = f.getParameters();
862-
assertEquals(3, parameters.length);
923+
assertEquals(4, parameters.length);
863924

864-
assertParameters(p1, parameters[0], false);
925+
assertParameter(p1, parameters[0], false);
926+
assertEquals(0, parameters[0].getOrdinal());
865927
assertEquals("r12:4 (auto)", parameters[0].getVariableStorage().toString());
866-
assertParameters(p2, parameters[1], false);
867-
assertEquals("Stack[0x0]:8", parameters[1].getVariableStorage().toString());
868-
assertParameters(p3, parameters[2], false);
869-
assertEquals("r11:1", parameters[2].getVariableStorage().toString());
928+
assertParameter(p2, parameters[1], false);
929+
assertEquals(1, parameters[1].getOrdinal());
930+
assertEquals("r11:4 (auto)", parameters[1].getVariableStorage().toString());
931+
assertParameter(p3, parameters[2], false);
932+
assertEquals(2, parameters[2].getOrdinal());
933+
assertEquals("Stack[0x0]:8", parameters[2].getVariableStorage().toString());
934+
assertParameter(p4, parameters[3], false);
935+
assertEquals(3, parameters[3].getOrdinal());
936+
assertEquals("r10:1", parameters[3].getVariableStorage().toString());
870937

871938
}
872939

@@ -908,13 +975,13 @@ public void testUpdateFunctionDynamicStorage2() throws Exception {
908975
ParameterImpl thisParam = new ParameterImpl(Function.THIS_PARAM_NAME,
909976
new PointerDataType(classStruct), VariableStorage.UNASSIGNED_STORAGE, program);
910977

911-
assertParameters(p1, parameters[0], false);
978+
assertParameter(p1, parameters[0], false);
912979
assertEquals("r12:4 (auto)", parameters[0].getVariableStorage().toString());
913-
assertParameters(thisParam, parameters[1], false);
980+
assertParameter(thisParam, parameters[1], false);
914981
assertEquals("r11:4 (auto)", parameters[1].getVariableStorage().toString());
915-
assertParameters(p2, parameters[2], false);
982+
assertParameter(p2, parameters[2], false);
916983
assertEquals("Stack[0x0]:8", parameters[2].getVariableStorage().toString());
917-
assertParameters(p3, parameters[3], false);
984+
assertParameter(p3, parameters[3], false);
918985
assertEquals("r10:1", parameters[3].getVariableStorage().toString());
919986

920987
// try again with DB params
@@ -933,13 +1000,17 @@ public void testUpdateFunctionDynamicStorage2() throws Exception {
9331000
parameters = f.getParameters();
9341001
assertEquals(4, parameters.length);
9351002

936-
assertParameters(p1, parameters[0], false);
1003+
assertParameter(p1, parameters[0], false);
1004+
assertEquals(0, parameters[0].getOrdinal());
9371005
assertEquals("r12:4 (auto)", parameters[0].getVariableStorage().toString());
938-
assertParameters(thisParam, parameters[1], false);
1006+
assertParameter(thisParam, parameters[1], false);
1007+
assertEquals(1, parameters[1].getOrdinal());
9391008
assertEquals("r11:4 (auto)", parameters[1].getVariableStorage().toString());
940-
assertParameters(p2, parameters[2], false);
1009+
assertParameter(p2, parameters[2], false);
1010+
assertEquals(2, parameters[2].getOrdinal());
9411011
assertEquals("Stack[0x0]:8", parameters[2].getVariableStorage().toString());
942-
assertParameters(p3, parameters[3], false);
1012+
assertParameter(p3, parameters[3], false);
1013+
assertEquals(3, parameters[3].getOrdinal());
9431014
assertEquals("r10:1", parameters[3].getVariableStorage().toString());
9441015

9451016
// try again with DB params and custom storage
@@ -951,19 +1022,23 @@ public void testUpdateFunctionDynamicStorage2() throws Exception {
9511022
assertEquals("__thiscall", f.getCallingConventionName());
9521023

9531024
return1 = f.getReturn();
954-
assertParameters(returnVar, return1, false);
1025+
assertParameter(returnVar, return1, false);
9551026
assertEquals("r12:4", return1.getVariableStorage().toString());
9561027

9571028
parameters = f.getParameters();
9581029
assertEquals(4, parameters.length);
9591030

960-
assertParameters(p1, parameters[0], false);
1031+
assertParameter(p1, parameters[0], false);
1032+
assertEquals(0, parameters[0].getOrdinal());
9611033
assertEquals("r12:4", parameters[0].getVariableStorage().toString());
962-
assertParameters(thisParam, parameters[1], false);
1034+
assertParameter(thisParam, parameters[1], false);
1035+
assertEquals(1, parameters[1].getOrdinal());
9631036
assertEquals("r11:4", parameters[1].getVariableStorage().toString());
964-
assertParameters(p2, parameters[2], false);
1037+
assertParameter(p2, parameters[2], false);
1038+
assertEquals(2, parameters[2].getOrdinal());
9651039
assertEquals("Stack[0x0]:8", parameters[2].getVariableStorage().toString());
966-
assertParameters(p3, parameters[3], false);
1040+
assertParameter(p3, parameters[3], false);
1041+
assertEquals(3, parameters[3].getOrdinal());
9671042
assertEquals("r10:1", parameters[3].getVariableStorage().toString());
9681043

9691044
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -826,7 +826,7 @@ void updateParametersAndReturn() {
826826
DataType dt = VariableUtilities.getAutoDataType(this,
827827
returnParam.getFormalDataType(), storage);
828828
try {
829-
autoParams.add(new AutoParameterImpl(dt, autoIndex, storage, this));
829+
autoParams.add(new AutoParameterImpl(dt, autoIndex++, storage, this));
830830
}
831831
catch (InvalidInputException e) {
832832
Msg.error(this,

0 commit comments

Comments
 (0)