From 87cc46efa20c90a0d57eb65b340178dd29f94ef6 Mon Sep 17 00:00:00 2001 From: ghidra1 Date: Wed, 22 Dec 2021 15:12:50 -0500 Subject: [PATCH] GP-1618 Improved DataType.getParents to avoid returning duplicates for DB case which is used during change notification processing. --- .../DecompilerDataTypeReferenceFinder.java | 3 +- .../program/database/data/DataTypeDB.java | 9 +-- .../database/data/DataTypeManagerDB.java | 76 +++++++++++++------ .../database/data/ParentChildAdapter.java | 11 ++- .../data/ParentChildDBAdapterNoTable.java | 5 +- .../database/data/ParentChildDBAdapterV0.java | 8 +- .../program/model/data/AbstractDataType.java | 3 +- .../ghidra/program/model/data/DataType.java | 11 ++- .../program/model/data/DataTypeImpl.java | 5 +- .../program/model/data/DataTypeManager.java | 4 + .../program/model/data/StubDataType.java | 3 +- 11 files changed, 91 insertions(+), 47 deletions(-) diff --git a/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/DecompilerDataTypeReferenceFinder.java b/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/DecompilerDataTypeReferenceFinder.java index 8dcaa49b5b..6ab7e20900 100644 --- a/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/DecompilerDataTypeReferenceFinder.java +++ b/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/DecompilerDataTypeReferenceFinder.java @@ -174,8 +174,7 @@ public class DecompilerDataTypeReferenceFinder implements DataTypeReferenceFinde types.add(dt); - DataType[] parents = dt.getParents(); - for (DataType parent : parents) { + for (DataType parent : dt.getParents()) { buildTypeLineage(parent, types); } } diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeDB.java index d286ca933d..00f405135d 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeDB.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeDB.java @@ -17,8 +17,7 @@ package ghidra.program.database.data; import java.io.IOException; import java.net.URL; -import java.nio.ByteBuffer; -import java.util.List; +import java.util.Collection; import db.DBRecord; import ghidra.docking.settings.Settings; @@ -452,10 +451,8 @@ abstract class DataTypeDB extends DatabaseObject implements DataType { } @Override - public DataType[] getParents() { - List parents = dataMgr.getParentDataTypes(key); - DataType[] array = new DataType[parents.size()]; - return parents.toArray(array); + public Collection getParents() { + return dataMgr.getParentDataTypes(key); } @Override diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeManagerDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeManagerDB.java index 1a5743f02b..92e29e9ee2 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeManagerDB.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeManagerDB.java @@ -1350,8 +1350,7 @@ abstract public class DataTypeManagerDB implements DataTypeManager { private void replaceUsesInOtherDataTypes(DataType existingDt, DataType newDt) { if (existingDt instanceof DataTypeDB) { - DataType[] dts = existingDt.getParents(); - for (DataType dt : dts) { + for (DataType dt : existingDt.getParents()) { dt.dataTypeReplaced(existingDt, newDt); } } @@ -3482,12 +3481,14 @@ abstract public class DataTypeManagerDB implements DataTypeManager { } } - List getParentDataTypes(long childID) { + List getParentDataTypes(long dataTypeId) { lock.acquire(); try { - long[] ids = parentChildAdapter.getParentIds(childID); + Set parentIds = parentChildAdapter.getParentIds(dataTypeId); + // NOTE: Use of Set for containing datatypes is avoided due to the excessive + // overhead of DataType.equals List dts = new ArrayList<>(); - for (long id : ids) { + for (long id : parentIds) { DataType dt = getDataType(id); if (dt == null) { // cleanup invalid records for missing parent @@ -3498,7 +3499,6 @@ abstract public class DataTypeManagerDB implements DataTypeManager { } } return dts; - } catch (IOException e) { dbError(e); @@ -3509,6 +3509,52 @@ abstract public class DataTypeManagerDB implements DataTypeManager { return null; } + /** + * NOTE: method use should be avoided since use of a Set for containing + * datatypes can cause use of DataType.equals method which should + * be avoided for performance reasons. + * @param dataTypeId id of datatype whose parents should be found + * @return set of known parent datatypes + */ + @Deprecated + Set getParentDataTypeSet(long dataTypeId) { + lock.acquire(); + try { + Set parentIds = parentChildAdapter.getParentIds(dataTypeId); + Set dts = new HashSet<>(); + for (long id : parentIds) { + DataType dt = getDataType(id); + if (dt == null) { + // cleanup invalid records for missing parent + attemptRecordRemovalForParent(id); + } + else { + dts.add(dt); + } + } + return dts; + } + catch (IOException e) { + dbError(e); + } + finally { + lock.release(); + } + return null; + } + + @Override + public Set getDataTypesContaining(DataType dataType) { + if (dataType instanceof DataTypeDB) { + DataTypeDB dataTypeDb = (DataTypeDB) dataType; + if (dataTypeDb.getDataTypeManager() != this) { + return Set.of(); + } + return getParentDataTypeSet(dataTypeDb.getKey()); + } + return Set.of(); + } + private void attemptRecordRemovalForParent(long parentKey) throws IOException { lock.acquire(); try { @@ -3521,24 +3567,6 @@ abstract public class DataTypeManagerDB implements DataTypeManager { } } - @Override - public Set getDataTypesContaining(DataType dataType) { - Set set = new HashSet<>(); - if (dataType instanceof DataTypeDB) { - long dataTypeID = ((DataTypeDB) dataType).getKey(); - try { - long[] ids = parentChildAdapter.getParentIds(dataTypeID); - for (long id : ids) { - set.add(getDataType(id)); - } - } - catch (IOException e) { - dbError(e); - } - } - return set; - } - @Override public Pointer getPointer(DataType dt) { return new PointerDataType(dt, -1, this); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParentChildAdapter.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParentChildAdapter.java index 4e60ee23c8..6b2b8daabb 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParentChildAdapter.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParentChildAdapter.java @@ -16,6 +16,7 @@ package ghidra.program.database.data; import java.io.IOException; +import java.util.Set; import db.DBConstants; import db.DBHandle; @@ -71,7 +72,15 @@ abstract class ParentChildAdapter { abstract void removeRecord(long parentID, long childID) throws IOException; - abstract long[] getParentIds(long childID) throws IOException; + /** + * Get the unique set of parent ID associated with the specified childID. + * Since composite parents may have duplicate parent-child records, this method + * avoids returning the same parent more than once. + * @param childID child datatype ID + * @return set of parent datatype IDs + * @throws IOException if a DB IO error occurs + */ + abstract Set getParentIds(long childID) throws IOException; abstract void removeAllRecordsForParent(long parentID) throws IOException; diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParentChildDBAdapterNoTable.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParentChildDBAdapterNoTable.java index e3ce350572..a612895a5e 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParentChildDBAdapterNoTable.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParentChildDBAdapterNoTable.java @@ -16,6 +16,7 @@ package ghidra.program.database.data; import java.io.IOException; +import java.util.Set; import db.DBHandle; import ghidra.util.exception.VersionException; @@ -43,8 +44,8 @@ class ParentChildDBAdapterNoTable extends ParentChildAdapter { } @Override - long[] getParentIds(long childID) throws IOException { - return new long[0]; + Set getParentIds(long childID) throws IOException { + return Set.of(); } @Override diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParentChildDBAdapterV0.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParentChildDBAdapterV0.java index 1c52bc1609..d2278d3f40 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParentChildDBAdapterV0.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParentChildDBAdapterV0.java @@ -16,6 +16,8 @@ package ghidra.program.database.data; import java.io.IOException; +import java.util.HashSet; +import java.util.Set; import db.*; import ghidra.util.exception.VersionException; @@ -73,12 +75,12 @@ class ParentChildDBAdapterV0 extends ParentChildAdapter { } @Override - long[] getParentIds(long childID) throws IOException { + Set getParentIds(long childID) throws IOException { Field[] ids = table.findRecords(new LongField(childID), CHILD_COL); - long[] parentIds = new long[ids.length]; + Set parentIds = new HashSet<>(ids.length); for (int i = 0; i < ids.length; i++) { DBRecord rec = table.getRecord(ids[i]); - parentIds[i] = rec.getLongValue(PARENT_COL); + parentIds.add(rec.getLongValue(PARENT_COL)); } return parentIds; } diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/AbstractDataType.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/AbstractDataType.java index 4ec56dfd0f..34eb720e06 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/AbstractDataType.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/AbstractDataType.java @@ -16,6 +16,7 @@ package ghidra.program.model.data; import java.net.URL; +import java.util.Collection; import ghidra.docking.settings.Settings; import ghidra.program.model.mem.MemBuffer; @@ -170,7 +171,7 @@ public abstract class AbstractDataType implements DataType { } @Override - public DataType[] getParents() { + public Collection getParents() { // not-applicable return null; } diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataType.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataType.java index e80f3ff2ca..12869f88c6 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataType.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataType.java @@ -16,6 +16,7 @@ package ghidra.program.model.data; import java.net.URL; +import java.util.Collection; import ghidra.docking.settings.Settings; import ghidra.docking.settings.SettingsDefinition; @@ -484,11 +485,13 @@ public interface DataType { public void dataTypeNameChanged(DataType dt, String oldName); /** - * Get the parents of this datatype - * - * @return an array of parents of this datatype + * Get the parents of this datatype. + * + * NOTE: This method is intended to be used on a DB-managed datatype only and is not + * fully supported for use with non-DB datatype instances. + * @return parents of this datatype */ - public DataType[] getParents(); + public Collection getParents(); /** * Gets the alignment to be used when aligning this datatype within another datatype. diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeImpl.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeImpl.java index 2d10b8fffb..fc38b7c127 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeImpl.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeImpl.java @@ -120,7 +120,7 @@ public abstract class DataTypeImpl extends AbstractDataType { } @Override - public DataType[] getParents() { + public Collection getParents() { List parents = new ArrayList<>(); Iterator> iterator = parentList.iterator(); while (iterator.hasNext()) { @@ -133,8 +133,7 @@ public abstract class DataTypeImpl extends AbstractDataType { parents.add(dataType); } } - DataType[] array = new DataType[parents.size()]; - return parents.toArray(array); + return parents; } /** diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeManager.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeManager.java index dbfc56759d..129812659b 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeManager.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeManager.java @@ -530,8 +530,12 @@ public interface DataTypeManager { /** * Returns the data types within this data type manager that contain the specified data type. + * The specified dataType must belong to this datatype manager. An empty set will be + * returned for unsupported datatype instances. * @param dataType the data type * @return a set of data types that contain the specified data type. + * @deprecated the method {@link DataType#getParents()} should be used instead. + * Use of {@link Set} implementations for containing DataTypes is also inefficient. */ public Set getDataTypesContaining(DataType dataType); } diff --git a/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/model/data/StubDataType.java b/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/model/data/StubDataType.java index 778b462a45..a4ee1c60a3 100644 --- a/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/model/data/StubDataType.java +++ b/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/model/data/StubDataType.java @@ -17,6 +17,7 @@ package ghidra.program.model.data; import java.net.URL; import java.nio.ByteBuffer; +import java.util.Collection; import ghidra.docking.settings.Settings; import ghidra.docking.settings.SettingsDefinition; @@ -254,7 +255,7 @@ public class StubDataType implements DataType { } @Override - public DataType[] getParents() { + public Collection getParents() { throw new UnsupportedOperationException(); }