Merge remote-tracking branch 'origin/patch'

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

@@ -671,8 +671,8 @@ abstract class CompositeDB extends DataTypeDB implements CompositeInternal {
 	}
 
 	/**
-	 * Copy packing and alignment settings from specified composite without
-	 * repacking or notification.
+	 * Set packing and alignment settings.  Record is modified but it is not written to the
+	 * database and no repacking or notification is performed.
 	 * @param composite instance whose packing and alignment are to be copied
 	 * @throws IOException if database IO error occured
 	 */
@@ -681,7 +681,6 @@ abstract class CompositeDB extends DataTypeDB implements CompositeInternal {
 			composite.getStoredMinimumAlignment());
 		record.setIntValue(CompositeDBAdapter.COMPOSITE_PACKING_COL,
 			composite.getStoredPackingValue());
-		compositeAdapter.updateRecord(record, true);
 	}
 
 	@Override

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

@@ -220,15 +220,7 @@ class DataTypeComponentDB implements InternalDataTypeComponent {
 	@Override
 	public void setFieldName(String name) throws DuplicateNameException {
 		if (record != null) {
-			if (name != null) {
-				name = name.trim();
-				if (name.length() == 0 || name.equals(getDefaultFieldName())) {
-					name = null;
-				}
-				else {
-					checkDuplicateName(name);
-				}
-			}
+			name = checkFieldName(name);
 			record.setString(ComponentDBAdapter.COMPONENT_FIELD_NAME_COL, name);
 			updateRecord(true);
 		}
@@ -246,6 +238,19 @@ class DataTypeComponentDB implements InternalDataTypeComponent {
 		}
 	}
 
+	private String checkFieldName(String name) throws DuplicateNameException {
+		if (name != null) {
+			name = name.trim();
+			if (name.length() == 0 || name.equals(getDefaultFieldName())) {
+				name = null;
+			}
+			else {
+				checkDuplicateName(name);
+			}
+		}
+		return name;
+	}
+
 	@Override
 	public int hashCode() {
 		// It is not expected that these objects ever be put in a hash map
@@ -392,6 +397,24 @@ class DataTypeComponentDB implements InternalDataTypeComponent {
 		return record;
 	}
 
+	/**
+	 * Perform special-case component update that does not result in size or alignment changes. 
+	 * @param name new component name
+	 * @param dt new resolved datatype
+	 * @param cmt new comment
+	 */
+	void update(String name, DataType dt, String cmt) {
+		if (record != null) {
+			// TODO: Need to check field name and throw DuplicateNameException
+			// name = checkFieldName(name);
+			record.setString(ComponentDBAdapter.COMPONENT_FIELD_NAME_COL, name);
+			record.setLongValue(ComponentDBAdapter.COMPONENT_DT_ID_COL,
+				dataMgr.getResolvedID(dt));
+			record.setString(ComponentDBAdapter.COMPONENT_COMMENT_COL, cmt);
+			updateRecord(false);
+		}
+	}
+
 	@Override
 	public void setDataType(DataType newDt) {
 		// intended for internal use only - note exsiting settings should be preserved

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

@@ -179,7 +179,7 @@ class StructureDB extends CompositeDB implements StructureInternal {
 			throw new IllegalArgumentException(e.getMessage(), e);
 		}
 	}
-
+	
 	private DataTypeComponent doAdd(DataType dataType, int length, String name, String comment,
 			boolean validatePackAndNotify)
 			throws DataTypeDependencyException, IllegalArgumentException {
@@ -219,7 +219,9 @@ class StructureDB extends CompositeDB implements StructureInternal {
 				structLength += structureGrowth;
 
 				if (validatePackAndNotify) {
-					repack(false, false); // may not recognize length change
+					if (isPackingEnabled()) {
+						repack(false, false); // may not recognize length change
+					}
 					record.setIntValue(CompositeDBAdapter.COMPOSITE_NUM_COMPONENTS_COL,
 						numComponents);
 					record.setIntValue(CompositeDBAdapter.COMPOSITE_LENGTH_COL, structLength);
@@ -1301,6 +1303,53 @@ class StructureDB extends CompositeDB implements StructureInternal {
 		return null;
 	}
 
+	/**
+	 * Perform component replacement(s)
+	 * @param replacedComponents ordered list of components to be replaced
+	 * @param offset offset of component replacement
+	 * @param dataType datatype of component replacement
+	 * @param length length of component replacement
+	 * @param componentName name of component replacement (may be null)
+	 * @param comment comment for component replacement (may be null)
+	 * @return new/updated component (may be null if replacement is not a defined component)
+	 * @throws IOException if an IO error occurs
+	 */
+	private DataTypeComponent doComponentReplacement(
+			LinkedList<DataTypeComponentDB> replacedComponents, int offset, DataType dataType,
+			int length, String componentName, String comment)
+			throws IOException {
+
+		// Attempt quick update of a single defined component if possible.
+		// A quick update requires that component characteristics including length, offset, 
+		// and alignment (if packed) not change.  All other situations generally require a
+		// repack.
+		DataTypeComponentDB oldComponent = replacedComponents.get(0);
+		DataType oldDt = oldComponent.getDataType();
+		if (replacedComponents.size() == 1 &&
+			oldDt != DEFAULT &&
+			dataType != DEFAULT &&
+			length == oldComponent.getLength() &&
+			offset == oldComponent.getOffset() &&
+			(!isPackingEnabled() || dataType.getAlignment() == oldDt.getAlignment())) {
+
+			oldComponent.update(componentName, dataType, comment);
+
+			setLastChangeTime(System.currentTimeMillis());
+			dataMgr.dataTypeChanged(this, false);
+			return oldComponent;
+		}
+
+		DataTypeComponent replaceComponent = replaceComponents(replacedComponents, dataType,
+			offset, length, componentName, comment);
+
+		repack(false, false);
+		record.setIntValue(CompositeDBAdapter.COMPOSITE_LENGTH_COL, structLength);
+		compositeAdapter.updateRecord(record, true);
+		notifySizeChanged(false);
+
+		return replaceComponent;
+	}
+
 	@Override
 	public final DataTypeComponent replace(int ordinal, DataType dataType, int length)
 			throws IllegalArgumentException {
@@ -1308,9 +1357,8 @@ class StructureDB extends CompositeDB implements StructureInternal {
 	}
 
 	@Override
-	public DataTypeComponent replace(int ordinal, DataType dataType, int length,
-			String componentName,
-			String comment) {
+	public DataTypeComponent replace(int ordinal, DataType dataType, int length, 
+			String componentName, String comment) {
 		lock.acquire();
 		try {
 			checkDeleted();
@@ -1399,14 +1447,8 @@ class StructureDB extends CompositeDB implements StructureInternal {
 				replacedComponents.add(origDtc);
 			}
 
-			DataTypeComponent replaceComponent =
-				replaceComponents(replacedComponents, dataType, offset, length, componentName,
-					comment);
-
-			repack(false, false);
-			record.setIntValue(CompositeDBAdapter.COMPOSITE_LENGTH_COL, structLength);
-			compositeAdapter.updateRecord(record, true);
-			notifySizeChanged(false);
+			DataTypeComponent replaceComponent = doComponentReplacement(replacedComponents, offset,
+				dataType, length, componentName, comment);
 
 			return replaceComponent != null ? replaceComponent : getComponent(ordinal);
 		}
@@ -1423,8 +1465,8 @@ class StructureDB extends CompositeDB implements StructureInternal {
 	}
 
 	@Override
-	public DataTypeComponent replaceAtOffset(int offset, DataType dataType, int length, String name,
-			String comment) throws IllegalArgumentException {
+	public DataTypeComponent replaceAtOffset(int offset, DataType dataType, int length,
+			String componentName, String comment) throws IllegalArgumentException {
 		if (offset < 0) {
 			throw new IllegalArgumentException("Offset cannot be negative.");
 		}
@@ -1452,12 +1494,12 @@ class StructureDB extends CompositeDB implements StructureInternal {
 				// defined component found - advance to last one containing offset
 				index = advanceToLastComponentContainingOffset(index, offset);
 				origDtc = components.get(index);
-
+				
 				// case 1: only defined component(s) at offset are zero-length
 				if (origDtc.getLength() == 0) {
 					if (isPackingEnabled()) {
 						// if packed: insert after zero-length component
-						return insert(index + 1, dataType, length, name, comment);
+						return insert(index + 1, dataType, length, componentName, comment);
 					}
 					// if non-packed: replace undefined component which immediately follows the
 					// zero-length component
@@ -1490,7 +1532,7 @@ class StructureDB extends CompositeDB implements StructureInternal {
 				if (isPackingEnabled()) {
 					// case 4: if replacing padding for packed structure perform insert at correct
 					// ordinal
-					return insert(index, dataType, length, name, comment);
+					return insert(index, dataType, length, componentName, comment);
 				}
 
 				// case 5: replace undefined component at offset - compute undefined component to
@@ -1509,14 +1551,9 @@ class StructureDB extends CompositeDB implements StructureInternal {
 			}
 
 			length = getPreferredComponentLength(dataType, length);
-
-			DataTypeComponent replaceComponent =
-				replaceComponents(replacedComponents, dataType, offset, length, name, comment);
-
-			repack(false, false);
-			record.setIntValue(CompositeDBAdapter.COMPOSITE_LENGTH_COL, structLength);
-			compositeAdapter.updateRecord(record, true);
-			notifySizeChanged(false);
+			
+			DataTypeComponent replaceComponent = doComponentReplacement(replacedComponents, offset,
+				dataType, length, componentName, comment);
 
 			return replaceComponent != null ? replaceComponent : getComponentContaining(offset);
 		}
@@ -1579,6 +1616,9 @@ class StructureDB extends CompositeDB implements StructureInternal {
 	void doReplaceWith(StructureInternal struct, boolean notify)
 			throws DataTypeDependencyException, IOException {
 
+		int oldAlignment = getAlignment();
+		int oldLength = structLength;
+
 		// pre-resolved component types to catch dependency issues early
 		DataTypeComponent[] otherComponents = struct.getDefinedComponents();
 		DataType[] resolvedDts = new DataType[otherComponents.length];
@@ -1598,25 +1638,39 @@ class StructureDB extends CompositeDB implements StructureInternal {
 		structAlignment = -1;
 		computedAlignment = -1;
 
-		doSetPackingAndAlignment(struct); // updates timestamp
+		doSetPackingAndAlignment(struct);
 
-		if (struct.isPackingEnabled()) {
-			doReplaceWithPacked(struct, resolvedDts);
+		if (struct.getDataTypeManager() == dataMgr) {
+			// assume layout is good and can just be copied without packing
+			// NOTE: this optimization has been added to StructureDB only
+			doCopy(struct, otherComponents, resolvedDts);
 		}
 		else {
-			doReplaceWithNonPacked(struct, resolvedDts);
+			if (struct.isPackingEnabled()) {
+				doReplaceWithPacked(struct, resolvedDts);
+			}
+			else {
+				doReplaceWithNonPacked(struct, resolvedDts);
+			}
+			repack(false, false);
 		}
 
-		repack(false, false);
-
 		// must force record update
 		record.setIntValue(CompositeDBAdapter.COMPOSITE_NUM_COMPONENTS_COL, numComponents);
 		record.setIntValue(CompositeDBAdapter.COMPOSITE_LENGTH_COL, structLength);
 		record.setIntValue(CompositeDBAdapter.COMPOSITE_ALIGNMENT_COL, structAlignment);
-		compositeAdapter.updateRecord(record, notify);
+		compositeAdapter.updateRecord(record, true); // updates timestamp
 
 		if (notify) {
-			notifySizeChanged(false);
+			if (structLength != oldLength) {
+				notifySizeChanged(false);
+			}
+			else if (structAlignment != oldAlignment) {
+				notifyAlignmentChanged(false);
+			}
+			else {
+				dataMgr.dataTypeChanged(this, false);
+			}
 		}
 
 		if (pointerPostResolveRequired) {
@@ -1682,7 +1736,28 @@ class StructureDB extends CompositeDB implements StructureInternal {
 				new DataTypeComponentDB(dataMgr, componentAdapter, this, rec);
 			components.add(newDtc);
 		}
-		repack(false, false);
+	}
+
+	private void doCopy(Structure struct, DataTypeComponent[] definedComponents,
+			DataType[] resolvedDts) throws IOException {
+
+		// simple replication of struct
+		structLength = struct.isZeroLength() ? 0 : struct.getLength();
+		numComponents = struct.getNumComponents();
+		structAlignment = struct.getAlignment();
+
+		DataTypeComponent[] otherComponents = struct.getDefinedComponents();
+		for (int i = 0; i < otherComponents.length; i++) {
+			DataTypeComponent dtc = otherComponents[i];
+			DataType dt = resolvedDts[i]; // ancestry check already performed by caller
+			DBRecord rec =
+				componentAdapter.createRecord(dataMgr.getResolvedID(dt), key, dtc.getLength(),
+				dtc.getOrdinal(), dtc.getOffset(), dtc.getFieldName(), dtc.getComment());
+			dt.addParent(this);
+			DataTypeComponentDB newDtc =
+				new DataTypeComponentDB(dataMgr, componentAdapter, this, rec);
+			components.add(newDtc);
+		}
 	}
 
 	@Override
@@ -1950,6 +2025,9 @@ class StructureDB extends CompositeDB implements StructureInternal {
 	}
 
 	private void shiftOffsets(int definedComponentIndex, int deltaOrdinal, int deltaOffset) {
+		if (deltaOffset == 0 && deltaOrdinal == 0) {
+			return;
+		}
 		for (int i = definedComponentIndex; i < components.size(); i++) {
 			DataTypeComponentDB dtc = components.get(i);
 			shiftOffset(dtc, deltaOrdinal, deltaOffset);

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

@@ -344,7 +344,7 @@ class UnionDB extends CompositeDB implements UnionInternal {
 			doAdd(resolvedDts[i], dtc.getLength(), dtc.getFieldName(), dtc.getComment(), false);
 		}
 
-		repack(false, false);
+		repack(false, false); // updates timestamp
 
 		if (notify) {
 			notifySizeChanged(false); // assume size and/or alignment changed

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

@@ -128,19 +128,7 @@ public class DataTypeComponentImpl implements InternalDataTypeComponent, Seriali
 
 	@Override
 	public void setFieldName(String name) throws DuplicateNameException {
-		if (name != null) {
-			name = name.trim();
-			if (name.length() == 0 || name.equals(getDefaultFieldName())) {
-				name = null;
-			}
-			else {
-				if (name.equals(this.fieldName)) {
-					return;
-				}
-				checkDuplicateName(name);
-			}
-		}
-		this.fieldName = name;
+		this.fieldName = checkFieldName(name);
 	}
 
 	private void checkDuplicateName(String name) throws DuplicateNameException {
@@ -155,6 +143,19 @@ public class DataTypeComponentImpl implements InternalDataTypeComponent, Seriali
 		}
 	}
 
+	private String checkFieldName(String name) throws DuplicateNameException {
+		if (name != null) {
+			name = name.trim();
+			if (name.length() == 0 || name.equals(getDefaultFieldName())) {
+				name = null;
+			}
+			else {
+				checkDuplicateName(name);
+			}
+		}
+		return name;
+	}
+
 	public static void checkDefaultFieldName(String fieldName) throws DuplicateNameException {
 		if (fieldName.startsWith(DataTypeComponent.DEFAULT_FIELD_NAME_PREFIX)) {
 			String subname =
@@ -186,6 +187,20 @@ public class DataTypeComponentImpl implements InternalDataTypeComponent, Seriali
 		return parent;
 	}
 
+	/**
+	 * Perform special-case component update that does not result in size or alignment changes. 
+	 * @param name new component name
+	 * @param dt new resolved datatype
+	 * @param cmt new comment
+	 */
+	void update(String name, DataType dt, String cmt) {
+		// TODO: Need to check field name and throw DuplicateNameException
+		// this.fieldName =  = checkFieldName(name);
+		this.fieldName = name;
+		this.dataType = dt;
+		this.comment = cmt;
+	}
+
 	@Override
 	public void update(int ordinal, int offset, int length) {
 		this.ordinal = ordinal;

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

@@ -44,7 +44,8 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 
 	/**
 	 * Construct a new structure with the given name and length. The root category will be used.
-	 * 
+	 * NOTE: A constructor form which accepts a {@link DataTypeManager} should be used when possible
+	 * since there may be performance benefits during datatype resolution.
 	 * @param name the name of the new structure
 	 * @param length the initial size of the structure in bytes. If 0 is specified the structure
 	 *            will report its length as 1 and {@link #isNotYetDefined()} will return true.
@@ -69,7 +70,8 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 
 	/**
 	 * Construct a new structure with the given name and length within the specified categry path.
-	 * 
+	 * NOTE: A constructor form which accepts a {@link DataTypeManager} should be used when possible
+	 * since there may be performance benefits during datatype resolution.
 	 * @param path the category path indicating where this data type is located.
 	 * @param name the name of the new structure
 	 * @param length the initial size of the structure in bytes. If 0 is specified the structure
@@ -417,6 +419,9 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 	}
 
 	private void shiftOffsets(int index, int deltaOrdinal, int deltaOffset) {
+		if (deltaOffset == 0 && deltaOrdinal == 0) {
+			return;
+		}
 		for (int i = index; i < components.size(); i++) {
 			DataTypeComponentImpl dtc = components.get(i);
 			shiftOffset(dtc, deltaOrdinal, deltaOffset);
@@ -581,14 +586,9 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 			dtc = new DataTypeComponentImpl(DataType.DEFAULT, this, 1, numComponents, structLength);
 		}
 		else {
-
-			int offset = structLength;
-			int ordinal = numComponents;
-
 			int componentLength = getPreferredComponentLength(dataType, length);
-
-			dtc = new DataTypeComponentImpl(dataType, this, componentLength, ordinal, offset,
-				componentName, comment);
+			dtc = new DataTypeComponentImpl(dataType, this, componentLength, numComponents,
+				structLength, componentName, comment);
 			dataType.addParent(this);
 			components.add(dtc);
 		}
@@ -602,7 +602,9 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 		structLength += structureGrowth;
 
 		if (packAndNotify) {
-			repack(false);
+			if (isPackingEnabled()) {
+				repack(false);
+			}
 			notifySizeChanged();
 		}
 		return dtc;
@@ -1216,7 +1218,6 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 			components.add(new DataTypeComponentImpl(dt, this, length, dtc.getOrdinal(),
 				dtc.getOffset(), dtc.getFieldName(), dtc.getComment()));
 		}
-		repack(false);
 	}
 
 	@Override
@@ -1362,6 +1363,46 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 		return comps;
 	}
 
+	/**
+	 * Perform component replacement(s)
+	 * @param replacedComponents ordered list of components to be replaced
+	 * @param offset offset of component replacement
+	 * @param dataType datatype of component replacement
+	 * @param length length of component replacement
+	 * @param componentName name of component replacement (may be null)
+	 * @param comment comment for component replacement (may be null)
+	 * @return new/updated component (may be null if replacement is not a defined component)
+	 */
+	private DataTypeComponent doComponentReplacement(
+			LinkedList<DataTypeComponentImpl> replacedComponents, int offset, DataType dataType,
+			int length, String componentName, String comment) {
+
+		// Attempt quick update of a single defined component if possible.
+		// A quick update requires that component characteristics including length, offset, 
+		// and alignment (if packed) not change.  All other situations generally require a
+		// repack.
+		DataTypeComponentImpl oldComponent = replacedComponents.get(0);
+		DataType oldDt = oldComponent.getDataType();
+		if (replacedComponents.size() == 1 &&
+			oldDt != DEFAULT &&
+			dataType != DEFAULT &&
+			length == oldComponent.getLength() &&
+			offset == oldComponent.getOffset() &&
+			(!isPackingEnabled() || dataType.getAlignment() == oldDt.getAlignment())) {
+
+			oldComponent.update(componentName, dataType, comment);
+			return oldComponent;
+		}
+
+		DataTypeComponent replaceComponent = replaceComponents(replacedComponents, dataType,
+			offset, length, componentName, comment);
+
+		repack(false);
+		notifySizeChanged();
+
+		return replaceComponent;
+	}
+
 	@Override
 	public final DataTypeComponent replace(int index, DataType dataType, int length) {
 		return replace(index, dataType, length, null, null);
@@ -1453,18 +1494,15 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 			replacedComponents.add(origDtc);
 		}
 
-		DataTypeComponent replaceComponent =
-			replaceComponents(replacedComponents, dataType, offset, length, componentName, comment);
-
-		repack(false);
-		notifySizeChanged();
+		DataTypeComponent replaceComponent = doComponentReplacement(replacedComponents, offset,
+			dataType, length, componentName, comment);
 
 		return replaceComponent != null ? replaceComponent : getComponent(ordinal);
 	}
 
 	@Override
 	public DataTypeComponent replaceAtOffset(int offset, DataType dataType, int length,
-			String name, String comment) throws IllegalArgumentException {
+			String componentName, String comment) throws IllegalArgumentException {
 		if (offset < 0) {
 			throw new IllegalArgumentException("Offset cannot be negative.");
 		}
@@ -1492,7 +1530,7 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 			if (origDtc.getLength() == 0) {
 				if (isPackingEnabled()) {
 					// if packed: insert after zero-length component 
-					return insert(index + 1, dataType, length, name, comment);
+					return insert(index + 1, dataType, length, componentName, comment);
 				}
 				// if non-packed: replace undefined component which immediately follows the zero-length component
 				replacedComponents.add(new DataTypeComponentImpl(DataType.DEFAULT, this, 1,
@@ -1522,7 +1560,7 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 
 			if (isPackingEnabled()) {
 				// case 4: if replacing padding for packed struction perform insert at correct ordinal
-				return insert(index, dataType, length, name, comment);
+				return insert(index, dataType, length, componentName, comment);
 			}
 
 			// case 5: replace undefined component at offset - compute undefined component to be replaced
@@ -1541,11 +1579,8 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 
 		length = getPreferredComponentLength(dataType, length);
 
-		DataTypeComponent replaceComponent =
-			replaceComponents(replacedComponents, dataType, offset, length, name, comment);
-
-		repack(false);
-		notifySizeChanged();
+		DataTypeComponent replaceComponent = doComponentReplacement(replacedComponents, offset,
+			dataType, length, componentName, comment);
 
 		return replaceComponent != null ? replaceComponent : getComponentContaining(offset);
 	}

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

@@ -37,6 +37,8 @@ public class UnionDataType extends CompositeDataTypeImpl implements UnionInterna
 	 * Construct a new empty union with the given name within the
 	 * specified categry path.  An empty union will report its length as 1 and 
 	 * {@link #isNotYetDefined()} will return true.
+	 * NOTE: A constructor form which accepts a {@link DataTypeManager} should be used when possible
+	 * since there may be performance benefits during datatype resolution.
 	 * @param path the category path indicating where this data type is located.
 	 * @param name the name of the new union
 	 */
@@ -82,7 +84,9 @@ public class UnionDataType extends CompositeDataTypeImpl implements UnionInterna
 	}
 
 	/**
-	 * Construct a new UnionDataType
+	 * Construct a new UnionDataType.
+	 * NOTE: A constructor form which accepts a {@link DataTypeManager} should be used when possible
+	 * since there may be performance benefits during datatype resolution.
 	 * @param name the name of this dataType
 	 */
 	public UnionDataType(String name) {