Merge remote-tracking branch 'origin/Ghidra_11.3'

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

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

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/StorageAddressModel.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.
@@ -161,40 +161,7 @@ class StorageAddressModel {
 
 	private void validate() {
 		statusText = "";
-		isValid = hasValidVarnodes() && hasCorrectAllocatedSize() && isProperMix() && hasNoDups();
+		isValid = hasValidVarnodes() && hasCorrectAllocatedSize();
-	}
-
-	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;
 	}
 
 	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() {
+	private VariableStorage buildStorage() throws InvalidInputException {
-		if (!isValid) {
-			return null;
-		}
 		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.

Ghidra/Features/Base/src/test/java/ghidra/app/plugin/core/function/editor/StorageEditorModelBETest.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.
@@ -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());
+	}
 }

Ghidra/Features/Base/src/test/java/ghidra/app/plugin/core/function/editor/StorageEditorModelTest.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.
@@ -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,
+		model = new StorageAddressModel(program, storage, new ModelChangeListener() {
-			new ModelChangeListener() {
 
-				@Override
+			@Override
-				public void dataChanged() {
+			public void dataChanged() {
-					dataChangeCalled = true;
+				dataChangeCalled = true;
-				}
+			}
 
-				@Override
+			@Override
-				public void tableRowsChanged() {
+			public void tableRowsChanged() {
-					// nothing here
+				// 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());
 	}
 
 }

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

@@ -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");
 				}
 			}
 		}