GT-3414 code review, unit tests for DefinedDataIterator.

Ghidra/Features/Base/src/main/java/ghidra/program/database/ProgramBuilder.java

@@ -695,6 +695,14 @@ public class ProgramBuilder {
 		return c;
 	}
 
+	public void applyFixedLengthDataType(String addressString, DataType dt, int length)
+			throws CodeUnitInsertionException {
+		startTransaction();
+		DataUtilities.createData(program, addr(addressString), dt, length, false,
+			ClearDataMode.CLEAR_ALL_CONFLICT_DATA);
+		endTransaction();
+	}
+
 	public void applyDataType(String addressString, DataType dt) {
 		applyDataType(addressString, dt, 1);
 	}
@@ -874,7 +882,7 @@ public class ProgramBuilder {
 	}
 
 	public Data createString(String address, String string, Charset charset, boolean nullTerminate,
-			AbstractStringDataType dataType) throws Exception {
+			DataType dataType) throws Exception {
 		if (nullTerminate) {
 			string = string + "\0";
 		}
@@ -883,7 +891,7 @@ public class ProgramBuilder {
 	}
 
 	public Data createString(String address, byte[] stringBytes, Charset charset,
-			AbstractStringDataType dataType) throws Exception {
+			DataType dataType) throws Exception {
 		Address addr = addr(address);
 		setBytes(address, stringBytes);
 		if (dataType != null) {

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/string/DefinedStringIteratorTest.java

@@ -72,10 +72,17 @@ public class DefinedStringIteratorTest extends AbstractGhidraHeadlessIntegration
 		builder.createEncodedString("0x500", "This is the last string", StandardCharsets.US_ASCII,
 			false);
 
+		ArrayDataType charArray = new ArrayDataType(new CharDataType(), 50, 1);
+		builder.createString("0x600", "The 600 chararray", StandardCharsets.US_ASCII, true,
+			charArray);
+
 		// create an empty area for tests to do their own thing
-		builder.createUninitializedMemory("uninitialized", "0x3000", 100);
+		builder.createUninitializedMemory("uninitialized", "0x3000", 0x1000);
+		builder.applyDataType("0x3100", charArray);
+		builder.applyFixedLengthDataType("0x3200", new StringDataType(), 10);
 
 		program = builder.getProgram();
+
 	}
 
 	@Test
@@ -117,6 +124,11 @@ public class DefinedStringIteratorTest extends AbstractGhidraHeadlessIntegration
 		assertEquals(addr(0x500), foundString.getAddress());
 		assertEquals("This is the last string", foundString.getString(program.getMemory()));
 
+		assertTrue(iterator.hasNext());
+		foundString = iterator.next();
+		assertEquals(addr(0x600), foundString.getAddress());
+		assertEquals("The 600 chararray", foundString.getString(program.getMemory()));
+
 		assertFalse(iterator.hasNext());
 	}
 

Ghidra/Features/Base/src/test/java/ghidra/program/util/DefinedDataIteratorTest.java

@@ -0,0 +1,149 @@
+/* ###
+ * 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.util;
+
+import static org.junit.Assert.*;
+
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import ghidra.program.database.ProgramDB;
+import ghidra.program.model.data.*;
+import ghidra.program.model.listing.Data;
+import ghidra.program.model.util.CodeUnitInsertionException;
+import ghidra.test.AbstractGhidraHeadlessIntegrationTest;
+import ghidra.test.ToyProgramBuilder;
+import util.CollectionUtils;
+
+public class DefinedDataIteratorTest extends AbstractGhidraHeadlessIntegrationTest {
+
+	private ToyProgramBuilder builder;
+	private ProgramDB program;
+	private DataTypeManager dtm;
+	private DataType intDT;
+	private StringDataType stringDT;
+	private CharDataType charDT;
+	private DataType charArray;
+	private StructureDataType struct1DT;
+	private ArrayDataType structArray;
+	private StructureDataType struct2DT;
+	private TypeDef intTD;
+
+	@Before
+	public void setUp() throws Exception {
+
+		builder = new ToyProgramBuilder("DefinedDataIteratorTests", false);
+		program = builder.getProgram();
+		dtm = program.getDataTypeManager();
+
+		intDT = AbstractIntegerDataType.getSignedDataType(4, dtm);
+		intTD = new TypedefDataType("int_typedef", intDT);
+		stringDT = StringDataType.dataType;
+		charDT = new CharDataType(dtm);
+		charArray = new ArrayDataType(charDT, 20, charDT.getLength());
+
+		struct1DT = new StructureDataType("struct1", 100);
+		struct1DT.replaceAtOffset(0, intDT, intDT.getLength(), "f1", null);
+		struct1DT.replaceAtOffset(10, charArray, charArray.getLength(), "f2", null);
+		struct1DT.replaceAtOffset(50, stringDT, 10, "f3", null);
+
+		structArray = new ArrayDataType(struct1DT, 10, struct1DT.getLength());
+
+		struct2DT = new StructureDataType("struct2", 200);
+		struct2DT.replaceAtOffset(0, intDT, intDT.getLength(), "f1", null);
+		struct2DT.replaceAtOffset(10, struct1DT, intDT.getLength(), "f2", null);
+
+		builder.createMemory("test", "0x0", 0x2000);
+		program = builder.getProgram();
+	}
+
+	@Test
+	public void test_Ints() throws Exception {
+		builder.applyFixedLengthDataType("0x0", intDT, intDT.getLength());
+		builder.createString("0x10", "test1", StandardCharsets.UTF_8, true, stringDT);
+		builder.applyFixedLengthDataType("0x100", struct1DT, struct1DT.getLength());
+
+		List<Data> list = CollectionUtils.asList(
+			DefinedDataIterator.byDataType(program, dt -> dt instanceof IntegerDataType));
+
+		assertTrue(list.get(0).getAddress().getOffset() == 0x0);
+		assertTrue(list.get(1).getAddress().getOffset() == 0x100);
+
+		assertEquals(2, list.size());
+	}
+
+	@Test
+	public void test_Strings() throws Exception {
+		builder.applyFixedLengthDataType("0x0", intDT, intDT.getLength());
+		builder.createString("0x10", "test1", StandardCharsets.UTF_8, true, stringDT);
+		builder.applyFixedLengthDataType("0x100", struct1DT, struct1DT.getLength());
+
+		List<Data> list = CollectionUtils.asList(DefinedDataIterator.definedStrings(program));
+
+		assertTrue(list.get(0).getAddress().getOffset() == 0x10);
+		assertTrue(list.get(1).getAddress().getOffset() == 0x100 + 10);
+		assertTrue(list.get(2).getAddress().getOffset() == 0x100 + 50);
+
+		assertEquals(3, list.size());
+	}
+
+	@Test
+	public void test_ArrayOfStructs() throws Exception {
+		builder.applyFixedLengthDataType("0x0", intDT, intDT.getLength());
+		builder.createString("0x10", "test1", StandardCharsets.UTF_8, true, stringDT);
+		builder.applyFixedLengthDataType("0x100", structArray, structArray.getLength());
+
+		int numElements = structArray.getNumElements();
+		int lastEle = numElements - 1;
+		int elementSize = structArray.getElementLength();
+
+		List<Data> list = CollectionUtils.asList(DefinedDataIterator.definedStrings(program));
+
+		assertEquals(list.get(0).getAddress().getOffset(), 0x10);
+		assertEquals(list.get(1 + 0).getAddress().getOffset(), 0x100 + 10);
+		assertEquals(list.get(1 + 1).getAddress().getOffset(), 0x100 + 50);
+
+		assertEquals(list.get(1 + (lastEle * 2) + 0).getAddress().getOffset(),
+			0x100 + (elementSize * lastEle) + 10);
+		assertEquals(list.get(1 + (lastEle * 2) + 1).getAddress().getOffset(),
+			0x100 + (elementSize * lastEle) + 50);
+
+		assertEquals(1 + (numElements * 2), list.size());
+	}
+
+	@Test
+	public void test_Typedefs() throws CodeUnitInsertionException {
+		// 3 ints: 2 are typedefs, 1 is regular int
+		builder.applyFixedLengthDataType("0x0", intTD, intTD.getLength());
+		builder.applyFixedLengthDataType("0x10", intTD, intTD.getLength());
+		builder.applyFixedLengthDataType("0x20", intDT, intTD.getLength());
+
+		// iterating by data type ignores typedefs, so we should get all 3 ints
+		List<Data> list = CollectionUtils.asList(
+			DefinedDataIterator.byDataType(program, dt -> dt instanceof IntegerDataType));
+
+		assertEquals(3, list.size());
+
+		// iterating by data instance, we can inspect the actual data type and get the
+		// typedef
+		list = CollectionUtils.asList(DefinedDataIterator.byDataInstance(program,
+			data -> data.getDataType() instanceof TypeDef));
+		assertEquals(2, list.size());
+	}
+}

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

@@ -26,9 +26,15 @@ import util.CollectionUtils;
  * @see CollectionUtils#asIterable
  */
 public interface DataIterator extends Iterator<Data> {
-	public static final DataIterator EMPTY = Of(/*nothing*/);
+	public static final DataIterator EMPTY = of(/*nothing*/);
 
-	public static DataIterator Of(Data... dataInstances) {
+	/**
+	 * Create a DataIterator that returns a sequence of the specified items.
+	 * 
+	 * @param dataInstances variable length list of items that will be iterated
+	 * @return new Iterator 
+	 */
+	public static DataIterator of(Data... dataInstances) {
 		return new IteratorWrapper(Arrays.asList(dataInstances).iterator());
 	}
 

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/util/DefinedDataIterator.java

@@ -23,7 +23,7 @@ import ghidra.program.model.data.*;
 import ghidra.program.model.listing.*;
 
 /**
- * Iterator that visits each defined data instance in a Program or in the footprint of
+ * Iterator that visits each defined data instance in the initialized memory of a Program or in the footprint of
  * a specified data element.
  * <p>
  * Data elements that are nested inside of composites or arrays are visited, not just the
@@ -32,7 +32,7 @@ import ghidra.program.model.listing.*;
 public class DefinedDataIterator implements DataIterator {
 
 	/**
-	 * Creates a new iterator that traverses the entire Program's address space, visiting
+	 * Creates a new iterator that traverses the entire Program's address space, returning
 	 * data instances that successfully match the predicate.
 	 *
 	 * @param program Program to search
@@ -45,7 +45,21 @@ public class DefinedDataIterator implements DataIterator {
 	}
 
 	/**
-	 * Creates a new iterator that traverses the entire Program's address space.
+	 * Creates a new iterator that traverses the entire Program's address space, returning
+	 * data instances that successfully match the predicate.
+	 *
+	 * @param program Program to search
+	 * @param dataInstancePredicate {@link Predicate} that tests each data instance's properties
+	 * @return new iterator
+	 */
+	public static DefinedDataIterator byDataInstance(Program program,
+			Predicate<Data> dataInstancePredicate) {
+		return new DefinedDataIterator(program, null, null, dataInstancePredicate);
+	}
+
+	/**
+	 * Creates a new iterator that traverses the entire Program's address space returning
+	 * data instances that are strings.
 	 *
 	 * @param program Ghidra {@link Program} to search
 	 * @return new iterator
@@ -57,7 +71,8 @@ public class DefinedDataIterator implements DataIterator {
 	}
 
 	/**
-	 * Creates a new iterator that traverses a portion of the Program's address space.
+	 * Creates a new iterator that traverses a portion of the Program's address space returning
+	 * data instances that are strings.
 	 *
 	 * @param program Ghidra {@link Program} to search
 	 * @param addrs addresses to limit the iteration to
@@ -84,6 +99,12 @@ public class DefinedDataIterator implements DataIterator {
 
 	private Predicate<DataType> dataTypePredicate;
 	private Predicate<Data> dataInstancePredicate;
+
+	/**
+	 * LIFO stack of iterators.  Newly found iterators of sub-components are
+	 * pushed onto the end and become the current iterator.  When an iterator is exhausted, 
+	 * it is popped of the end and the uncovered iterator is now the current.    
+	 */
 	private Deque<DataIterator> itStack = new ArrayDeque<>();
 	private Data currentDataResult;
 
@@ -101,7 +122,7 @@ public class DefinedDataIterator implements DataIterator {
 		this.dataTypePredicate = dataTypePredicate;
 		this.dataInstancePredicate = dataInstancePredicate;
 
-		itStack.addLast(DataIterator.Of(singleDataInstance));
+		itStack.addLast(DataIterator.of(singleDataInstance));
 	}
 
 	@Override
@@ -160,6 +181,8 @@ public class DefinedDataIterator implements DataIterator {
 			return recursiveMatchesDataTypePredicate(elementDT);
 		}
 		else if (dt instanceof Structure) {
+			// handle Structures and general Composite's separately so
+			// we can focus on just the defined elements of a structure
 			Structure comp = (Structure) dt;
 			for (DataTypeComponent dtc : comp.getDefinedComponents()) {
 				if (recursiveMatchesDataTypePredicate(dtc.getDataType())) {