GP-1500 DWARF anon structure naming improvement

Fix issue where anon structs / unions could be incorrectly replaced with other anon structs / unions that were present in the data type graph when a containing type was submitted to the DataTypeManager.

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/analysis/DWARFAnalyzer.java

@@ -72,11 +72,6 @@ public class DWARFAnalyzer extends AbstractAnalyzer {
 	private static final String OPTION_OUTPUT_INLINE_FUNC_COMMENTS_DESC =
 		"Add comments to the start of inlined functions";
 
-	private static final String OPTION_COPY_ANON_TYPES =
-		"Create local copy of anonymous types for struct fields";
-	private static final String OPTION_COPY_ANON_TYPES_DESC =
-		"Clones anonymous types used in a struct and creates a local copy using the name of the field.";
-
 	private static final String OPTION_OUTPUT_FUNC_SIGS = "Output function signatures";
 	private static final String OPTION_OUTPUT_FUNC_SIGS_DESC =
 		"Create function signature data types for each function encountered in the DWARF debug data.";
@@ -165,6 +160,7 @@ public class DWARFAnalyzer extends AbstractAnalyzer {
 		return false;
 	}
 
+	@Deprecated(forRemoval = true, since = "10.0")
 	private boolean oldCheckIfDWARFImported(Program prog) {
 		// this was the old way of checking if the DWARF analyzer had already been run.  Keep
 		// it around for a little bit so existing programs that have already imported DWARF data
@@ -208,9 +204,6 @@ public class DWARFAnalyzer extends AbstractAnalyzer {
 		options.registerOption(OPTION_NAME_LENGTH_CUTOFF, importOptions.getNameLengthCutoff(), null,
 			OPTION_NAME_LENGTH_CUTOFF_DESC);
 
-		options.registerOption(OPTION_COPY_ANON_TYPES, importOptions.isCopyRenameAnonTypes(), null,
-			OPTION_COPY_ANON_TYPES_DESC);
-
 		options.registerOption(OPTION_OUTPUT_FUNC_SIGS, importOptions.isCreateFuncSignatures(),
 			null, OPTION_OUTPUT_FUNC_SIGS_DESC);
 	}
@@ -235,8 +228,6 @@ public class DWARFAnalyzer extends AbstractAnalyzer {
 			options.getInt(OPTION_IMPORT_LIMIT_DIE_COUNT, importOptions.getImportLimitDIECount()));
 		importOptions.setNameLengthCutoff(
 			options.getInt(OPTION_NAME_LENGTH_CUTOFF, importOptions.getNameLengthCutoff()));
-		importOptions.setCopyRenameAnonTypes(
-			options.getBoolean(OPTION_COPY_ANON_TYPES, importOptions.isCopyRenameAnonTypes()));
 		importOptions.setCreateFuncSignatures(
 			options.getBoolean(OPTION_OUTPUT_FUNC_SIGS, importOptions.isCreateFuncSignatures()));
 	}

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/dwarf4/DWARFUtil.java

@@ -28,6 +28,7 @@ import ghidra.app.util.bin.format.dwarf4.attribs.DWARFAttributeValue;
 import ghidra.app.util.bin.format.dwarf4.attribs.DWARFNumericAttribute;
 import ghidra.app.util.bin.format.dwarf4.encoding.DWARFAttribute;
 import ghidra.app.util.bin.format.dwarf4.encoding.DWARFTag;
+import ghidra.app.util.bin.format.dwarf4.expression.DWARFExpressionException;
 import ghidra.app.util.bin.format.dwarf4.next.DWARFProgram;
 import ghidra.program.model.data.DataType;
 import ghidra.program.model.data.DataTypeComponent;
@@ -325,6 +326,47 @@ public class DWARFUtil {
 		return "anon_" + getContainerTypeName(diea) + "_for_" + sb.toString();
 	}
 
+	/**
+	 * Creates a fingerprint of the layout of an (anonymous) structure using its
+	 * size, number of members, and the hashcode of the member field names.
+	 * 
+	 * @param diea struct/union/class
+	 * @return formatted string, example "80_5_73dc6de9" (80 bytes, 5 fields, hex hash of field names) 
+	 */
+	public static String getStructLayoutFingerprint(DIEAggregate diea) {
+		long structSize = diea.getUnsignedLong(DWARFAttribute.DW_AT_byte_size, 0);
+		int memberCount = 0;
+		List<String> memberNames = new ArrayList<>();
+		for (DebugInfoEntry childEntry : diea.getHeadFragment().getChildren()) {
+			if (!(childEntry.getTag() == DWARFTag.DW_TAG_member ||
+				childEntry.getTag() == DWARFTag.DW_TAG_inheritance)) {
+				continue;
+			}
+			DIEAggregate childDIEA = diea.getProgram().getAggregate(childEntry);
+			if (childDIEA.hasAttribute(DWARFAttribute.DW_AT_external)) {
+				continue;
+			}
+			memberCount++;
+
+			String memberName = childDIEA.getName();
+			int memberOffset = 0;
+			try {
+				memberOffset =
+					childDIEA.parseDataMemberOffset(DWARFAttribute.DW_AT_data_member_location, 0);
+			}
+			catch (DWARFExpressionException | IOException e) {
+				// ignore, leave as default value 0
+			}
+			if (memberName == null) {
+				memberName = "UNNAMED_MEMBER_" + memberCount;
+			}
+			memberName = String.format("%04x_%s", memberOffset, memberName);
+			memberNames.add(memberName);
+		}
+		Collections.sort(memberNames);	// "hexoffset_name"
+		return String.format("%d_%d_%08x", structSize, memberCount, memberNames.hashCode());
+	}
+
 	/**
 	 * Create a name for a lexical block, with "_" separated numbers indicating nesting
 	 * information of the lexical block.

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/dwarf4/next/DWARFDataTypeImporter.java

@@ -15,13 +15,13 @@
  */
 package ghidra.app.util.bin.format.dwarf4.next;
 
-import java.io.IOException;
 import java.util.*;
 import java.util.stream.Collectors;
 
+import java.io.IOException;
+
 import org.apache.commons.lang3.StringUtils;
 
-import ghidra.app.plugin.core.datamgr.util.DataTypeUtils;
 import ghidra.app.util.DataTypeNamingUtil;
 import ghidra.app.util.bin.format.dwarf4.*;
 import ghidra.app.util.bin.format.dwarf4.encoding.*;
@@ -30,9 +30,7 @@ import ghidra.program.database.DatabaseObject;
 import ghidra.program.database.data.DataTypeUtilities;
 import ghidra.program.model.data.*;
 import ghidra.program.model.data.Enum;
-import ghidra.util.InvalidNameException;
 import ghidra.util.Msg;
-import ghidra.util.exception.DuplicateNameException;
 
 /**
  * Creates Ghidra {@link DataType}s using information from DWARF debug entries.  The caller
@@ -48,7 +46,6 @@ public class DWARFDataTypeImporter {
 	private DWARFDataTypeManager dwarfDTM;
 	private DWARFImportOptions importOptions;
 	private DWARFDataType voidDDT;
-	private DWARFNameInfo rootDNI;
 
 	/**
 	 * Tracks which {@link DIEAggregate DIEAs} have been visited by {@link #getDataTypeWorker(DIEAggregate, DataType)}
@@ -93,7 +90,6 @@ public class DWARFDataTypeImporter {
 		this.importOptions = importOptions;
 		this.voidDDT = new DWARFDataType(dwarfDTM.getVoidType(),
 			DWARFNameInfo.fromDataType(dwarfDTM.getVoidType()), -1);
-		this.rootDNI = prog.getUncategorizedRootDNI();
 	}
 
 	public DWARFDataType getDDTByInstance(DataType dtInstance) {
@@ -370,28 +366,6 @@ public class DWARFDataTypeImporter {
 		return refdDT;
 	}
 
-//	/**
-//	 * Mash parameter datatype names together to make a mangling suffix to append to
-//	 * a function def name.
-//	 * <p>
-//	 * @param returnTypeDT
-//	 * @param parameters
-//	 * @return
-//	 */
-//	private String getMangledFuncDefName(DataType returnTypeDT,
-//			List<ParameterDefinition> parameters) {
-//		StringBuilder sb = new StringBuilder();
-//		sb.append(mangleDTName(returnTypeDT.getName()));
-//		for (ParameterDefinition p : parameters) {
-//			sb.append("_").append(mangleDTName(p.getDataType().getName()));
-//		}
-//		return sb.toString();
-//	}
-//
-//	private String mangleDTName(String s) {
-//		return s.replaceAll(" ", "_").replaceAll("\\*", "ptr");
-//	}
-
 	/**
 	 * Creates a Ghidra {@link Enum} datatype.
 	 * <p>
@@ -849,14 +823,6 @@ public class DWARFDataTypeImporter {
 				}
 			}
 
-			// If the child's datatype is an anon datatype, copy the datatype into our
-			// structure's categorypath and give it a name based on the member name.
-			if (isAnonDataType(childDT) && importOptions.isCopyRenameAnonTypes()) {
-				DataType copiedType = copyAnonTypeForMember(childDT.dataType,
-					new CategoryPath(structure.getCategoryPath(), structure.getName()), memberName);
-				childDT = new DWARFDataType(copiedType, null, childDT.offsets);
-			}
-
 			boolean hasMemberOffset =
 				childDIEA.hasAttribute(DWARFAttribute.DW_AT_data_member_location);
 
@@ -1233,7 +1199,6 @@ public class DWARFDataTypeImporter {
 
 		boolean typedefWithSameName = DataTypeUtilities.equalsIgnoreConflict(
 			typedefDNI.asDataTypePath().getPath(), refdDT.dataType.getPathName());
-		boolean typedefPointingToAnonType = isAnonDataType(refdDT);
 
 		if (typedefWithSameName) {
 			if (importOptions.isElideTypedefsWithSameName()) {
@@ -1247,27 +1212,6 @@ public class DWARFDataTypeImporter {
 			typedefDNI = typedefDNI.replaceName(newName, newName);
 		}
 
-		if (typedefPointingToAnonType && importOptions.isCopyRenameAnonTypes() &&
-			DWARFUtil.getReferringTypedef(refdDIEA) == diea) {
-			// if this typedef points to an anon type (and we are the only typedef pointing
-			// to the anon type), copy the anon type to our namespace as our name, return
-			// it instead of a new typedef
-			DWARFDataType result = new DWARFDataType(
-				DataTypeUtils.copyToNamedBaseDataType(refdDT.dataType, dataTypeManager), typedefDNI,
-				diea.getOffset());
-			try {
-				DataType namedType = DataTypeUtils.getNamedBaseDataType(result.dataType);
-				namedType.setNameAndCategory(typedefDNI.getParent().asCategoryPath(),
-					typedefDNI.getName());
-				dataTypeInstanceToDDTMap.put(namedType, result);
-			}
-			catch (InvalidNameException | DuplicateNameException e) {
-				// fall thru to default action of just returning original type unchanged
-			}
-
-			return result;
-		}
-
 		TypedefDataType typedefDT = new TypedefDataType(typedefDNI.getParentCP(),
 			typedefDNI.getName(), refdDT.dataType, dataTypeManager);
 		updateMapping(refdDT.dataType, typedefDT.getDataType());
@@ -1292,67 +1236,6 @@ public class DWARFDataTypeImporter {
 		return new DWARFDataType(dt, dni, diea.getOffset());
 	}
 
-	/**
-	 * Returns true if the specified {@link DataType} (or if its a pointer, the pointed to
-	 * DataType) is a data type that did not have a name and was assigned an name in the form
-	 * "anon_datatype".
-	 *
-	 * @param dt
-	 * @return
-	 */
-	private static boolean isAnonDataType(DWARFDataType ddt) {
-		if (ddt.dni != null && ddt.dni.isAnon()) {
-			return true;
-		}
-		DataType namedType = DataTypeUtils.getNamedBaseDataType(ddt.dataType);
-		return namedType.getName().startsWith("anon_");
-	}
-
-	/**
-	 * Copy an anon Datatype (or a chain of pointers to a anon DataType) into a new CategoryPath
-	 * with a new name that is appropriate for a structure member field.
-	 * <p>
-	 * Use this method when a struct has a field with an anon datatype.  The new copied
-	 * datatype will be called "anonorigname_for_structurefieldname"
-	 * <p>
-	 *
-	 * @param dt - DataType to copy
-	 * @param destCategory {@link CategoryPath} to copy to
-	 * @param membername the name of the structure member that uses this anon datatype.
-	 * @return new DataType that is a copy of the old type, but in a new location and name.
-	 */
-	private DataType copyAnonTypeForMember(DataType dt, CategoryPath destCategory,
-			String membername) {
-		List<Pointer> ptrChainTypes = new ArrayList<>();
-		DataType actualDT = dt;
-		while (actualDT instanceof Pointer) {
-			ptrChainTypes.add((Pointer) actualDT);
-			actualDT = ((Pointer) actualDT).getDataType();
-		}
-
-		if (actualDT.getCategoryPath().equals(destCategory)) {
-			return dt;
-		}
-
-		DataType copy = actualDT.copy(dataTypeManager);
-		try {
-			copy.setNameAndCategory(destCategory, copy.getName() + "_for_" + membername);
-		}
-		catch (InvalidNameException | DuplicateNameException e) {
-			Msg.error(this, "Failed to copy anon type " + dt);
-			return dt;
-		}
-
-		DataType result = copy;
-		for (int i = ptrChainTypes.size() - 1; i >= 0; i--) {
-			Pointer origPtr = ptrChainTypes.get(i);
-			result = new PointerDataType(result,
-				origPtr.hasLanguageDependantLength() ? -1 : origPtr.getLength(), dataTypeManager);
-		}
-
-		return result;
-	}
-
 	static class DWARFDataType {
 		DataType dataType;
 		DWARFNameInfo dni;

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/dwarf4/next/DWARFProgram.java

@@ -15,9 +15,10 @@
  */
 package ghidra.app.util.bin.format.dwarf4.next;
 
+import java.util.*;
+
 import java.io.Closeable;
 import java.io.IOException;
-import java.util.*;
 
 import org.apache.commons.collections4.ListValuedMap;
 import org.apache.commons.collections4.multimap.ArrayListValuedHashMap;
@@ -343,6 +344,26 @@ public class DWARFProgram implements Closeable {
 			}
 		}
 
+		if (name == null && diea.isStructureType()) {
+			String fingerprint = DWARFUtil.getStructLayoutFingerprint(diea);
+
+			// check to see if there are struct member defs that ref this anon type
+			// and build a name using the field names
+			List<DIEAggregate> referringMembers = (diea != null)
+					? diea.getProgram().getTypeReferers(diea, DWARFTag.DW_TAG_member)
+					: null;
+
+			String referringMemberNames = getReferringMemberFieldNames(referringMembers);
+			if (!referringMemberNames.isEmpty()) {
+				parentDNI = getName(referringMembers.get(0).getParent());
+				referringMemberNames = "_for_" + referringMemberNames;
+			}
+			name =
+				"anon_" + DWARFUtil.getContainerTypeName(diea) + "_" + fingerprint +
+					referringMemberNames;
+			return parentDNI.createChild(null, name, DWARFUtil.getSymbolTypeFromDIE(diea));
+		}
+
 		boolean isAnon = false;
 		if (name == null) {
 			switch (diea.getTag()) {
@@ -425,6 +446,36 @@ public class DWARFProgram implements Closeable {
 
 	}
 
+	private String getReferringMemberFieldNames(List<DIEAggregate> referringMembers) {
+		if (referringMembers == null || referringMembers.isEmpty()) {
+			return "";
+		}
+		DIEAggregate commonParent = referringMembers.get(0).getParent();
+		StringBuilder result = new StringBuilder();
+		for (DIEAggregate referringMember : referringMembers) {
+			if (commonParent != referringMember.getParent()) {
+				// if there is an inbound referring link that isn't from the same parent,
+				// abort
+				return "";
+			}
+			String memberName = referringMember.getName();
+			if (memberName == null) {
+				int positionInParent =
+					DWARFUtil.getMyPositionInParent(referringMember.getHeadFragment());
+				if (positionInParent == -1) {
+					continue;
+				}
+				DWARFNameInfo parentDNI = getName(commonParent);
+				memberName = parentDNI.getName() + "_" + Integer.toString(positionInParent);
+			}
+			if (result.length() > 0) {
+				result.append("_");
+			}
+			result.append(memberName);
+		}
+		return result.toString();
+	}	
+	
 	/**
 	 * Transform a string with a C++ template-like syntax into a hopefully shorter version that
 	 * uses a fixed-length hash of the original string.

Ghidra/Features/Base/src/test.slow/java/ghidra/app/util/bin/format/dwarf4/next/DWARFDataTypeImporterTest.java

@@ -18,9 +18,10 @@ package ghidra.app.util.bin.format.dwarf4.next;
 import static ghidra.app.util.bin.format.dwarf4.encoding.DWARFAttribute.*;
 import static org.junit.Assert.*;
 
-import java.io.IOException;
 import java.util.*;
 
+import java.io.IOException;
+
 import org.junit.Test;
 
 import ghidra.app.util.bin.format.dwarf4.*;
@@ -701,7 +702,59 @@ public class DWARFDataTypeImporterTest extends DWARFTestBase {
 		Structure structdt = (Structure) dataMgr.getDataType(rootCP, "mystruct");
 		DataTypeComponent dtc = structdt.getComponentContaining(14);
 		DataType anonDT = dtc.getDataType();
-		assertEquals("anon_struct_for_f3_f4", anonDT.getName());
+		assertEquals("anon_struct_10_1_a7b17f80_for_f3_f4", anonDT.getName());
+		assertEquals("/DWARF/_UNCATEGORIZED_/mystruct", anonDT.getCategoryPath().getPath());
+	}
+
+	@Test
+	public void testStructAnonNaming()
+			throws CancelledException, IOException, DWARFException {
+		// tests that the dwarf context / location where an anon struct is defined
+		// does not affect where the Ghidra data type is created (in the parent struct's
+		// category path)
+		DebugInfoEntry intDIE = addInt(cu);
+		DebugInfoEntry floatDIE = addFloat(cu);
+
+		//-----------------------
+		DebugInfoEntry anonStructDIE = newStruct(null, 10).create(cu);
+		newMember(anonStructDIE, "blah1", intDIE, 0).create(cu);
+
+		DebugInfoEntry struct1DIE = newStruct("mystruct", 100).create(cu);
+		newMember(struct1DIE, "f1", intDIE, 0).create(cu);
+		newMember(struct1DIE, "f2", floatDIE, 10).create(cu);
+		newMember(struct1DIE, "f3", anonStructDIE, 14).create(cu);
+		newMember(struct1DIE, "f4", anonStructDIE, 54).create(cu);
+		//----------------------
+
+		importAllDataTypes();
+
+		Structure structdt = (Structure) dataMgr.getDataType(rootCP, "mystruct");
+		DataTypeComponent dtc = structdt.getComponentContaining(14);
+		DataType anonDT = dtc.getDataType();
+
+		assertEquals("anon_struct_10_1_a7b17f80_for_f3_f4", anonDT.getName());
+		assertEquals("/DWARF/_UNCATEGORIZED_/mystruct", anonDT.getCategoryPath().getPath());
+	}
+
+	@Test
+	public void testTypedefToAnonStruct()
+			throws CancelledException, IOException, DWARFException {
+		// tests that an anon struct with a typedef to it gets the name of the typedef
+		// and that the typedef itself isn't created
+
+		DebugInfoEntry intDIE = addInt(cu);
+		DebugInfoEntry anonStructDIE = newStruct(null, 10).create(cu);
+		newMember(anonStructDIE, "f1", intDIE, 0).create(cu);
+		newMember(anonStructDIE, "f2", intDIE, 0).create(cu);
+		addTypedef("mystruct", anonStructDIE, cu);
+
+		//----------------------
+
+		importAllDataTypes();
+
+		Structure mystruct = (Structure) dataMgr.getDataType(rootCP, "mystruct");
+		assertNotNull(mystruct);
+		assertEquals(1, dataMgr.getDataTypes(rootCP).length); // should not have an anon_struct_xyz either
 	}
 
 	@Test

Ghidra/Features/Base/src/test.slow/java/ghidra/app/util/bin/format/dwarf4/next/DWARFNameInfoTest.java

@@ -134,7 +134,8 @@ public class DWARFNameInfoTest extends DWARFTestBase {
 		DataType substructDT = dwarfDTM.getDataType(substructDIE.getOffset(), null);
 
 		assertEquals(rootCP.getPath() + "/struct", structDT.getPathName());
-		assertEquals(rootCP.getPath() + "/struct/anon_struct_0", substructDT.getPathName());
+		assertEquals(rootCP.getPath() + "/struct/anon_struct_200_0_00000001",
+			substructDT.getPathName());
 	}
 
 	@Test
@@ -150,7 +151,7 @@ public class DWARFNameInfoTest extends DWARFTestBase {
 		DataType substructDT = dwarfDTM.getDataType(substructDIE.getOffset(), null);
 
 		assertEquals(rootCP.getPath() + "/struct", structDT.getPathName());
-		assertEquals(rootCP.getPath() + "/struct/anon_struct_for_f1",
+		assertEquals(rootCP.getPath() + "/struct/anon_struct_10_0_00000001_for_f1",
 			substructDT.getPathName());
 	}
 
@@ -168,7 +169,7 @@ public class DWARFNameInfoTest extends DWARFTestBase {
 		DataType substructDT = dwarfDTM.getDataType(substructDIE.getOffset(), null);
 
 		assertEquals(rootCP.getPath() + "/struct", structDT.getPathName());
-		assertEquals(rootCP.getPath() + "/struct/anon_struct_for_f1_f2",
+		assertEquals(rootCP.getPath() + "/struct/anon_struct_10_0_00000001_for_f1_f2",
 			substructDT.getPathName());
 	}