From ba536a57eeb2042178deaa483eb806dee70a5d30 Mon Sep 17 00:00:00 2001
From: ghizard <50744617+ghizard@users.noreply.github.com>
Date: Mon, 14 Nov 2022 13:36:52 -0500
Subject: [PATCH] GP-2838 - PDB Fix primary symbol override logic for
 functions, affecting function sigs from mangled.

---
 .../pdbapplicator/DefaultPdbApplicator.java   | 15 +++++++
 .../pdbapplicator/FunctionSymbolApplier.java  | 39 ++++++++++++-------
 2 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/Ghidra/Features/PDB/src/main/java/ghidra/app/util/pdb/pdbapplicator/DefaultPdbApplicator.java b/Ghidra/Features/PDB/src/main/java/ghidra/app/util/pdb/pdbapplicator/DefaultPdbApplicator.java
index 0773822eb8..9d50e87ff7 100644
--- a/Ghidra/Features/PDB/src/main/java/ghidra/app/util/pdb/pdbapplicator/DefaultPdbApplicator.java
+++ b/Ghidra/Features/PDB/src/main/java/ghidra/app/util/pdb/pdbapplicator/DefaultPdbApplicator.java
@@ -272,6 +272,21 @@ public class DefaultPdbApplicator implements PdbApplicator {
 //		return;
 //	}
 
+		// WANTED TO put the following block in place of the one beneath it, but it would require
+		// that we visit all appliers to make sure they have the requisite logic to override
+		// primary mangled symbols with the appropriate global symbols that have the data types.
+		// See FunctionSymbolApplier for logic used in the "if" case below.
+
+//		// Processing public (mangled) symbols first, but global symbol processing can change
+//		// which symbol is marked primary to the global one if that global symbol provided a rich
+//		// function definition data type.  Doing this will prevent the mangled symbol from applying
+//		// the function signature (unless there is an option set to force the mangled symbol to be
+//		// the primary symbol).
+//		processPublicSymbols();
+//		processGlobalSymbolsNoTypedefs();
+
+		// WANTED TO replace the following block with the one above.  See comment above.
+
 		// Doing globals before publics, as publics are those that can have mangled names.  By
 		// applying the non-mangled symbols first, we can get full type information from the
 		// underlying type.  Then we can apply the mangled symbols and demangle them without
diff --git a/Ghidra/Features/PDB/src/main/java/ghidra/app/util/pdb/pdbapplicator/FunctionSymbolApplier.java b/Ghidra/Features/PDB/src/main/java/ghidra/app/util/pdb/pdbapplicator/FunctionSymbolApplier.java
index 484eac9c29..bfaf8b744b 100644
--- a/Ghidra/Features/PDB/src/main/java/ghidra/app/util/pdb/pdbapplicator/FunctionSymbolApplier.java
+++ b/Ghidra/Features/PDB/src/main/java/ghidra/app/util/pdb/pdbapplicator/FunctionSymbolApplier.java
@@ -252,17 +252,22 @@ public class FunctionSymbolApplier extends MsSymbolApplier {
 	}
 
 	private boolean applyFunction(TaskMonitor monitor) {
-		applicator.createSymbol(address, getName(), true);
 		function = createFunction(monitor);
 		if (function == null) {
 			return false;
 		}
 
+		boolean succeededSetFunctionSignature = false;
 		if (!function.isThunk() &&
 			function.getSignatureSource().isLowerPriorityThan(SourceType.IMPORTED)) {
-			setFunctionDefinition(monitor);
+			succeededSetFunctionSignature = setFunctionDefinition(monitor);
 			function.setNoReturn(isNonReturning);
 		}
+		// If signature was set, then override existing primary mangled symbol with
+		// the global symbol that provided this signature so that Demangler does not overwrite
+		// the richer data type we get with global symbols.
+		applicator.createSymbol(address, getName(), succeededSetFunctionSignature);
+
 		currentFrameSize = 0;
 		return true;
 	}
@@ -289,11 +294,16 @@ public class FunctionSymbolApplier extends MsSymbolApplier {
 		return myFunction;
 	}
 
+	/**
+	 * returns true only if we set a function signature
+	 * @param monitor monitor
+	 * @return true if function signature was set
+	 */
 	private boolean setFunctionDefinition(TaskMonitor monitor) {
 		if (procedureSymbol == null) {
 			// TODO: is there anything we can do with thunkSymbol?
 			// long x = thunkSymbol.getParentPointer();
-			return true;
+			return false;
 		}
 		// Rest presumes procedureSymbol.
 		RecordNumber typeRecordNumber = procedureSymbol.getTypeRecordNumber();
@@ -308,23 +318,24 @@ public class FunctionSymbolApplier extends MsSymbolApplier {
 				((PrimitiveTypeApplier) applier).isNoType())) {
 				applicator.appendLogMsg("Error: Failed to resolve datatype RecordNumber " +
 					typeRecordNumber + " at " + address);
-				return false;
 			}
+			return false;
 		}
 
 		DataType dataType = applier.getDataType();
 		// Since we know the applier is an AbstractionFunctionTypeApplier, then dataType is either
 		//  FunctionDefinition or no type (typedef).
-		if (dataType instanceof FunctionDefinition) {
-			FunctionDefinition def = (FunctionDefinition) dataType;
-			ApplyFunctionSignatureCmd sigCmd =
-				new ApplyFunctionSignatureCmd(address, def, SourceType.IMPORTED);
-			if (!sigCmd.applyTo(applicator.getProgram(), monitor)) {
-				applicator.appendLogMsg(
-					"PDB Warning: Failed to apply signature to function at address " + address +
-						" due to " + sigCmd.getStatusMsg() + "; dataType: " + def.getName());
-				return false;
-			}
+		if (!(dataType instanceof FunctionDefinition)) {
+			return false;
+		}
+		FunctionDefinition def = (FunctionDefinition) dataType;
+		ApplyFunctionSignatureCmd sigCmd =
+			new ApplyFunctionSignatureCmd(address, def, SourceType.IMPORTED);
+		if (!sigCmd.applyTo(applicator.getProgram(), monitor)) {
+			applicator.appendLogMsg(
+				"PDB Warning: Failed to apply signature to function at address " + address +
+					" due to " + sigCmd.getStatusMsg() + "; dataType: " + def.getName());
+			return false;
 		}
 		return true;
 	}