diff --git a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/util/exporter/CppExporter.java b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/util/exporter/CppExporter.java index e2ab23cb84..4c84198cdf 100644 --- a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/util/exporter/CppExporter.java +++ b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/util/exporter/CppExporter.java @@ -295,7 +295,7 @@ public class CppExporter extends Exporter { DataTypeWriter dataTypeWriter = new DataTypeWriter(dtm, headerWriter, isUseCppStyleComments); headerWriter.write(getFakeCTypeDefinitions(dtm.getDataOrganization())); - dataTypeWriter.write(dtm, monitor); + dataTypeWriter.write(monitor); headerWriter.println(""); headerWriter.println(""); @@ -308,7 +308,7 @@ public class CppExporter extends Exporter { DataTypeManager dtm = program.getDataTypeManager(); DataTypeWriter dataTypeWriter = new DataTypeWriter(dtm, cFileWriter, isUseCppStyleComments); - dataTypeWriter.write(dtm, monitor); + dataTypeWriter.write(monitor); } if (cFileWriter != null) { diff --git a/Ghidra/Framework/Generic/src/main/java/ghidra/util/graph/AbstractDependencyGraph.java b/Ghidra/Framework/Generic/src/main/java/ghidra/util/graph/AbstractDependencyGraph.java index 6dcc88aed6..a41b253247 100644 --- a/Ghidra/Framework/Generic/src/main/java/ghidra/util/graph/AbstractDependencyGraph.java +++ b/Ghidra/Framework/Generic/src/main/java/ghidra/util/graph/AbstractDependencyGraph.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. @@ -237,7 +237,7 @@ public abstract class AbstractDependencyGraph { * graph, dependencies will be removed and additional values will be eligible to be returned * by this method. Once a value has been retrieved using this method, it will be considered * "visited" and future calls to this method will not include those values. To continue - * processing the values in the graph, all values return from this method should eventually + * processing the values in the graph, all values returned from this method should eventually * be deleted from the graph to "free up" other values. NOTE: values retrieved by this method * will no longer be eligible for return by the pop() method. * diff --git a/Ghidra/Framework/Generic/src/main/java/ghidra/util/graph/DeterministicDependencyGraph.java b/Ghidra/Framework/Generic/src/main/java/ghidra/util/graph/DeterministicDependencyGraph.java index 523d084e32..4f51dd4142 100644 --- a/Ghidra/Framework/Generic/src/main/java/ghidra/util/graph/DeterministicDependencyGraph.java +++ b/Ghidra/Framework/Generic/src/main/java/ghidra/util/graph/DeterministicDependencyGraph.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. @@ -25,8 +25,9 @@ import org.apache.commons.collections4.set.ListOrderedSet; * memory than {@link DependencyGraph}, and if memory is not an issue, it also seems to be * slightly faster as well. *

- * This class was implemented to provide determinism while doing - * developmental debugging. + * This class was implemented to provide determinism while doing developmental debugging. + *

+ * Note: the types used in this graph must be comparable in order to provider consistent ordering. * * @param the type of value. * diff --git a/Ghidra/Framework/Generic/src/test/java/ghidra/util/datastruct/DependencyGraphTest.java b/Ghidra/Framework/Generic/src/test/java/ghidra/util/datastruct/DependencyGraphTest.java index e353bbab4a..4ae02b1474 100644 --- a/Ghidra/Framework/Generic/src/test/java/ghidra/util/datastruct/DependencyGraphTest.java +++ b/Ghidra/Framework/Generic/src/test/java/ghidra/util/datastruct/DependencyGraphTest.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. @@ -19,7 +19,6 @@ import static org.junit.Assert.*; import java.util.*; -import org.junit.Assert; import org.junit.Test; import ghidra.util.graph.*; @@ -186,7 +185,7 @@ public class DependencyGraphTest { while (!graph.isEmpty()) { graph.pop(); } - Assert.fail("Expected cycle exception"); + fail("Expected cycle exception"); } catch (IllegalStateException e) { // expected @@ -285,28 +284,26 @@ public class DependencyGraphTest { /** * Given a dependency map, does the captured linear order of execution * satisfy the ordering constraints? - * @param the type of the keys being compared * @param dependencyGraph a map where keys are predecessors and values * are successors that depend on the respective key * @param visitedOrder the actual execution order to be tested - * @return */ private void checkOrderSatisfiesDependencies(AbstractDependencyGraph dependencyGraph, List visitedOrder) { if (visitedOrder.size() > dependencyGraph.size()) { - Assert.fail("More items were visited than the number of items in the graph"); + fail("More items were visited than the number of items in the graph"); } if (visitedOrder.size() < dependencyGraph.size()) { - Assert.fail("Not all items in the graph were visited"); + fail("Not all items in the graph were visited"); } - HashSet items = new HashSet<>(visitedOrder); + Set items = new HashSet<>(visitedOrder); if (items.size() != visitedOrder.size()) { - Assert.fail("duplicate item(s) in linearOrder\n"); + fail("duplicate item(s) in linearOrder\n"); } - HashMap visitedOrderMap = new HashMap<>(); + Map visitedOrderMap = new HashMap<>(); for (int i = 0; i < visitedOrder.size(); i++) { visitedOrderMap.put(visitedOrder.get(i), i); } @@ -314,21 +311,21 @@ public class DependencyGraphTest { for (String key : dependencyGraph.getValues()) { Integer visitedOrdinal = visitedOrderMap.get(key); if (visitedOrdinal == null) { - Assert.fail("dependencyGraph key " + key + " not in linearOrder\n"); + fail("dependencyGraph key " + key + " not in linearOrder\n"); } Set dependents = dependencyGraph.getDependentValues(key); for (String dependent : dependents) { if (key.equals(dependent)) { - Assert.fail("dependencyGraph key " + key + " depends on itself\n"); + fail("dependencyGraph key " + key + " depends on itself\n"); } Integer dependentVisitedOrdinal = visitedOrderMap.get(dependent); if (dependentVisitedOrdinal == null) { - Assert.fail("dependent " + dependent + " of dependencyGraph key " + key + + fail("dependent " + dependent + " of dependencyGraph key " + key + " not in linearOrder\n"); } if (dependentVisitedOrdinal <= visitedOrdinal) { - Assert.fail("dependent " + dependent + " of dependencyGraph key " + key + + fail("dependent " + dependent + " of dependencyGraph key " + key + " came first (" + dependentVisitedOrdinal + " < " + visitedOrdinal + ")\n"); } } diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeWriter.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeWriter.java index 0a26127821..b3623623a7 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeWriter.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeWriter.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. @@ -23,6 +23,7 @@ import org.apache.commons.lang3.StringUtils; import ghidra.util.Msg; import ghidra.util.exception.CancelledException; +import ghidra.util.graph.DeterministicDependencyGraph; import ghidra.util.task.TaskMonitor; /** @@ -43,9 +44,11 @@ public class DataTypeWriter { private Set resolved = new HashSet<>(); private Map resolvedTypeMap = new HashMap<>(); - private Set deferredCompositeDeclarations = new HashSet<>(); - private ArrayDeque deferredTypeFIFO = new ArrayDeque<>(); - private Set deferredTypes = new HashSet<>(); + + private DeterministicDependencyGraph compositeDependencyGraph = + new DeterministicDependencyGraph<>(); + private LinkedHashSet deferredCompositeInternalTypes = new LinkedHashSet<>(); + private int writerDepth = 0; private Writer writer; private DataTypeManager dtm; @@ -126,14 +129,13 @@ public class DataTypeWriter { /** * Converts all data types in the data type manager into ANSI-C code. - * @param dataTypeManager the manager containing the data types to write * @param monitor the task monitor * @throws IOException if there is an exception writing the output * @throws CancelledException if the action is cancelled by the user */ - public void write(DataTypeManager dataTypeManager, TaskMonitor monitor) + public void write(TaskMonitor monitor) throws IOException, CancelledException { - write(dataTypeManager.getRootCategory(), monitor); + write(dtm.getRootCategory(), monitor); } /** @@ -199,9 +201,8 @@ public class DataTypeWriter { } private void deferWrite(DataType dt) { - if (!resolved.contains(dt) && !deferredTypes.contains(dt)) { - deferredTypes.add(dt); - deferredTypeFIFO.addLast(dt); + if (!resolved.contains(dt)) { + deferredCompositeInternalTypes.add(dt); } } @@ -222,6 +223,9 @@ public class DataTypeWriter { */ private void doWrite(DataType dt, TaskMonitor monitor, boolean throwExceptionOnInvalidType) throws IOException, CancelledException { + + monitor.checkCancelled(); + if (dt == null) { return; } @@ -280,27 +284,25 @@ public class DataTypeWriter { writer.write(EOL); writer.write(EOL); } - else if (dt instanceof Dynamic) { - writeDynamicBuiltIn((Dynamic) dt, monitor); + else if (dt instanceof Dynamic dynamic) { + writeDynamicBuiltIn(dynamic, monitor); } - else if (dt instanceof Structure) { - Structure struct = (Structure) dt; + else if (dt instanceof Structure struct) { writeCompositePreDeclaration(struct, monitor); - deferredCompositeDeclarations.add(struct); + addCompositeToDependencyGraph(struct, monitor); } - else if (dt instanceof Union) { - Union union = (Union) dt; + else if (dt instanceof Union union) { writeCompositePreDeclaration(union, monitor); - deferredCompositeDeclarations.add(union); + addCompositeToDependencyGraph(union, monitor); } - else if (dt instanceof Enum) { - writeEnum((Enum) dt, monitor); + else if (dt instanceof Enum enumm) { + writeEnum(enumm, monitor); } - else if (dt instanceof TypeDef) { - writeTypeDef((TypeDef) dt, monitor); + else if (dt instanceof TypeDef typeDef) { + writeTypeDef(typeDef, monitor); } - else if (dt instanceof BuiltInDataType) { - writeBuiltIn((BuiltInDataType) dt, monitor); + else if (dt instanceof BuiltInDataType bidt) { + writeBuiltIn(bidt, monitor); } else if (dt instanceof BitFieldDataType) { // skip @@ -319,15 +321,34 @@ public class DataTypeWriter { --writerDepth; } + private void addCompositeToDependencyGraph(Composite composite, TaskMonitor monitor) + throws CancelledException { + + // Each composite will be a node in the graph. Composite dependencies are added below. + compositeDependencyGraph.addValue(new CompositeNode(composite)); + + for (DataTypeComponent component : composite.getDefinedComponents()) { + + monitor.checkCancelled(); + + DataType dt = component.getDataType(); + if (dt instanceof Composite childComposite) { + CompositeNode start = new CompositeNode(composite); + CompositeNode end = new CompositeNode(childComposite); + compositeDependencyGraph.addDependency(start, end); + } + } + } + private void writeDeferredDeclarations(TaskMonitor monitor) throws IOException, CancelledException { - while (!deferredTypes.isEmpty()) { - DataType dt = deferredTypeFIFO.removeFirst(); - deferredTypes.remove(dt); + + while (!deferredCompositeInternalTypes.isEmpty()) { + DataType dt = deferredCompositeInternalTypes.removeFirst(); write(dt, monitor); } + writeDeferredCompositeDeclarations(monitor); - deferredCompositeDeclarations.clear(); } private DataType getBaseArrayTypedefType(DataType dt) { @@ -345,81 +366,38 @@ public class DataTypeWriter { return dt; } - private boolean containsComposite(Composite container, Composite contained) { - for (DataTypeComponent component : container.getDefinedComponents()) { - DataType dt = getBaseArrayTypedefType(component.getDataType()); - if (dt instanceof Composite && dt.getName().equals(contained.getName()) && - dt.isEquivalent(contained)) { - return true; - } - } - return false; - } - private void writeDeferredCompositeDeclarations(TaskMonitor monitor) throws IOException, CancelledException { - int cnt = deferredCompositeDeclarations.size(); - if (cnt == 0) { - return; - } - LinkedList list = new LinkedList<>(deferredCompositeDeclarations); - if (list.size() > 1) { - // Establish dependency ordering - int sortChange = 1; - while (sortChange != 0) { - sortChange = 0; - for (int i = cnt - 1; i > 0; i--) { - if (resortComposites(list, i)) { - ++sortChange; - } - } - } + CompositeNode node = compositeDependencyGraph.pop(); + while (node != null) { + writeCompositeBody(node.composite, monitor); + node = compositeDependencyGraph.pop(); } - - for (Composite composite : list) { - writeCompositeBody(composite, monitor); - } - } - - private boolean resortComposites(List list, int index) { - int listSize = list.size(); - if (listSize <= 0) { - return false; - } - Composite composite = list.get(index); - for (int i = 0; i < index; i++) { - Composite other = list.get(i); - if (containsComposite(other, composite)) { - list.remove(index); - list.add(i, composite); - composite = null; - return true; - } - } - return false; } private String getDynamicComponentString(Dynamic dynamicType, String fieldName, int length) { - if (dynamicType.canSpecifyLength()) { - DataType replacementBaseType = dynamicType.getReplacementBaseType(); - if (replacementBaseType != null) { - replacementBaseType = replacementBaseType.clone(dtm); - int elementLen = replacementBaseType.getLength(); - if (elementLen <= 0) { - Msg.error(this, - dynamicType.getClass().getSimpleName() + - " returned bad replacementBaseType: " + - replacementBaseType.getClass().getSimpleName()); - } - else { - int elementCnt = (length + elementLen - 1) / elementLen; - return replacementBaseType.getDisplayName() + " " + fieldName + "[" + - elementCnt + "]"; - } - } + if (!dynamicType.canSpecifyLength()) { + return null; } - return null; + + DataType replacementBaseType = dynamicType.getReplacementBaseType(); + if (replacementBaseType == null) { + return null; + } + + replacementBaseType = replacementBaseType.clone(dtm); + int elementLen = replacementBaseType.getLength(); + if (elementLen <= 0) { + String dynamicName = dynamicType.getClass().getSimpleName(); + String replacementName = replacementBaseType.getClass().getSimpleName(); + Msg.error(this, dynamicName + " returned bad replacementBaseType: " + replacementName); + return null; + } + + int elementCnt = (length + elementLen - 1) / elementLen; + return replacementBaseType.getDisplayName() + " " + fieldName + "[" + + elementCnt + "]"; } private void writeCompositePreDeclaration(Composite composite, TaskMonitor monitor) @@ -433,16 +411,12 @@ public class DataTypeWriter { writer.write(EOL); writer.write(EOL); - for (DataTypeComponent component : composite.getComponents()) { - if (monitor.isCancelled()) { - break; - } + for (DataTypeComponent component : composite.getDefinedComponents()) { + monitor.checkCancelled(); + // force resolution of field datatype DataType componentType = component.getDataType(); deferWrite(componentType); - - // TODO the return value of this is not used--delete? - getTypeDeclaration(null, componentType, component.getLength(), true, monitor); } } @@ -642,7 +616,8 @@ public class DataTypeWriter { return; } } - // TODO: A comment explaining the special 'P' case would be helpful!! Smells like fish. + + // auto-pointer-typedef generated with composite else if (baseType instanceof Pointer && typedefName.startsWith("P")) { DataType dt = ((Pointer) baseType).getDataType(); if (dt instanceof TypeDef) { @@ -891,4 +866,47 @@ public class DataTypeWriter { buf.append(")"); return buf.toString(); } + + // A simple Composite class to use in the dependency graph to speed up the equals() call + private class CompositeNode implements Comparable { + + private Composite composite; + + CompositeNode(Composite composite) { + this.composite = composite; + } + + @Override + public int compareTo(CompositeNode o) { + String pn1 = composite.getPathName(); + String pn2 = o.composite.getPathName(); + return pn1.compareTo(pn2); + } + + @Override + public int hashCode() { + return composite.hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + + CompositeNode other = (CompositeNode) obj; + return composite.getUniversalID().equals(other.composite.getUniversalID()); + } + + @Override + public String toString() { + return composite.toString(); + } + } }