GP-1123 Check for name collision when creating placeholder structure

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/symbol/DBTraceFunctionSymbol.java

@@ -253,8 +253,8 @@ public class DBTraceFunctionSymbol extends DBTraceNamespaceSymbol
 		DBTraceBookmarkType errType =
 			manager.trace.getBookmarkManager().getOrDefineBookmarkType(BookmarkType.ERROR);
 		manager.trace.getBookmarkManager()
-				.addBookmark(getLifespan(), entryPoint, errType,
+				.addBookmark(getLifespan(), entryPoint, errType, "Bad Variables Removed",
-					"Bad Variables Removed", "Removed " + badns.size() + " bad variables");
+					"Removed " + badns.size() + " bad variables");
 		for (AbstractDBTraceVariableSymbol s : badns) {
 			s.delete();
 		}
@@ -366,9 +366,9 @@ public class DBTraceFunctionSymbol extends DBTraceNamespaceSymbol
 			}
 			this.callFixup = newCallFixup;
 			update(FIXUP_COLUMN);
-			manager.trace.setChanged(
+			manager.trace
-				new TraceChangeRecord<>(TraceFunctionChangeType.CHANGED_CALL_FIXUP, getSpace(),
+					.setChanged(new TraceChangeRecord<>(TraceFunctionChangeType.CHANGED_CALL_FIXUP,
-					this, oldCallFixup, newCallFixup));
+						getSpace(), this, oldCallFixup, newCallFixup));
 		}
 	}
 
@@ -498,8 +498,8 @@ public class DBTraceFunctionSymbol extends DBTraceNamespaceSymbol
 			}
 			StringBuilder sb = new StringBuilder();
 			DBTraceParameterSymbol retVar = getReturn();
-			sb.append((formalSignature ? retVar.getFormalDataType()
+			sb.append((formalSignature ? retVar.getFormalDataType() : retVar.getDataType())
-					: retVar.getDataType()).getDisplayName());
+					.getDisplayName());
 			sb.append(' ');
 			if (includeCallingConvention && hasExplicitCallingConvention()) {
 				String cc = getCallingConventionName();
@@ -1626,9 +1626,9 @@ public class DBTraceFunctionSymbol extends DBTraceNamespaceSymbol
 			else {
 				andFlags(NO_RETURN_CLEAR);
 			}
-			manager.trace.setChanged(
+			manager.trace
-				new TraceChangeRecord<>(TraceFunctionChangeType.CHANGED_NORETURN, getSpace(), this,
+					.setChanged(new TraceChangeRecord<>(TraceFunctionChangeType.CHANGED_NORETURN,
-					!hasNoReturn, hasNoReturn));
+						getSpace(), this, !hasNoReturn, hasNoReturn));
 		}
 	}
 
@@ -1833,9 +1833,11 @@ public class DBTraceFunctionSymbol extends DBTraceNamespaceSymbol
 			// NOTE: Check for existence first, to avoid resolving unnecessarily.
 			// TODO: If ever struct-class are strongly related, fix that here, too.
 			classStruct = VariableUtilities.findOrCreateClassStruct((GhidraClass) parentNS, dtm);
+			if (classStruct != null) {
 				dtm.resolve(classStruct, null);
 			}
 		}
