From 277a0f5843a8fcd04e876e02e944a37df9f814c6 Mon Sep 17 00:00:00 2001 From: ghidra1 Date: Wed, 9 Nov 2022 12:12:14 -0500 Subject: [PATCH] GP-2777 Structure build-up and resolution performance improvements --- .../util/bin/format/pe/cli/blobs/CliBlob.java | 13 +- .../format/pe/cli/streams/CliStreamBlob.java | 6 +- .../program/database/data/CompositeDB.java | 5 +- .../database/data/DataTypeComponentDB.java | 41 +++-- .../program/database/data/StructureDB.java | 148 +++++++++++++----- .../ghidra/program/database/data/UnionDB.java | 2 +- .../model/data/DataTypeComponentImpl.java | 41 +++-- .../program/model/data/StructureDataType.java | 83 +++++++--- .../program/model/data/UnionDataType.java | 6 +- 9 files changed, 255 insertions(+), 90 deletions(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/cli/blobs/CliBlob.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/cli/blobs/CliBlob.java index 70fdf198c0..6ab46a5fbe 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/cli/blobs/CliBlob.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/cli/blobs/CliBlob.java @@ -159,7 +159,18 @@ public class CliBlob implements StructConverter { @Override public DataType toDataType() { - Structure struct = new StructureDataType(new CategoryPath(PATH), "Blob_" + getName(), 0); + return toDataType(null); + } + + /** + * Create CLI Blob structure. + * NOTE: This form is provided to reduce resolution time when target datatype manager is known. + * @param dtm datatype manager or null if target datatype manager is unknown. + * @return blob structure + */ + public DataType toDataType(DataTypeManager dtm) { + Structure struct = + new StructureDataType(new CategoryPath(PATH), "Blob_" + getName(), 0, dtm); struct.add(getSizeDataType(), "Size", "coded integer - blob size"); struct.add(getContentsDataType(), getContentsName(), getContentsComment()); return struct; diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/cli/streams/CliStreamBlob.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/cli/streams/CliStreamBlob.java index 87b3ea13f2..05df8151af 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/cli/streams/CliStreamBlob.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/cli/streams/CliStreamBlob.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. @@ -122,7 +122,7 @@ public class CliStreamBlob extends CliAbstractStream { // Make sure the old blob has the same size as the new blob DataType oldBlobDataType = oldBlobDataComponent.getDataType(); - DataType newBlobDataType = updatedBlob.toDataType(); + DataType newBlobDataType = updatedBlob.toDataType(program.getDataTypeManager()); if (oldBlobDataType.getLength() != newBlobDataType.getLength()) { Msg.error(this, "Cannot replace existing blob at address " + addr + " with " + diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/CompositeDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/CompositeDB.java index 28b5aee7a8..291dc5120e 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/CompositeDB.java +++ b/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 diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeComponentDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeComponentDB.java index e9ae27d789..67d3dc89f5 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeComponentDB.java +++ b/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 diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/StructureDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/StructureDB.java index 5acef9ca97..6fbbece0c1 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/StructureDB.java +++ b/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 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); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/UnionDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/UnionDB.java index 0334ac0ba2..dd8869a237 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/UnionDB.java +++ b/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 diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeComponentImpl.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeComponentImpl.java index afdc2ee086..d979629820 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeComponentImpl.java +++ b/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; diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/StructureDataType.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/StructureDataType.java index a708df4e27..106ca77109 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/StructureDataType.java +++ b/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 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); } diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/UnionDataType.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/UnionDataType.java index 7ac6b3aa4f..a4e14ad8a2 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/UnionDataType.java +++ b/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) {