From 67fb7857c5b6a506a25a98bbaffd4a0e40b063ea Mon Sep 17 00:00:00 2001 From: ghidra1 Date: Mon, 9 Nov 2020 17:10:19 -0500 Subject: [PATCH] GP-260 Added ability to fixup composites due to primitive datatype size change such as a data origanization may cause. --- .../FixupCompositeDataTypesScript.java | 47 ++++++++ .../actions/AlignAllDataTypesAction.java | 3 +- .../util/bin/format/pdb/ApplyDataTypes.java | 5 +- .../program/database/data/CompositeDB.java | 31 ++--- .../database/data/DataTypeManagerDB.java | 111 ++++++++++++++++-- .../program/database/data/StructureDB.java | 102 ++++++++++++---- .../ghidra/program/database/data/UnionDB.java | 33 +++++- .../ghidra/program/model/listing/Program.java | 2 +- .../database/data/StructureDBTest.java | 24 ++++ certification.local.manifest | 1 - 10 files changed, 304 insertions(+), 55 deletions(-) create mode 100644 Ghidra/Features/Base/ghidra_scripts/FixupCompositeDataTypesScript.java diff --git a/Ghidra/Features/Base/ghidra_scripts/FixupCompositeDataTypesScript.java b/Ghidra/Features/Base/ghidra_scripts/FixupCompositeDataTypesScript.java new file mode 100644 index 0000000000..d58ce6ec9c --- /dev/null +++ b/Ghidra/Features/Base/ghidra_scripts/FixupCompositeDataTypesScript.java @@ -0,0 +1,47 @@ +/* ### + * 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. + */ +// +// Fixes up all composite datatypes within the current program to account for +// any changes to primitive datatype sizes or alignment rules as defined +// by the associated data organization. +// +// This script requires exclusive access to the program to avoid the possibilty +// of excessive change conflicts. +// +// This script can be run multiple times without harm +//@category Data Types +import ghidra.app.script.GhidraScript; +import ghidra.program.database.data.DataTypeManagerDB; + +public class FixupCompositeDataTypesScript extends GhidraScript { + + @Override + protected void run() throws Exception { + + if (currentProgram == null) { + return; + } + + if (!currentProgram.hasExclusiveAccess()) { + popup("This script requires an exclusive checkout of the program"); + return; + } + + DataTypeManagerDB dtm = (DataTypeManagerDB) currentProgram.getDataTypeManager(); + dtm.fixupComposites(monitor); + } + +} diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/actions/AlignAllDataTypesAction.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/actions/AlignAllDataTypesAction.java index 797fb1e241..8abbd3163f 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/actions/AlignAllDataTypesAction.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/actions/AlignAllDataTypesAction.java @@ -89,6 +89,7 @@ public class AlignAllDataTypesAction extends DockingAction { "Are you sure you want to align all of the data types in " + dataTypeManager.getName() + "?\nBoth structures and unions that are currently unaligned will become aligned.\n" + + "This could cause component offsets to change and datatype sizes to change.\n" + "Do you want to continue?", "Continue", OptionDialog.WARNING_MESSAGE); if (result == OptionDialog.CANCEL_OPTION) { return; @@ -125,7 +126,7 @@ public class AlignAllDataTypesAction extends DockingAction { private void alignEachStructure(DataTypeManager dataTypeManager, DataOrganization dataOrganization) { - Iterator allComposites = dataTypeManager.getAllComposites(); + Iterator allComposites = dataTypeManager.getAllComposites(); while (allComposites.hasNext()) { Composite composite = allComposites.next(); composite.setInternallyAligned(true); diff --git a/Ghidra/Features/PDB/src/main/java/ghidra/app/util/bin/format/pdb/ApplyDataTypes.java b/Ghidra/Features/PDB/src/main/java/ghidra/app/util/bin/format/pdb/ApplyDataTypes.java index c25b847375..c127de0bee 100644 --- a/Ghidra/Features/PDB/src/main/java/ghidra/app/util/bin/format/pdb/ApplyDataTypes.java +++ b/Ghidra/Features/PDB/src/main/java/ghidra/app/util/bin/format/pdb/ApplyDataTypes.java @@ -24,7 +24,6 @@ import ghidra.app.util.bin.format.pdb.PdbParser.PdbXmlMember; import ghidra.app.util.importer.MessageLog; import ghidra.graph.*; import ghidra.graph.algo.GraphNavigator; -import ghidra.graph.jung.JungDirectedGraph; import ghidra.program.model.data.Composite; import ghidra.program.model.data.DataType; import ghidra.program.model.symbol.SymbolUtilities; @@ -67,8 +66,8 @@ public class ApplyDataTypes { private List getCompositeDefinitionsInPostDependencyOrder( TaskMonitor monitor) { - JungDirectedGraph> graph = - new JungDirectedGraph<>(); + GDirectedGraph> graph = + GraphFactory.createDirectedGraph(); for (CompositeDefinition compositeDefinition : compositeQueue.values()) { graph.addVertex(compositeDefinition); for (PdbMember m : compositeDefinition.memberList) { 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 73832131d0..c34db6f84f 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 @@ -69,26 +69,20 @@ abstract class CompositeDB extends DataTypeDB implements Composite { protected abstract void initialize(); /** - * Get the preferred length for a new component. For Unions and internally - * 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. - * + * Get the preferred length for a new component. Constraining length of fixed-length datatype + * may not be sustainable in response to datatype size changes over time. * @param dataType new component datatype - * @param length constrained length or -1 to force use of dataType size. - * Dynamic types such as string must have a positive length - * specified. + * @param length specified length required for Dynamic types such as string + * which must have a positive length specified. * @return preferred component length */ protected int getPreferredComponentLength(DataType dataType, int length) { - if ((isInternallyAligned() || (this instanceof Union)) && !(dataType instanceof Dynamic)) { - length = -1; // force use of datatype size + if (length > 0 && (dataType instanceof Composite) && + ((Composite) dataType).isNotYetDefined()) { + return length; } int dtLength = dataType.getLength(); - if (length <= 0) { - length = dtLength; - } - else if (dtLength > 0 && dtLength < length) { + if (dtLength > 0) { length = dtLength; } if (length <= 0) { @@ -789,4 +783,13 @@ abstract class CompositeDB extends DataTypeDB implements Composite { } return " pack(" + packingValue + ")"; } + + /** + * Perform any neccessary component adjustments based on + * sizes and alignment of components differing from their + * specification which may be influenced by the data organization. + * If this composite changes parents will not be + * notified - handling this is the caller's responsibility. + */ + protected abstract void fixupComponents(); } 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 ec17e24dbc..52a3fe12cf 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 @@ -28,6 +28,8 @@ import generic.jar.ResourceFile; import ghidra.app.plugin.core.datamgr.archive.BuiltInSourceArchive; import ghidra.framework.store.db.PackedDBHandle; import ghidra.framework.store.db.PackedDatabase; +import ghidra.graph.*; +import ghidra.graph.algo.GraphNavigator; import ghidra.program.database.*; import ghidra.program.database.map.AddressMap; import ghidra.program.model.address.Address; @@ -2711,7 +2713,7 @@ abstract public class DataTypeManagerDB implements DataTypeManager { class StructureIterator implements Iterator { private RecordIterator it; - private Structure nextStruct; + private StructureDB nextStruct; StructureIterator() throws IOException { it = compositeAdapter.getRecords(); @@ -2731,9 +2733,9 @@ abstract public class DataTypeManagerDB implements DataTypeManager { } @Override - public Structure next() { + public StructureDB next() { if (hasNext()) { - Structure s = nextStruct; + StructureDB s = nextStruct; nextStruct = null; return s; } @@ -2746,7 +2748,7 @@ abstract public class DataTypeManagerDB implements DataTypeManager { Record rec = it.next(); DataType dt = getDataType(rec.getKey(), rec); if (dt instanceof Structure) { - nextStruct = (Structure) dt; + nextStruct = (StructureDB) dt; return; } } @@ -2759,7 +2761,7 @@ abstract public class DataTypeManagerDB implements DataTypeManager { class CompositeIterator implements Iterator { private RecordIterator it; - private Composite nextComposite; + private CompositeDB nextComposite; CompositeIterator() throws IOException { it = compositeAdapter.getRecords(); @@ -2779,9 +2781,9 @@ abstract public class DataTypeManagerDB implements DataTypeManager { } @Override - public Composite next() { + public CompositeDB next() { if (hasNext()) { - Composite c = nextComposite; + CompositeDB c = nextComposite; nextComposite = null; return c; } @@ -2792,7 +2794,7 @@ abstract public class DataTypeManagerDB implements DataTypeManager { try { if (it.hasNext()) { Record rec = it.next(); - nextComposite = (Composite) getDataType(rec.getKey(), rec); + nextComposite = (CompositeDB) getDataType(rec.getKey(), rec); } } catch (IOException e) { @@ -3787,6 +3789,99 @@ abstract public class DataTypeManagerDB implements DataTypeManager { } } + /** + * Fixup all composites and thier components which may be affected by a data organization + * change include primitive type size changes and alignment changes. It is highly recommended + * that this program be open with exclusive access before invoking this method to avoid + * excessive merge conflicts with other users. + * @param monitor task monitor + * @throws CancelledException if operation is cancelled + */ + public void fixupComposites(TaskMonitor monitor) throws CancelledException { + lock.acquire(); + try { + + // NOTE: Any composite could be indirectly affected by a component size change + // based upon type relationships + + // NOTE: Composites brought in from archive may have incorrect component size + // if not aligned and should not be used to guage a primitive size change + + // Unfortunately parent table does not track use of primitives so a brute + // force search is required. Since all composites must be checked, this + // is combined with the composite graph generation to get ordered list + // of composites for subsequent size change operation. + + List orderedComposites = getAllCompositesInPostDependencyOrder(monitor); + + monitor.setProgress(0); + monitor.setMaximum(orderedComposites.size()); + monitor.setMessage("Updating Datatype Sizes..."); + + int count = 0; + for (CompositeDB c : orderedComposites) { + monitor.checkCanceled(); + c.fixupComponents(); + monitor.setProgress(++count); + } + + } + finally { + lock.release(); + } + } + + /** + * Get composite base type which corresponds to a specified datatype. + * Pointers to composites are ignored. This method is intended to be + * used by the {@link #getAllCompositesInPostDependencyOrder} method only. + * @param dt datatype + * @return base datatype if dt corresponds to a composite or array of composites, + * otherwise null is returned + */ + private CompositeDB getCompositeBaseType(DataType dt) { + while ((dt instanceof Array) || (dt instanceof TypeDef)) { + if (dt instanceof Array) { + dt = ((Array) dt).getDataType(); + } + else { + dt = ((TypeDef) dt).getBaseDataType(); + } + } + return (dt instanceof CompositeDB) ? (CompositeDB) dt : null; + } + + /* + * Graph all composites return an ordered list with leaves returned first and detect + * primitve size changes based upon specified primitiveTypeIds. It is assumed TypeDef + * use of primitives have already be handled elsewhere. + * All pointers are ignored and not followed during graph generation. + * This method is intended to facilitate datatype size change propogation in an + * orderly fashion to reduce size change propogation. + + * @param monitor task monitor + * @return order list of composites + * @throws CancelledException if task cancelled + */ + private List getAllCompositesInPostDependencyOrder(TaskMonitor monitor) + throws CancelledException { + + GDirectedGraph> graph = GraphFactory.createDirectedGraph(); + Iterator allComposites = getAllComposites(); + while (allComposites.hasNext()) { + monitor.checkCanceled(); + CompositeDB c = (CompositeDB) allComposites.next(); + graph.addVertex(c); + for (DataTypeComponent m : c.getDefinedComponents()) { + CompositeDB refC = getCompositeBaseType(m.getDataType()); + if (refC != null) { + graph.addEdge(new DefaultGEdge(c, refC)); + } + } + } + return GraphAlgorithms.getVerticesInPostOrder(graph, GraphNavigator.topDownNavigator()); + } + /** * Activate resolveCache and associated resolveQueue if not already active. If * this method returns true caller is responsible for flushing resolveQueue and 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 bfd9affe2a..75c55882cf 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 @@ -590,8 +590,7 @@ class StructureDB extends CompositeDB implements Structure { if (equals(dataType)) { return true; } - for (int i = 0; i < components.size(); i++) { - DataTypeComponent dtc = components.get(i); + for (DataTypeComponentDB dtc : components) { DataType subDt = dtc.getDataType(); if (subDt instanceof Composite) { if (((Composite) subDt).isPartOf(dataType)) { @@ -1198,8 +1197,7 @@ class StructureDB extends CompositeDB implements Structure { int oldLength = structLength; int oldMinAlignment = getMinimumAlignment(); - for (int i = 0; i < components.size(); i++) { - DataTypeComponentDB dtc = components.get(i); + for (DataTypeComponentDB dtc : components) { dtc.getDataType().removeParent(this); componentAdapter.removeRecord(dtc.getKey()); } @@ -1364,37 +1362,44 @@ class StructureDB extends CompositeDB implements Structure { try { checkDeleted(); if (isInternallyAligned()) { - adjustInternalAlignment(true); + adjustComponents(true); // notifies parents return; } boolean didChange = false; + boolean warn = false; int n = components.size(); for (int i = 0; i < n; i++) { DataTypeComponentDB 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) { - dtc.setLength(dtLen, true); - shiftOffsets(nextIndex, dtcLen - dtLen, 0); + int length = getPreferredComponentLength(dt, dtcLen); + if (length < dtcLen) { + dtc.setLength(length, true); + shiftOffsets(i + 1, dtcLen - length, 0); didChange = true; } - else if (dtLen > dtcLen) { - int consumed = consumeBytesAfter(i, dtLen - dtcLen); + else if (length > dtcLen) { + int consumed = consumeBytesAfter(i, length - dtcLen); if (consumed > 0) { dtc.updateRecord(); - shiftOffsets(nextIndex, -consumed, 0); + shiftOffsets(i + 1, -consumed, 0); didChange = true; } } + if (dtc.getLength() != length) { + warn = true; + } } } if (didChange) { - adjustInternalAlignment(true); - notifySizeChanged(); + adjustInternalAlignment(false); + notifySizeChanged(); // notifies parents + } + if (warn) { + Msg.warn(this, + "Failed to resize one or more structure components: " + getPathName()); } } finally { @@ -1402,6 +1407,56 @@ class StructureDB extends CompositeDB implements Structure { } } + @Override + protected void fixupComponents() { + if (isInternallyAligned()) { + // Do not notify parents + if (adjustComponents(false)) { + dataMgr.dataTypeChanged(this); + } + return; + } + boolean didChange = false; + boolean warn = false; + int n = components.size(); + for (int i = 0; i < n; i++) { + DataTypeComponentDB dtc = components.get(i); + DataType dt = dtc.getDataType(); + if (dt instanceof BitFieldDataType) { + // TODO: could get messy + continue; + } + int dtcLen = dtc.getLength(); + int length = getPreferredComponentLength(dt, dtcLen); + if (dtcLen != length) { + if (length < dtcLen) { + dtc.setLength(length, true); + shiftOffsets(i + 1, dtcLen - length, 0); + didChange = true; + } + else if (length > dtcLen) { + int consumed = consumeBytesAfter(i, length - dtcLen); + if (consumed > 0) { + dtc.updateRecord(); + shiftOffsets(i + 1, -consumed, 0); + didChange = true; + } + } + if (dtc.getLength() != length) { + warn = true; + } + } + } + if (didChange) { + // Do not notify parents + adjustInternalAlignment(false); + dataMgr.dataTypeChanged(this); + } + if (warn) { + Msg.warn(this, "Failed to resize one or more structure components: " + getPathName()); + } + } + @Override public void dataTypeAlignmentChanged(DataType dt) { lock.acquire(); @@ -1808,8 +1863,7 @@ class StructureDB extends CompositeDB implements Structure { flexibleArrayComponent = null; } - for (int i = 0; i < components.size(); i++) { - DataTypeComponentDB dtc = components.get(i); + for (DataTypeComponentDB dtc : components) { dtc.getDataType().removeParent(this); try { componentAdapter.removeRecord(dtc.getKey()); @@ -1885,12 +1939,14 @@ class StructureDB extends CompositeDB implements Structure { changed |= updateComposite(packResult.numComponents, packResult.structureLength, packResult.alignment, false); - if (notify & changed) { - if (oldLength != structLength) { - notifySizeChanged(); - } - else { - dataMgr.dataTypeChanged(this); + if (changed) { + if (notify) { + if (oldLength != structLength) { + notifySizeChanged(); + } + else { + dataMgr.dataTypeChanged(this); + } } return true; } 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 34bb985bb2..2b36b98953 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 @@ -277,8 +277,7 @@ class UnionDB extends CompositeDB implements Union { int oldLength = unionLength; int oldMinAlignment = getMinimumAlignment(); - for (int i = 0; i < components.size(); i++) { - DataTypeComponentDB dtc = components.get(i); + for (DataTypeComponentDB dtc : components) { dtc.getDataType().removeParent(this); removeComponent(dtc.getKey()); } @@ -432,7 +431,7 @@ class UnionDB extends CompositeDB implements Union { } } if (changed) { - adjustLength(true, false); + adjustLength(true, false); // notifies parents } } finally { @@ -440,6 +439,30 @@ class UnionDB extends CompositeDB implements Union { } } + @Override + protected void fixupComponents() { + boolean changed = false; + for (DataTypeComponentDB dtc : components) { + DataType dt = dtc.getDataType(); + if (dt instanceof BitFieldDataType) { + dt = adjustBitField(dt); // in case base type changed + } + int dtcLen = dtc.getLength(); + int length = getPreferredComponentLength(dt, dtcLen); + if (length != dtcLen) { + dtc.setLength(length, true); + changed = true; + } + } + if (changed || isInternallyAligned()) { + // NOTE: since we do not retain our external alignment we have no way of knowing if + // it has changed, so we must assume it has if we are an aligned union + // Do not notify parents + adjustLength(false, false); + dataMgr.dataTypeChanged(this); + } + } + @Override public void dataTypeAlignmentChanged(DataType dt) { adjustInternalAlignment(true); @@ -614,7 +637,9 @@ class UnionDB extends CompositeDB implements Union { catch (IOException e) { dataMgr.dbError(e); } - notifySizeChanged(); + if (notify) { + notifySizeChanged(); + } } else if (notify) { dataMgr.dataTypeChanged(this); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/Program.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/Program.java index 912bb952e6..2b3227cd97 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/Program.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/Program.java @@ -82,7 +82,7 @@ public interface Program extends DataTypeManagerDomainObject { * Returns the program's datatype manager. */ @Override - public DataTypeManager getDataTypeManager(); + public ProgramBasedDataTypeManager getDataTypeManager(); /** * Returns the programs function manager. diff --git a/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/database/data/StructureDBTest.java b/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/database/data/StructureDBTest.java index 10dcb137d4..86937a9c36 100644 --- a/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/database/data/StructureDBTest.java +++ b/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/database/data/StructureDBTest.java @@ -85,6 +85,30 @@ public class StructureDBTest extends AbstractGTest { return (Pointer) dataMgr.resolve(new Pointer32DataType(dataType), null); } + @Test + public void testEmpty() throws Exception { + Structure s = new StructureDataType("foo", 0); + assertTrue(s.isNotYetDefined()); + assertEquals(0, s.getNumComponents()); + assertEquals(0, s.getNumDefinedComponents()); + Structure s2 = (Structure) dataMgr.resolve(s, null); + assertTrue(s2.isNotYetDefined()); + assertEquals(0, s2.getNumComponents()); + assertEquals(0, s.getNumDefinedComponents()); + } + + @Test + public void testSizeOne() throws Exception { + Structure s = new StructureDataType("foo", 1); + assertFalse(s.isNotYetDefined()); + assertEquals(1, s.getNumComponents()); + assertEquals(0, s.getNumDefinedComponents()); + Structure s2 = (Structure) dataMgr.resolve(s, null); + assertFalse(s2.isNotYetDefined()); + assertEquals(1, s2.getNumComponents()); + assertEquals(0, s2.getNumDefinedComponents()); + } + @Test public void testAdd() throws Exception { assertEquals(8, struct.getLength()); diff --git a/certification.local.manifest b/certification.local.manifest index 9f2425a0b4..beb26db281 100644 --- a/certification.local.manifest +++ b/certification.local.manifest @@ -9,5 +9,4 @@ LICENSE||GHIDRA||||END| NOTICE||GHIDRA||||END| README.md||GHIDRA||||END| build.gradle||GHIDRA||||END| -ghidra.repos.config||GHIDRA||||END| settings.gradle||GHIDRA||||END|