GP-260 Corrected composite resolveequivalence issue introduced with

GP-260

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/CompositeDB.java

@@ -69,20 +69,26 @@ abstract class CompositeDB extends DataTypeDB implements Composite {
 	protected abstract void initialize();
 
 	/**
-	 * Get the preferred length for a new component. Constraining length of fixed-length datatype
+	 * Get the preferred length for a new component. For Unions and internally
-	 * may not be sustainable in response to datatype size changes over time.
+	 * aligned structures the preferred component length for a fixed-length dataType
+	 * will be the length of that dataType. Otherwise the length returned will be no
+	 * larger than the specified length.
+	 * 
 	 * @param dataType new component datatype
-	 * @param length   specified length required for Dynamic types such as string 
+	 * @param length   constrained length or -1 to force use of dataType size.
-	 * 				   which must have a positive length specified.
+	 *                 Dynamic types such as string must have a positive length
+	 *                 specified.
 	 * @return preferred component length
 	 */
 	protected int getPreferredComponentLength(DataType dataType, int length) {
-		if (length > 0 && (dataType instanceof Composite) &&
+		if ((isInternallyAligned() || (this instanceof Union)) && !(dataType instanceof Dynamic)) {
-			((Composite) dataType).isNotYetDefined()) {
+			length = -1; // force use of datatype size
-			return length;
 		}
 		int dtLength = dataType.getLength();
-		if (dtLength > 0) {
+		if (length <= 0) {
+			length = dtLength;
+		}
+		else if (dtLength > 0 && dtLength < length) {
 			length = dtLength;
 		}
 		if (length <= 0) {

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeComponentDB.java

@@ -345,17 +345,20 @@ class DataTypeComponentDB implements InternalDataTypeComponent {
 		DataType myParent = getParent();
 		boolean aligned =
 			(myParent instanceof Composite) ? ((Composite) myParent).isInternallyAligned() : false;
-		// Components don't need to have matching offset when they are aligned, only matching ordinal.
+		// Components don't need to have matching offset when they are aligned
-		// NOTE: use getOffset() and getOrdinal() methods since returned values will differ from
+		// NOTE: use getOffset() method since returned values will differ from
 		// stored values for flexible array component
 		if ((!aligned && (getOffset() != dtc.getOffset())) ||
-			// Components don't need to have matching length when they are aligned. Is this correct?
-			(!aligned && (getLength() != dtc.getLength())) || getOrdinal() != dtc.getOrdinal() ||
 			!SystemUtilities.isEqual(getFieldName(), dtc.getFieldName()) ||
 			!SystemUtilities.isEqual(getComment(), dtc.getComment())) {
 			return false;
 		}
 
+		// Component lengths need only be checked for dynamic types
+		if (getLength() != dtc.getLength() && (myDt instanceof Dynamic)) {
+			return false;
+		}
+
 		return DataTypeUtilities.isSameOrEquivalentDataType(myDt, otherDt);
 	}
 

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/StructureDB.java

@@ -684,8 +684,9 @@ class StructureDB extends CompositeDB implements Structure {
 	}
 
 	/**
-	 * Create copy of structure for target dtm (source archive information is discarded). WARNING!
+	 * Create copy of structure for target dtm (source archive information is discarded). 
-	 * copying unaligned structures which contain bitfields can produce invalid results when
+	 * <p>
+	 * WARNING! copying unaligned structures which contain bitfields can produce invalid results when
 	 * switching endianess due to the differences in packing order.
 	 * 
 	 * @param dtm target data type manager
@@ -1281,7 +1282,22 @@ class StructureDB extends CompositeDB implements Structure {
 
 			DataType dt = resolvedDts[i]; // ancestry check already performed by caller
 
-			int length = getPreferredComponentLength(dt, dtc.getLength());
+			int length = dt.getLength();
+			if (length <= 0 || dtc.isBitFieldComponent()) {
+				length = dtc.getLength();
+			}
+			else {
+				// do not exceed available space
+				int maxOffset;
+				int nextIndex = i + 1;
+				if (nextIndex < otherComponents.length) {
+					maxOffset = otherComponents[nextIndex].getOffset();
+				}
+				else {
+					maxOffset = struct.getLength();
+				}
+				length = Math.min(length, maxOffset - dtc.getOffset());
+			}
 
 			Record rec = componentAdapter.createRecord(dataMgr.getResolvedID(dt), key, length,
 				dtc.getOrdinal(), dtc.getOffset(), dtc.getFieldName(), dtc.getComment());
@@ -1358,6 +1374,9 @@ class StructureDB extends CompositeDB implements Structure {
 
 	@Override
 	public void dataTypeSizeChanged(DataType dt) {
+		if (dt instanceof BitFieldDataType) {
+			return; // unsupported
+		}
 		lock.acquire();
 		try {
 			checkDeleted();
@@ -1374,7 +1393,10 @@ class StructureDB extends CompositeDB implements Structure {
 					// assume no impact to bitfields since base types
 					// should not change size
 					int dtcLen = dtc.getLength();
-					int length = getPreferredComponentLength(dt, dtcLen);
+					int length = dt.getLength();
+					if (length <= 0) {
+						length = dtcLen;
+					}
 					if (length < dtcLen) {
 						dtc.setLength(length, true);
 						shiftOffsets(i + 1, dtcLen - length, 0);
@@ -1427,7 +1449,10 @@ class StructureDB extends CompositeDB implements Structure {
 				continue;
 			}
 			int dtcLen = dtc.getLength();
-			int length = getPreferredComponentLength(dt, dtcLen);
+			int length = dt.getLength();
+			if (length <= 0) {
+				length = dtcLen;
+			}
 			if (dtcLen != length) {
 				if (length < dtcLen) {
 					dtc.setLength(length, true);
@@ -1514,14 +1539,18 @@ class StructureDB extends CompositeDB implements Structure {
 				return false;
 			}
 
-			int myNumComps = getNumComponents();
+			int myNumComps = components.size();
-			int otherNumComps = struct.getNumComponents();
+			int otherNumComps = struct.getNumDefinedComponents();
 			if (myNumComps != otherNumComps) {
 				return false;
 			}
+			DataTypeComponent[] otherDefinedComponents = struct.getDefinedComponents();
+			if (otherDefinedComponents.length != myNumComps) { // safety check
+				return false;
+			}
 			for (int i = 0; i < myNumComps; i++) {
-				DataTypeComponent myDtc = getComponent(i);
+				DataTypeComponent myDtc = components.get(i);
-				DataTypeComponent otherDtc = struct.getComponent(i);
+				DataTypeComponent otherDtc = otherDefinedComponents[i];
 				if (!myDtc.isEquivalent(otherDtc)) {
 					return false;
 				}

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/UnionDB.java

@@ -418,14 +418,19 @@ class UnionDB extends CompositeDB implements Union {
 
 	@Override
 	public void dataTypeSizeChanged(DataType dt) {
+		if (dt instanceof BitFieldDataType) {
+			return; // unsupported
+		}
 		lock.acquire();
 		try {
 			checkDeleted();
 			boolean changed = false;
 			for (DataTypeComponentDB dtc : components) {
-				int length = dtc.getLength();
 				if (dtc.getDataType() == dt) {
-					length = getPreferredComponentLength(dt, length);
+					int length = dt.getLength();
+					if (length <= 0) {
+						length = dtc.getLength();
+					}
 					dtc.setLength(length, true);
 					changed = true;
 				}
@@ -448,7 +453,10 @@ class UnionDB extends CompositeDB implements Union {
 				dt = adjustBitField(dt); // in case base type changed
 			}
 			int dtcLen = dtc.getLength();
-			int length = getPreferredComponentLength(dt, dtcLen);
+			int length = dt.getLength();
+			if (length <= 0) {
+				length = dtcLen;
+			}
 			if (length != dtcLen) {
 				dtc.setLength(length, true);
 				changed = true;

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeComponent.java

@@ -160,7 +160,9 @@ public interface DataTypeComponent {
 	 * Returns true if the given dataTypeComponent is equivalent to this dataTypeComponent.
 	 * A dataTypeComponent is "equivalent" if the other component has a data type
 	 * that is equivalent to this component's data type. The dataTypeComponents must
-	 * also have the same offset, length, ordinal, field name, and comment.
+	 * also have the same offset, field name, and comment.  The length is only checked
+	 * for components which are dyanmic and whose size must be specified when creating
+	 * a component.
 	 * @param dtc the dataTypeComponent being tested for equivalence.
 	 * @return true if the given dataTypeComponent is equivalent to this dataTypeComponent.
 	 */

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeComponentImpl.java

@@ -329,17 +329,20 @@ public class DataTypeComponentImpl implements InternalDataTypeComponent, Seriali
 		DataType myParent = getParent();
 		boolean aligned =
 			(myParent instanceof Composite) ? ((Composite) myParent).isInternallyAligned() : false;
-		// Components don't need to have matching offset when they are aligned, only matching ordinal.
+		// Components don't need to have matching offset when they are aligned
-		if ((!aligned && (getOffset() != dtc.getOffset())) ||
+		// NOTE: use getOffset() method since returned values will differ from
-			// Components don't need to have matching length when they are aligned. Is this correct?
-			// NOTE: use getOffset() and getOrdinal() methods since returned values will differ from
 		// stored values for flexible array component
-			(!aligned && (getLength() != dtc.getLength())) || getOrdinal() != dtc.getOrdinal() ||
+		if ((!aligned && (getOffset() != dtc.getOffset())) ||
 			!SystemUtilities.isEqual(getFieldName(), dtc.getFieldName()) ||
 			!SystemUtilities.isEqual(getComment(), dtc.getComment())) {
 			return false;
 		}
 
+		// Component lengths need only be checked for dynamic types
+		if (getLength() != dtc.getLength() && (myDt instanceof Dynamic)) {
+			return false;
+		}
+
 		return DataTypeUtilities.isSameOrEquivalentDataType(myDt, otherDt);
 	}
 

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/StructureDataType.java

@@ -797,15 +797,18 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 			return false;
 		}
 
-		int myNumComps = getNumComponents();
+		int myNumComps = components.size();
-		int otherNumComps = struct.getNumComponents();
+		int otherNumComps = struct.getNumDefinedComponents();
 		if (myNumComps != otherNumComps) {
 			return false;
 		}
+		DataTypeComponent[] otherDefinedComponents = struct.getDefinedComponents();
+		if (otherDefinedComponents.length != myNumComps) { // safety check
+			return false;
+		}
 		for (int i = 0; i < myNumComps; i++) {
-			DataTypeComponent myDtc = getComponent(i);
+			DataTypeComponent myDtc = components.get(i);
-			DataTypeComponent otherDtc = struct.getComponent(i);
+			DataTypeComponent otherDtc = otherDefinedComponents[i];
-
 			if (!myDtc.isEquivalent(otherDtc)) {
 				return false;
 			}
@@ -815,6 +818,9 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 
 	@Override
 	public void dataTypeSizeChanged(DataType dt) {
+		if (dt instanceof BitFieldDataType) {
+			return; // unsupported
+		}
 		if (isInternallyAligned()) {
 			adjustInternalAlignment();
 			return;
@@ -823,21 +829,23 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 		int n = components.size();
 		for (int i = 0; i < n; i++) {
 			DataTypeComponentImpl dtc = components.get(i);
-			int nextIndex = i + 1;
 			if (dtc.getDataType() == dt) {
 				// assume no impact to bitfields since base types
 				// should not change size
-				int dtLen = dt.getLength();
 				int dtcLen = dtc.getLength();
-				if (dtLen < dtcLen) {
+				int length = dt.getLength();
-					dtc.setLength(dtLen);
+				if (length <= 0) {
-					shiftOffsets(nextIndex, dtcLen - dtLen, 0);
+					length = dtcLen;
+				}
+				if (length < dtcLen) {
+					dtc.setLength(length);
+					shiftOffsets(i + 1, dtcLen - length, 0);
 					didChange = true;
 				}
-				else if (dtLen > dtcLen) {
+				else if (length > dtcLen) {
-					int consumed = consumeBytesAfter(i, dtLen - dtcLen);
+					int consumed = consumeBytesAfter(i, length - dtcLen);
 					if (consumed > 0) {
-						shiftOffsets(nextIndex, 0 - consumed, 0);
+						shiftOffsets(i + 1, 0 - consumed, 0);
 						didChange = true;
 					}
 				}
@@ -890,8 +898,9 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 	}
 
 	/**
-	 * Create copy of structure for target dtm (source archive information is discarded). WARNING!
+	 * Create copy of structure for target dtm (source archive information is discarded). 
-	 * copying unaligned structures which contain bitfields can produce invalid results when
+	 * <p>
+	 * WARNING! copying unaligned structures which contain bitfields can produce invalid results when
 	 * switching endianess due to the differences in packing order.
 	 * 
 	 * @param dtm target data type manager
@@ -991,8 +1000,7 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 	private void doReplaceWithAligned(Structure struct) {
 		// assumes components is clear and that alignment characteristics have been set
 		DataTypeComponent[] otherComponents = struct.getDefinedComponents();
-		for (int i = 0; i < otherComponents.length; i++) {
+		for (DataTypeComponent dtc : otherComponents) {
-			DataTypeComponent dtc = otherComponents[i];
 			DataType dt = dtc.getDataType();
 			int length = (dt instanceof Dynamic) ? dtc.getLength() : -1;
 			add(dt, length, dtc.getFieldName(), dtc.getComment());
@@ -1011,11 +1019,25 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 		DataTypeComponent[] otherComponents = struct.getDefinedComponents();
 		for (int i = 0; i < otherComponents.length; i++) {
 			DataTypeComponent dtc = otherComponents[i];
-
 			DataType dt = dtc.getDataType().clone(dataMgr);
 			checkAncestry(dt);
 
-			int length = getPreferredComponentLength(dt, dtc.getLength());
+			int length = dt.getLength();
+			if (length <= 0 || dtc.isBitFieldComponent()) {
+				length = dtc.getLength();
+			}
+			else {
+				// do not exceed available space
+				int maxOffset;
+				int nextIndex = i + 1;
+				if (nextIndex < otherComponents.length) {
+					maxOffset = otherComponents[nextIndex].getOffset();
+				}
+				else {
+					maxOffset = struct.getLength();
+				}
+				length = Math.min(length, maxOffset - dtc.getOffset());
+			}
 
 			components.add(new DataTypeComponentImpl(dt, this, length, dtc.getOrdinal(),
 				dtc.getOffset(), dtc.getFieldName(), dtc.getComment()));
@@ -1355,8 +1377,7 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 
 	@Override
 	public void deleteAll() {
-		for (int i = 0; i < components.size(); i++) {
+		for (DataTypeComponentImpl dtc : components) {
-			DataTypeComponent dtc = components.get(i);
 			dtc.getDataType().removeParent(this);
 		}
 		components.clear();

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/UnionDataType.java

@@ -372,11 +372,16 @@ public class UnionDataType extends CompositeDataTypeImpl implements Union {
 
 	@Override
 	public void dataTypeSizeChanged(DataType dt) {
+		if (dt instanceof BitFieldDataType) {
+			return; // unsupported
+		}
 		boolean changed = false;
 		for (DataTypeComponentImpl dtc : components) {
-			int length = dtc.getLength();
 			if (dtc.getDataType() == dt) {
-				length = getPreferredComponentLength(dt, length);
+				int length = dt.getLength();
+				if (length <= 0) {
+					length = dtc.getLength();
+				}
 				dtc.setLength(length);
 				changed = true;
 			}