+	}
 
 	@Override
 	public boolean isThunk() {
@@ -1858,8 +1860,7 @@ public class DBTraceFunctionSymbol extends DBTraceNamespaceSymbol
 	}
 
 	private List<Address> getFunctionThunkAddresses(long functionKey, boolean recursive) {
-		Collection<DBTraceFunctionSymbol> thunkSymbols =
+		Collection<DBTraceFunctionSymbol> thunkSymbols = manager.functionsByThunked.get(getKey());
-			manager.functionsByThunked.get(getKey());
 		if (thunkSymbols == null || thunkSymbols.isEmpty()) {
 			return null;
 		}
@@ -1916,8 +1917,7 @@ public class DBTraceFunctionSymbol extends DBTraceNamespaceSymbol
 		try (LockHold hold = LockHold.lock(manager.lock.readLock())) {
 			Set<Function> result = new HashSet<>();
 			for (DBTraceReference ref : manager.trace.getReferenceManager()
-					.getReferencesToRange(
+					.getReferencesToRange(lifespan, new AddressRangeImpl(entryPoint, entryPoint))) {
-						lifespan, new AddressRangeImpl(entryPoint, entryPoint))) {
 				if (monitor.isCancelled()) {
 					break;
 				}
@@ -1942,8 +1942,7 @@ public class DBTraceFunctionSymbol extends DBTraceNamespaceSymbol
 			Set<Function> result = new HashSet<>();
 			for (AddressRange rng : getBody()) {
 				for (DBTraceReference ref : manager.trace.getReferenceManager()
-						.getReferencesFromRange(
+						.getReferencesFromRange(lifespan, rng)) {
-							lifespan, rng)) {
 					if (monitor.isCancelled()) {
 						return result;
 					}

Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/FillOutStructureCmd.java

@@ -516,6 +516,9 @@ public class FillOutStructureCmd extends BackgroundCommand {
 				return null;
 			}
 			Structure structDT = VariableUtilities.findOrCreateClassStruct(f);
+			if (structDT == null) {
+				return null;
+			}
 // FIXME: How should an existing packed structure be handled? Growing and offset-based placement does not apply
 			int len = structDT.isZeroLength() ? 0 : structDT.getLength();
 			if (len < size) {

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/function/FunctionDB.java

@@ -209,7 +209,7 @@ public class FunctionDB extends DatabaseObject implements Function {
 	@Override
 	public ExternalLocation getExternalLocation() {
 		if (isExternal()) {
-			ExternalManagerDB extMgr = (ExternalManagerDB) program.getExternalManager();
+			ExternalManagerDB extMgr = program.getExternalManager();
 			return extMgr.getExternalLocation(getSymbol());
 		}
 		return null;
@@ -993,7 +993,7 @@ public class FunctionDB extends DatabaseObject implements Function {
 					v.setStorageAndDataType(storage, var.getDataType());
 				}
 				else {
-					SymbolManager symbolMgr = (SymbolManager) program.getSymbolTable();
+					SymbolManager symbolMgr = program.getSymbolTable();
 					VariableSymbolDB s = symbolMgr.createVariableSymbol(name, this,
 						SymbolType.LOCAL_VAR, firstUseOffset, storage, source);
 					s.setStorageAndDataType(storage, var.getDataType());
@@ -1460,7 +1460,7 @@ public class FunctionDB extends DatabaseObject implements Function {
 			}
 
 			// Append new parameters if needed
-			SymbolManager symbolMgr = (SymbolManager) program.getSymbolTable();
+			SymbolManager symbolMgr = program.getSymbolTable();
 			for (int i = newParamIndex; i < newParams.size(); i++) {
 				Variable newParam = newParams.get(i);
 				DataType dt = (newParam instanceof Parameter && !useCustomStorage)
@@ -1683,7 +1683,7 @@ public class FunctionDB extends DatabaseObject implements Function {
 							}
 						}
 					}
-					SymbolManager symbolMgr = (SymbolManager) program.getSymbolTable();
+					SymbolManager symbolMgr = program.getSymbolTable();
 					VariableSymbolDB s = symbolMgr.createVariableSymbol(name, this,
 						SymbolType.PARAMETER, ordinal, storage, paramSource);
 					s.setStorageAndDataType(storage, var.getDataType());
@@ -2671,9 +2671,11 @@ public class FunctionDB extends DatabaseObject implements Function {
 			// relationship between a class namespace and its structure is needed
 			// so its name and category can track properly.
 			classStruct = VariableUtilities.findOrCreateClassStruct(this);
+			if (classStruct != null) {
 				dataTypeManager.resolve(classStruct, null);
 			}
 		}
+	}
 
 	void dataTypeChanged(VariableDB var) {
 		manager.functionChanged(this,

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/VariableUtilities.java

@@ -750,17 +750,37 @@ public class VariableUtilities {
 	 * Create an empty placeholder class structure whose category is derived from
 	 * the function's class namespace.  NOTE: The structure will not be added to the data
 	 * type manager.
+	 * The structure is created in the program's preferred category, or in the root category
+	 * if a preferred category has not been set.  If a colliding data-type matching the class name
+	 * and category already exists, null is returned.
 	 * @param classNamespace class namespace
 	 * @param dataTypeManager data type manager's whose data organization should be applied.
-	 * @return new class structure
+	 * @return new class structure (or null if there is a collision)
 	 */
 	private static Structure createPlaceholderClassStruct(GhidraClass classNamespace,
 			DataTypeManager dataTypeManager) {
 
 		Namespace classParentNamespace = classNamespace.getParentNamespace();
+		CategoryPath prefRoot = null;
+		if (dataTypeManager instanceof ProgramBasedDataTypeManager) {
+			prefRoot = ((ProgramBasedDataTypeManager) dataTypeManager).getProgram()
+					.getPreferredRootNamespaceCategoryPath();
+		}
+		if (prefRoot == null) {
+			prefRoot = CategoryPath.ROOT;
+		}
 		CategoryPath category =
-			DataTypeUtilities.getDataTypeCategoryPath(CategoryPath.ROOT, classParentNamespace);
+			DataTypeUtilities.getDataTypeCategoryPath(prefRoot, classParentNamespace);
 
+		DataType existingDT = dataTypeManager.getDataType(category, classNamespace.getName());
+		if (existingDT != null) {
+			// If a data-type already exists in the parent, try to create in the child
+			category = DataTypeUtilities.getDataTypeCategoryPath(prefRoot, classNamespace);
+			existingDT = dataTypeManager.getDataType(category, classNamespace.getName());
+			if (existingDT != null) {	// If this also already exists
+				return null;			// Don't create a placeholder
+			}
+		}
 		StructureDataType structDT =
 			new StructureDataType(category, classNamespace.getName(), 0, dataTypeManager);
 		structDT.setDescription("PlaceHolder Class Structure");
@@ -782,6 +802,9 @@ public class VariableUtilities {
 	 * and may refer to a non-existing category path which corresponds to the class-parent's 
 	 * namespace.
 	 * 
+	 * If an unrelated data-type already exists matching the class name and category,
+	 * null is returned.
+	 * 
 	 * @param classNamespace class namespace
 	 * @param dataTypeManager data type manager which should be searched and whose
 	 * data organization should be used.
@@ -811,6 +834,9 @@ public class VariableUtilities {
 	 * and may refer to a non-existing category path which corresponds to the class-parent's 
 	 * namespace.
 	 * 
+	 * If the function is not part of a class, or if an unrelated data-type already exists with
+	 * the class's name and category, null is returned.
+	 * 
 	 * @param function function's whose class namespace is the basis for the structure
 	 * @return new or existing structure whose name matches the function's class namespace or
 	 * null if function not contained within a class namespace.