yetangitu/ghidra
1
0
Fork 0
mirror of https://github.com/NationalSecurityAgency/ghidra.git synced 2025-10-03 17:59:46 +02:00
Code Issues Releases Wiki Activity

GP-4913 / GP-5198 Correct Function StorageEditor issues

Browse source
This commit is contained in:
ghidra1 2025-01-10 12:31:29 -05:00
parent 2c4882c16a
commit b8afbf7d8b
5 changed files with 115 additions and 68 deletions
Download patch file Download diff file Expand all files Collapse all files

2
Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/ParameterDataTypeCellEditor.java
View file

@ -159,7 +159,7 @@ class ParameterDataTypeCellEditor extends AbstractCellEditor
fireEditingCanceled(); // user picked the same datatype
}
else {
dt = dataType;
dt = dataType.clone(dtm);
fireEditingStopped();
}
}

71
Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/StorageAddressModel.java
View file

@ -161,40 +161,7 @@ class StorageAddressModel {
private void validate() {
statusText = "";
isValid = hasValidVarnodes() && hasCorrectAllocatedSize() && isProperMix() && hasNoDups();
}
private boolean hasNoDups() {
AddressSet addressSet = new AddressSet();
for (int i = 0; i < varnodes.size(); i++) {
VarnodeInfo varnode = varnodes.get(i);
AddressRange range;
try {
range = new AddressRangeImpl(varnode.getAddress(), varnode.getSize());
}
catch (AddressOverflowException e) {
// should already have been checked
throw new AssertException("Unexpected exception", e);
}
if (addressSet.intersects(range.getMinAddress(), range.getMaxAddress())) {
statusText = "Row " + i + ": Overlapping storage address used.";
return false;
}
addressSet.add(range);
}
return true;
}
private boolean isProperMix() {
// all except last varnode must be a register
for (int i = 0; i < varnodes.size() - 1; i++) {
VarnodeInfo varnode = varnodes.get(i);
if (varnode.getType() != VarnodeType.Register) {
statusText = "Only the last entry may be of type " + varnode.getType();
return false;
}
}
return true;
isValid = hasValidVarnodes() && hasCorrectAllocatedSize();
}
private boolean hasCorrectAllocatedSize() {
@ -217,11 +184,22 @@ class StorageAddressModel {
private boolean hasValidVarnodes() {
for (int i = 0; i < varnodes.size(); i++) {
VarnodeInfo varnode = varnodes.get(i);
if (!(isValidSize(varnode, i) && isValidAddress(varnode, i))) {
if (varnode.getAddress() == null) {
statusText = "Row " + i + ": Storage not specified";
return false;
}
if (!(isValidSize(varnode, i) || !isValidAddress(varnode, i))) {
return false;
}
}
return true;
try {
buildStorage();
return true;
}
catch (InvalidInputException e) {
statusText = e.getMessage();
return false;
}
}
private boolean isValidSize(VarnodeInfo varnode, int row) {
@ -349,10 +327,7 @@ class StorageAddressModel {
return program;
}
VariableStorage getStorage() {
if (!isValid) {
return null;
}
private VariableStorage buildStorage() throws InvalidInputException {
if (varnodes.size() == 0) {
if (requiredSize == 0) {
return VariableStorage.VOID_STORAGE;
@ -361,10 +336,22 @@ class StorageAddressModel {
}
List<Varnode> varnodeList = new ArrayList<>(varnodes.size());
for (VarnodeInfo varnodeInfo : varnodes) {
varnodeList.add(new Varnode(varnodeInfo.getAddress(), varnodeInfo.getSize()));
Address addr = varnodeInfo.getAddress();
if (addr == null) {
// this method should not be invoked with this condition
return VariableStorage.UNASSIGNED_STORAGE;
}
varnodeList.add(new Varnode(addr, varnodeInfo.getSize()));
}
return new VariableStorage(program, varnodeList);
}
VariableStorage getStorage() {
if (!isValid) {
return null;
}
try {
return new VariableStorage(program, varnodeList);
return buildStorage();
}
catch (InvalidInputException e) {
// validation checks should prevent this.

32
Ghidra/Features/Base/src/test/java/ghidra/app/plugin/core/function/editor/StorageEditorModelBETest.java
View file

@ -40,7 +40,7 @@ public class StorageEditorModelBETest extends StorageEditorModelTest {
varnode = model.getVarnodes().get(1);
model.setVarnode(varnode, program.getRegister(testRegName).getAddress().add(6), 2);
assertTrue(!model.isValid());
assertEquals("Row 1: Overlapping storage address used.", model.getStatusText());
assertEquals("One or more conflicting storage varnodes", model.getStatusText());
}
@Test
@ -70,4 +70,34 @@ public class StorageEditorModelBETest extends StorageEditorModelTest {
assertEquals(register.getAddress().getOffset(),
model.getVarnodes().get(0).getAddress().getOffset());
}
@Override
@Test
public void testCompoundStorage() {
// Big-endian test case
Register register = model.getProgram().getRegister("g1");
assertNotNull(register);
VarnodeInfo varnode = model.getVarnodes().get(0);
assertEquals(VarnodeType.Stack, varnode.getType());
model.addVarnode();
varnode = model.getVarnodes().get(1);
model.setVarnodeType(varnode, VarnodeType.Register);
model.setVarnode(varnode, register);
assertTrue(!model.isValid());
assertEquals("Compound storage must use registers except for last BE varnode",
model.getStatusText());
// select last row (i.e., register) and move it up to make valid with stack being last
model.setSelectedVarnodeRows(new int[] { 1 });
model.moveSelectedVarnodeUp();
assertTrue(model.isValid());
}
}

53
Ghidra/Features/Base/src/test/java/ghidra/app/plugin/core/function/editor/StorageEditorModelTest.java
View file

@ -70,20 +70,19 @@ public class StorageEditorModelTest extends AbstractGuiTest {
Address address = stackSpace.getAddress(4);
VariableStorage storage = new VariableStorage(program, address, currentStorage);
model = new StorageAddressModel(program, storage,
new ModelChangeListener() {
model = new StorageAddressModel(program, storage, new ModelChangeListener() {
@Override
public void dataChanged() {
dataChangeCalled = true;
}
@Override
public void dataChanged() {
dataChangeCalled = true;
}
@Override
public void tableRowsChanged() {
// nothing here
}
@Override
public void tableRowsChanged() {
// nothing here
}
});
});
model.setRequiredSize(requiredStorage, unconstrained);
}
@ -297,7 +296,37 @@ public class StorageEditorModelTest extends AbstractGuiTest {
varnode = model.getVarnodes().get(1);
model.setVarnode(varnode, program.getRegister(testRegName).getAddress(), 2);
assertTrue(!model.isValid());
assertEquals("Row 1: Overlapping storage address used.", model.getStatusText());
assertEquals("One or more conflicting storage varnodes", model.getStatusText());
}
@Test
public void testCompoundStorage() {
// Little-endian test case
VarnodeInfo varnode = model.getVarnodes().get(0);
assertEquals(VarnodeType.Stack, varnode.getType());
model.addVarnode();
// select new row and move it up
model.setSelectedVarnodeRows(new int[] { 1 });
model.moveSelectedVarnodeUp();
varnode = model.getVarnodes().get(0);
model.setVarnodeType(varnode, VarnodeType.Register);
model.setVarnode(varnode, program.getRegister(testRegName).getAddress(), 4);
assertTrue(!model.isValid());
assertEquals("Compound storage must use registers except for first LE varnode",
model.getStatusText());
// select last row (i.e., stack) and move it as first to make valid
model.setSelectedVarnodeRows(new int[] { 1 });
model.moveSelectedVarnodeUp();
assertTrue(model.isValid());
}
}

13
Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/VariableStorage.java
View file

@ -181,7 +181,7 @@ public class VariableStorage implements Comparable<VariableStorage> {
return size;
}
private void checkVarnodes() throws InvalidInputException {
private void checkVarnodes() throws IllegalArgumentException, InvalidInputException {
if (varnodes.length == 0) {
throw new IllegalArgumentException("A minimum of one varnode must be specified");
}
@ -191,10 +191,11 @@ public class VariableStorage implements Comparable<VariableStorage> {
for (int i = 0; i < varnodes.length; i++) {
Varnode varnode = varnodes[i];
if (varnode == null) {
throw new InvalidInputException("Null varnode not permitted");
throw new IllegalArgumentException("Null varnode not permitted");
}
if (varnode.getSize() <= 0) {
throw new InvalidInputException("Unsupported varnode size: " + varnode.getSize());
throw new IllegalArgumentException(
"Unsupported varnode size: " + varnode.getSize());
}
boolean isRegister = false;
@ -246,13 +247,13 @@ public class VariableStorage implements Comparable<VariableStorage> {
if (programArch.getLanguage().isBigEndian()) {
if (i < (varnodes.length - 1) && !isRegister) {
throw new InvalidInputException(
"Compound storage must use registers except for least significant varnode");
"Compound storage must use registers except for last BE varnode");
}
}
else {
if (i > 0 && !isRegister) {
throw new InvalidInputException(
"Compound storage must use registers except for most significant varnode");
"Compound storage must use registers except for first LE varnode");
}
}
size += varnode.getSize();
@ -260,7 +261,7 @@ public class VariableStorage implements Comparable<VariableStorage> {
for (int i = 0; i < varnodes.length; i++) {
for (int j = i + 1; j < varnodes.length; j++) {
if (varnodes[i].intersects(varnodes[j])) {
throw new InvalidInputException("One or more conflicting varnodes");
throw new InvalidInputException("One or more conflicting storage varnodes");
}
}
}