diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/dwarf4/next/DWARFDataTypeConflictHandler.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/dwarf4/next/DWARFDataTypeConflictHandler.java index 753a78ecf4..e26702c39c 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/dwarf4/next/DWARFDataTypeConflictHandler.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/dwarf4/next/DWARFDataTypeConflictHandler.java @@ -171,21 +171,48 @@ class DWARFDataTypeConflictHandler extends DataTypeConflictHandler { !SystemUtilities.isEqual(fullDTCAt.getFieldName(), partDTC.getFieldName())) { return false; } - DataType partDT = partDTC.getDataType(); - DataType fullDT = fullDTCAt.getDataType(); - if (doRelaxedCompare(partDT, fullDT, visitedDataTypes) == RENAME_AND_ADD) { + if (!isMemberFieldPartiallyCompatible(fullDTCAt, partDTC, visitedDataTypes)) { return false; } } if ( part.getFlexibleArrayComponent() != null ) { - return full.getFlexibleArrayComponent() != null - && doRelaxedCompare(part.getFlexibleArrayComponent().getDataType(), - full.getFlexibleArrayComponent().getDataType(), visitedDataTypes) != RENAME_AND_ADD; + return full.getFlexibleArrayComponent() != null && + isMemberFieldPartiallyCompatible(full.getFlexibleArrayComponent(), + part.getFlexibleArrayComponent(), visitedDataTypes); } return true; } + boolean isMemberFieldPartiallyCompatible(DataTypeComponent fullDTC, DataTypeComponent partDTC, + Set visitedDataTypes) { + DataType partDT = partDTC.getDataType(); + DataType fullDT = fullDTC.getDataType(); + ConflictResult dtCompResult = doRelaxedCompare(partDT, fullDT, visitedDataTypes); + switch (dtCompResult) { + case RENAME_AND_ADD: + // The data type of the field in the 'full' structure is completely + // different than the field in the 'part' structure, therefore + // the candidate 'part' structure is not a partial definition of the full struct + return false; + case REPLACE_EXISTING: + // Return true (meaning the field from the 'full' struct is the same or better + // than the field from the 'part' structure) if the components are size compatible. + // This is an intentionally fuzzy match to allow structures with fields + // that are generally the same at a binary level to match. + // For example, the same structure defined in 2 separate compile units with + // slightly different types for the field (due to different compiler options + // or versions or languages) + return fullDTC.getLength() >= partDTC.getLength(); + case USE_EXISTING: + default: + // the data type of the field in the 'full' structure is the same as + // or a better version of the field in the 'part' structure. + return true; + } + + } + private DataTypeComponent getBitfieldByOffsets(Structure full, DataTypeComponent partDTC) { BitFieldDataType partBF = (BitFieldDataType) partDTC.getDataType(); diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/dwarf4/next/DWARFDataTypeImporter.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/dwarf4/next/DWARFDataTypeImporter.java index baf91e7d90..e8fd661d1f 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/dwarf4/next/DWARFDataTypeImporter.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/dwarf4/next/DWARFDataTypeImporter.java @@ -140,14 +140,11 @@ public class DWARFDataTypeImporter { if (result != null) { return result; } + + // Query the dwarfDTM for plain Ghidra dataTypes that were previously + // registered in a different import session. DataType alreadyImportedDT = dwarfDTM.getDataType(diea.getOffset(), null); - if (alreadyImportedDT != null && - !(alreadyImportedDT instanceof Array && - ((Array) alreadyImportedDT).getNumElements() == 1)) { - // HACK: don't re-use previously imported single-element - // Ghidra array datatype because they may have actually been an empty array - // definition we need the special meta-data flag DWARFDataType.isEmptyArrayType - // which is only available in a freshly created DWARFDataType. + if (shouldReuseAlreadyImportedDT(alreadyImportedDT)) { return new DWARFDataType(alreadyImportedDT, null, diea.getOffset()); } @@ -223,6 +220,28 @@ public class DWARFDataTypeImporter { return result; } + /** + * Returns true if the previously imported data type should be reused. + *

+ * Don't re-use previously imported single-element + * Ghidra array datatypes because they may have actually been an empty array + * definition and we need the special meta-data flag DWARFDataType.isEmptyArrayType + * which is only available in a freshly created DWARFDataType. + *

+ * Don't re-use empty structs (isNotYetDefined) to ensure that newer + * definitions of the same struct are given a chance to be resolved() + * into the DTM. + * + * @param alreadyImportedDT dataType to check + * @return boolean true if its okay to reuse the data type + */ + private boolean shouldReuseAlreadyImportedDT(DataType alreadyImportedDT) { + return alreadyImportedDT != null && + !alreadyImportedDT.isNotYetDefined() && + !(alreadyImportedDT instanceof Array && + ((Array) alreadyImportedDT).getNumElements() == 1); + } + /* * when a clone()'d datatype is created, update the current mappings to * point to the new instance instead of the old instance. diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/util/bin/format/dwarf4/next/DWARFDataTypeImporterTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/util/bin/format/dwarf4/next/DWARFDataTypeImporterTest.java index d1d33232c6..c6b8e8d800 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/util/bin/format/dwarf4/next/DWARFDataTypeImporterTest.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/util/bin/format/dwarf4/next/DWARFDataTypeImporterTest.java @@ -165,11 +165,10 @@ public class DWARFDataTypeImporterTest extends DWARFTestBase { * @throws DWARFException */ @Test - @SuppressWarnings("unused") public void testStructDanglingDecl() throws CancelledException, IOException, DWARFException { // CU1 - DebugInfoEntry declDIE = newDeclStruct("mystruct").create(cu); + newDeclStruct("mystruct").create(cu); // CU2 DebugInfoEntry intDIE = addInt(cu2); @@ -188,11 +187,85 @@ public class DWARFDataTypeImporterTest extends DWARFTestBase { assertNull(structdt2); } - /** + /* + * This test is more about the StructureDB implementation updating ordinals of + * components correctly when an embedded datatype changes size. + *

+ * The initial version of struct2.guardfield will have an ordinal of 15 to account for + * all the invisible undefines that are between the first field and the guardfield at offset + * 16. When the data type for struct1 is overwritten with new information that + * changes it size, the undefines are no longer needed and the ordinal of guardfield should + * just be 1. + */ + @Test + public void testStructDeclThenGrow() throws CancelledException, IOException, DWARFException { + + // CU1 + DebugInfoEntry intDIE = addInt(cu); + DebugInfoEntry struct1Decl = newDeclStruct("struct1").create(cu); + DebugInfoEntry struct2 = newStruct("struct2", 20).create(cu); + newMember(struct2, "struct1field", struct1Decl, 0).create(cu); + newMember(struct2, "guardfield", intDIE, 16).create(cu); + + // CU2 + DebugInfoEntry int2DIE = addInt(cu2); + + DebugInfoEntry struct1Impl = newStruct("struct1", 16).create(cu2); + newMember(struct1Impl, "f1", int2DIE, 0).create(cu2); + newMember(struct1Impl, "f2", int2DIE, 4).create(cu2); + newMember(struct1Impl, "f3", int2DIE, 8).create(cu2); + newMember(struct1Impl, "f4", int2DIE, 12).create(cu2); + + importAllDataTypes(); + + Structure struct2dt = (Structure)dataMgr.getDataType(rootCP, "struct2"); + + assertEquals(2, struct2dt.getNumComponents()); + assertEquals(2, struct2dt.getNumDefinedComponents()); + } + + @Test + public void testStructDeclThatIsLaterDefined() + throws CancelledException, IOException, DWARFException { + + // CU1 + // struct structA; // fwd decl + // struct structB { structA struct1field; int guardfield; } + DebugInfoEntry intDIE = addInt(cu); + DebugInfoEntry structADecl = newDeclStruct("structA").create(cu); + DebugInfoEntry structB = newStruct("structB", 20).create(cu); + newMember(structB, "structAfield", structADecl, 0).create(cu); + newMember(structB, "guardfield", intDIE, 16).create(cu); + + // CU2 + // struct structB { structA struct1field; int guardfield; } + // struct structA { int f1, f2, f3, f4; } + // Redefine structB with the same info, but to a fully + // specified structA instance instead of missing fwd decl. + // The order of the DIE records is important. The structure (structB) + // containing the problematic structA needs to be hit first so we can + // test that cached types are handled correctly. + DebugInfoEntry int2DIE = addInt(cu2); + DebugInfoEntry structB_cu2 = newStruct("structB", 20).create(cu2); + newMember(structB_cu2, "structAfield", getForwardOffset(cu2, 2), 0).create(cu2); + newMember(structB_cu2, "guardfield", int2DIE, 16).create(cu2); + + DebugInfoEntry structA_cu2 = newStruct("structA", 16).create(cu2); + newMember(structA_cu2, "f1", int2DIE, 0).create(cu2); + newMember(structA_cu2, "f2", int2DIE, 4).create(cu2); + newMember(structA_cu2, "f3", int2DIE, 8).create(cu2); + newMember(structA_cu2, "f4", int2DIE, 12).create(cu2); + + importAllDataTypes(); + + Structure structAdt = (Structure) dataMgr.getDataType(rootCP, "structA"); + Structure structBdt = (Structure) dataMgr.getDataType(rootCP, "structB"); + assertEquals(2, structBdt.getNumComponents()); + assertEquals(4, structAdt.getNumComponents()); + } + + /* * Test structure definition when the same structure is defined in two different CUs. - * @throws CancelledException - * @throws IOException - * @throws DWARFException */ @Test public void testStructDup() throws CancelledException, IOException, DWARFException { @@ -648,6 +721,49 @@ public class DWARFDataTypeImporterTest extends DWARFTestBase { } + /* + * Test flex array where dims were specified with no value, relying on + * default. + */ + @Test + public void testStructFlexarray_noValue() + throws CancelledException, IOException, DWARFException { + + DebugInfoEntry intDIE = addInt(cu); + DebugInfoEntry arrayDIE = newArray(cu, intDIE, true, -1); + + DebugInfoEntry structDIE = newStruct("mystruct", 100).create(cu); + newMember(structDIE, "f1", intDIE, 0).create(cu); + newMember(structDIE, "flexarray", arrayDIE, 100).create(cu); + + importAllDataTypes(); + + Structure structdt = (Structure) dataMgr.getDataType(rootCP, "mystruct"); + assertNotNull(structdt.getFlexibleArrayComponent()); + + } + + /* + * Test flex array where dims were specified using a count=0 value instead of a + * upperbound=-1. + */ + @Test + public void testStructFlexarray_0count() + throws CancelledException, IOException, DWARFException { + + DebugInfoEntry intDIE = addInt(cu); + DebugInfoEntry arrayDIE = newArrayUsingCount(cu, intDIE, 0); + + DebugInfoEntry structDIE = newStruct("mystruct", 100).create(cu); + newMember(structDIE, "f1", intDIE, 0).create(cu); + newMember(structDIE, "flexarray", arrayDIE, 100).create(cu); + + importAllDataTypes(); + + Structure structdt = (Structure) dataMgr.getDataType(rootCP, "mystruct"); + assertNotNull(structdt.getFlexibleArrayComponent()); + } + @Test public void testStructInteriorFlexarray() throws CancelledException, IOException, DWARFException { @@ -691,7 +807,7 @@ public class DWARFDataTypeImporterTest extends DWARFTestBase { Structure structdt = (Structure) dataMgr.getDataType(rootCP, "mystruct"); List bitfields = getBitFieldComponents(structdt); - Set expectedBitfieldSizes = new HashSet(Set.of(2, 3, 9)); + Set expectedBitfieldSizes = new HashSet<>(Set.of(2, 3, 9)); for (DataTypeComponent dtc : bitfields) { BitFieldDataType bfdt = (BitFieldDataType) dtc.getDataType(); expectedBitfieldSizes.remove(bfdt.getBitSize()); @@ -824,6 +940,9 @@ public class DWARFDataTypeImporterTest extends DWARFTestBase { // anon struct } + /* + * Test array defintions that use upper_bound attribute + */ @Test public void testArray() throws CancelledException, IOException, DWARFException { DebugInfoEntry intDIE = addInt(cu); @@ -833,8 +952,24 @@ public class DWARFDataTypeImporterTest extends DWARFTestBase { Array arr = (Array) dwarfDTM.getDataType(arrayDIE.getOffset(), null); assertNotNull(arr); + assertEquals(11, arr.getNumElements()); } + /* + * Tests array definitions that use count attribute instead of upper_bounds + */ + @Test + public void testArrayWithCountAttr() throws CancelledException, IOException, DWARFException { + DebugInfoEntry intDIE = addInt(cu); + DebugInfoEntry arrayDIE = newArrayUsingCount(cu, intDIE, 10); + + importAllDataTypes(); + + Array arr = (Array) dwarfDTM.getDataType(arrayDIE.getOffset(), null); + assertNotNull(arr); + assertEquals(10, arr.getNumElements()); + } + // not implemented yet public void testSubr() { // func ptrs diff --git a/Ghidra/Features/Base/src/test/java/ghidra/app/util/bin/format/dwarf4/DWARFTestBase.java b/Ghidra/Features/Base/src/test/java/ghidra/app/util/bin/format/dwarf4/DWARFTestBase.java index 6db58b90a1..6f58b91ac0 100644 --- a/Ghidra/Features/Base/src/test/java/ghidra/app/util/bin/format/dwarf4/DWARFTestBase.java +++ b/Ghidra/Features/Base/src/test/java/ghidra/app/util/bin/format/dwarf4/DWARFTestBase.java @@ -16,7 +16,7 @@ package ghidra.app.util.bin.format.dwarf4; import static ghidra.app.util.bin.format.dwarf4.encoding.DWARFAttribute.*; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; import java.io.IOException; @@ -264,9 +264,16 @@ public class DWARFTestBase extends AbstractGhidraHeadedIntegrationTest { DebugInfoEntry dataType, int offset) { assertTrue( dataType == null || dataType.getCompilationUnit() == parentStruct.getCompilationUnit()); + return newMember(parentStruct, fieldName, dataType.getOffset(), offset); + } + + protected DIECreator newMember(DebugInfoEntry parentStruct, String fieldName, + long memberDIEOffset, int offset) { DIECreator field = - new DIECreator(DWARFTag.DW_TAG_member).addString(DW_AT_name, fieldName).addRef( - DW_AT_type, dataType).setParent(parentStruct); + new DIECreator(DWARFTag.DW_TAG_member).addString(DW_AT_name, fieldName) + .addRef( + DW_AT_type, memberDIEOffset) + .setParent(parentStruct); if (offset != -1) { field.addInt(DW_AT_data_member_location, offset); } @@ -285,11 +292,11 @@ public class DWARFTestBase extends AbstractGhidraHeadedIntegrationTest { protected DebugInfoEntry newArray(MockDWARFCompilationUnit dcu, DebugInfoEntry baseTypeDIE, boolean elideEmptyDimRangeValue, int... dimensions) { - DebugInfoEntry arrayType = new DIECreator(DWARFTag.DW_TAG_array_type) // + DebugInfoEntry arrayType = new DIECreator(DWARFTag.DW_TAG_array_type) .addRef(DW_AT_type, baseTypeDIE).create(dcu); for (int dimIndex = 0; dimIndex < dimensions.length; dimIndex++) { int dim = dimensions[dimIndex]; - DIECreator dimDIE = new DIECreator(DWARFTag.DW_TAG_subrange_type) // + DIECreator dimDIE = new DIECreator(DWARFTag.DW_TAG_subrange_type) .setParent(arrayType); if (dim != -1 || !elideEmptyDimRangeValue) { dimDIE.addInt(DW_AT_upper_bound, dimensions[dimIndex]); @@ -299,6 +306,18 @@ public class DWARFTestBase extends AbstractGhidraHeadedIntegrationTest { return arrayType; } + protected DebugInfoEntry newArrayUsingCount(MockDWARFCompilationUnit dcu, + DebugInfoEntry baseTypeDIE, int count) { + DebugInfoEntry arrayType = new DIECreator(DWARFTag.DW_TAG_array_type) + .addRef(DW_AT_type, baseTypeDIE) + .create(dcu); + DIECreator dimDIE = new DIECreator(DWARFTag.DW_TAG_subrange_type) + .setParent(arrayType); + dimDIE.addInt(DW_AT_count, count); + dimDIE.create(dcu); + return arrayType; + } + protected Address addr(long l) { return space.getAddress(l); }