GP-1662 - Demangler - Fixed the MS demangler incorrectly creating

parameters types

Ghidra/Features/Base/src/main/java/ghidra/app/cmd/function/ApplyFunctionSignatureCmd.java

@@ -76,10 +76,6 @@ public class ApplyFunctionSignatureCmd extends BackgroundCommand {
 		this.setName = setName;
 	}
 
-	/**
-	 *
-	 * @see ghidra.framework.cmd.BackgroundCommand#applyTo(ghidra.framework.model.DomainObject, ghidra.util.task.TaskMonitor)
-	 */
 	@Override
 	public boolean applyTo(DomainObject obj, TaskMonitor monitor) {
 		program = (Program) obj;
@@ -111,10 +107,10 @@ public class ApplyFunctionSignatureCmd extends BackgroundCommand {
 
 	/**
 	 * Sets a function's signature in the program.
-	 * @param program The program containing the function.
 	 * @param func the function
 	 * @param signature the signature to apply
-	 * @param preserveCallingConvention if true, the functions calling convention will not be modified
+	 * @param preserveCallingConvention if true, the functions calling convention will not be
+	 * 			modified
 	 * @param forceName force the name of the signature onto the function
 	 *                  normally the name is only set on default function names (not user-defined).
 	 * @param source the source of this function signature
@@ -224,7 +220,7 @@ public class ApplyFunctionSignatureCmd extends BackgroundCommand {
 		PrototypeModel convention = function.getCallingConvention();
 		if (convention == null || !preserveCallingConvention) {
 			convention = preferredModel;
-// NOTE: This has been disable since it can cause imported signature information to be 
+// NOTE: This has been disable since it can cause imported signature information to be
 // ignored and overwritten by subsequent analysis
 //			if (convention == null && compilerSpec.getCallingConventions().length > 1) {
 //				// use default source for signature if convention is really unknown so that we
@@ -337,12 +333,13 @@ public class ApplyFunctionSignatureCmd extends BackgroundCommand {
 	 * The C language assumes array datatypes are passed simply as pointers (by reference) even though
 	 * other datatypes are passed by value.  This routine converts the datatype to the appropriate pointer
 	 * in situations where we need to get at the exact type being passed by "value"
-	 * @param dt
+	 * @param dt the type
-	 * @return
+	 * @param dtm the data type manager
+	 * @return the updated type
 	 */
-	public static DataType settleCDataType(DataType dt, DataTypeManager dtm) {
+	private static DataType settleCDataType(DataType dt, DataTypeManager dtm) {
 		if (dt == null) {
-			return dt;
+			return null;
 		}
 		DataType baseType = dt;
 		if (baseType instanceof TypedefDataType) {

Ghidra/Features/MicrosoftDemangler/src/test/java/ghidra/app/util/demangler/microsoft/MicrosoftDemanglerTest.java

@@ -24,6 +24,7 @@ import generic.test.AbstractGenericTest;
 import ghidra.app.util.demangler.*;
 import ghidra.program.database.ProgramDB;
 import ghidra.program.model.address.Address;
+import ghidra.program.model.listing.*;
 import ghidra.program.model.symbol.SourceType;
 import ghidra.program.model.symbol.SymbolTable;
 import ghidra.test.ToyProgramBuilder;
@@ -40,6 +41,31 @@ public class MicrosoftDemanglerTest extends AbstractGenericTest {
 		program = builder.getProgram();
 	}
 
+	@Test
+	public void testUnsignedShortParameter() throws Exception {
+
+		String mangled = "?InvokeHelperV@COleDispatchDriver@@QAEXJGGPAXPBEPAD@Z";
+
+		MicrosoftDemangler demangler = new MicrosoftDemangler();
+		DemangledObject demangledObject = demangler.demangle(mangled);
+
+		int txID = program.startTransaction("Test");
+
+		SymbolTable st = program.getSymbolTable();
+		st.createLabel(addr("01001000"), mangled, SourceType.ANALYSIS);
+
+		DemanglerOptions options = new DemanglerOptions();
+		demangledObject.applyTo(program, addr("01001000"), options, TaskMonitor.DUMMY);
+		program.endTransaction(txID, true);
+
+		FunctionManager fm = program.getFunctionManager();
+		Function function = fm.getFunctionAt(addr("01001000"));
+		Parameter[] parameters = function.getParameters();
+
+		// this was broken at one point, returning 'unsigned_short'
+		assertEquals("ushort", parameters[2].getDataType().getName());
+	}
+
 	@Test
 	public void testArrayVariable() throws Exception { // NullPointerException
 		String mangled = "?Te@NS1@BobsStuff@@0QAY0BAA@$$CBIA";
@@ -72,9 +98,8 @@ public class MicrosoftDemanglerTest extends AbstractGenericTest {
 		String mangled = "??_R0?AVCBob@@@8~";
 
 		MicrosoftDemangler demangler = new MicrosoftDemangler();
-		DemangledObject demangledObj = null;
 		try {
-			demangledObj = demangler.demangle(mangled);
+			demangler.demangle(mangled);
 		}
 		catch (DemangledException e) {
 			// Expected
@@ -88,9 +113,8 @@ public class MicrosoftDemanglerTest extends AbstractGenericTest {
 		String mangled = "?BobsStuffIO@344GPAUHINSTANCE__@@U_COMMPROP@@*E";
 
 		MicrosoftDemangler demangler = new MicrosoftDemangler();
-		DemangledObject demangledObj = null;
 		try {
-			demangledObj = demangler.demangle(mangled);
+			demangler.demangle(mangled);
 		}
 		catch (DemangledException e) {
 			// Expected
@@ -104,9 +128,8 @@ public class MicrosoftDemanglerTest extends AbstractGenericTest {
 		String mangled = "?BobsStuffIO@344GPAUHINSTANCE__@@U_COMMPROP@@-W";
 
 		MicrosoftDemangler demangler = new MicrosoftDemangler();
-		DemangledObject demangledObj = null;
 		try {
-			demangledObj = demangler.demangle(mangled);
+			demangler.demangle(mangled);
 		}
 		catch (DemangledException e) {
 			// Expected
@@ -120,9 +143,8 @@ public class MicrosoftDemanglerTest extends AbstractGenericTest {
 		String mangled = "?BobsStuffIO@344GPAUHINSTANCE__@@U_COMMPROP@@?W";
 
 		MicrosoftDemangler demangler = new MicrosoftDemangler();
-		DemangledObject demangledObj = null;
 		try {
-			demangledObj = demangler.demangle(mangled);
+			demangler.demangle(mangled);
 		}
 		catch (DemangledException e) {
 			// Expected
@@ -136,9 +158,8 @@ public class MicrosoftDemanglerTest extends AbstractGenericTest {
 		String mangled = "?BobsStuffIO@344GPAUHINSTANCE__@@U_COMMPROP@@~W";
 
 		MicrosoftDemangler demangler = new MicrosoftDemangler();
-		DemangledObject demangledObj = null;
 		try {
-			demangledObj = demangler.demangle(mangled);
+			demangler.demangle(mangled);
 		}
 		catch (DemangledException e) {
 			// Expected
@@ -152,9 +173,8 @@ public class MicrosoftDemanglerTest extends AbstractGenericTest {
 		String mangled = "?BobsStuffIO@344GPAUHINSTANCE__@@U_COMMPROP@@%W";
 
 		MicrosoftDemangler demangler = new MicrosoftDemangler();
-		DemangledObject demangledObj = null;
 		try {
-			demangledObj = demangler.demangle(mangled);
+			demangler.demangle(mangled);
 		}
 		catch (DemangledException e) {
 			// Expected
@@ -168,9 +188,8 @@ public class MicrosoftDemanglerTest extends AbstractGenericTest {
 		String mangled = "?BobsStuffIO@344GPAUHINSTANCE__@@U_COMMPROP@@`W";
 
 		MicrosoftDemangler demangler = new MicrosoftDemangler();
-		DemangledObject demangledObj = null;
 		try {
-			demangledObj = demangler.demangle(mangled);
+			demangler.demangle(mangled);
 		}
 		catch (DemangledException e) {
 			// Expected
@@ -184,9 +203,8 @@ public class MicrosoftDemanglerTest extends AbstractGenericTest {
 		String mangled = "?BobsStuffIO@344GPAUHINSTANCE__@@U_COMMPROP@@+W";
 
 		MicrosoftDemangler demangler = new MicrosoftDemangler();
-		DemangledObject demangledObj = null;
 		try {
-			demangledObj = demangler.demangle(mangled);
+			demangler.demangle(mangled);
 		}
 		catch (DemangledException e) {
 			// Expected
@@ -200,9 +218,8 @@ public class MicrosoftDemanglerTest extends AbstractGenericTest {
 		String mangled = "?BobsStuffIO@344GPAUHINSTANCE__@@U_COMMPROP@@/W";
 
 		MicrosoftDemangler demangler = new MicrosoftDemangler();
-		DemangledObject demangledObj = null;
 		try {
-			demangledObj = demangler.demangle(mangled);
+			demangler.demangle(mangled);
 		}
 		catch (DemangledException e) {
 			// Expected

Ghidra/Features/MicrosoftDmang/src/main/java/mdemangler/MDMangGhidra.java

@@ -839,10 +839,14 @@ public class MDMangGhidra extends MDMang {
 	 */
 	private String getDataTypeName(MDDataType dataType) {
 		String name = dataType.getName();
-		if (name.isBlank()) {
+		if (!name.isBlank()) {
-			return dataType.toString();
+			return name;
 		}
-		return name;
+		name = dataType.getTypeName();
+		if (!name.isBlank()) {
+			return name;
+		}
+		return dataType.toString();
 	}
 }
 

Ghidra/Framework/Project/src/main/java/ghidra/framework/cmd/BackgroundCommand.java

@@ -17,20 +17,15 @@ package ghidra.framework.cmd;
 
 import ghidra.framework.model.DomainObject;
 import ghidra.util.task.TaskMonitor;
-import ghidra.util.task.TaskMonitorAdapter;
 
 /**
- * Abstract command that will be run in a thread (in the background)
+ * Abstract command that will be run in a thread (in the background) other than the AWT(GUI)
- * other than the AWT(GUI) thread.  Use this to apply a long running
+ * thread.  Use this to apply a long running command that is cancellable.
- * command that is interruptable.
- * 
- * The monitor allows the command to display status information as it
- * executes.
- * 
- * This allows commands to make changes in the background so that the
- * GUI is not frozen and the user can still interact with the GUI.
  * 
+ * <p>The monitor allows the command to display status information as it executes.
  * 
+ * <p>This allows commands to make changes in the background so that the GUI is not frozen and the
+ * user can still interact with the GUI.
  */
 public abstract class BackgroundCommand implements Command {
 
@@ -51,22 +46,17 @@ public abstract class BackgroundCommand implements Command {
 		this.isModal = isModal;
 	}
 
-	/*
-	 * @see ghidra.framework.cmd.Command#applyTo(ghidra.framework.model.DomainObject)
-	 */
 	@Override
 	public final boolean applyTo(DomainObject obj) {
-		return applyTo(obj, TaskMonitorAdapter.DUMMY_MONITOR);
+		return applyTo(obj, TaskMonitor.DUMMY);
 	}
 
 	/**
-	 * Method called when this command is to apply changes to the
+	 * Method called when this command is to apply changes to the given domain object.  A monitor
-	 * given domain object.  A monitor is provided to display status
+	 * is provided to display status information about the command as it executes in the background.
-	 * information about the command as it executes in the background.
 	 * 
 	 * @param obj domain object that will be affected by the command
 	 * @param monitor monitor to show progress of the command
-	 * 
 	 * @return true if the command applied successfully
 	 */
 	public abstract boolean applyTo(DomainObject obj, TaskMonitor monitor);

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/ParameterDefinitionImpl.java

@@ -61,7 +61,7 @@ public class ParameterDefinitionImpl implements ParameterDefinition {
 	 * <ul>
 	 * <li>Function definition datatype</li>
 	 * <li>An unsized/zero-element array</li>
-	 * </ul>  
+	 * </ul>
 	 * @param dataType datatype to be checked.  If null specified the DEFAULT datatype will be returned.
 	 * @param dtMgr target datatype manager (null permitted which will adopt default data organization)
 	 * @param voidOK true if checking return datatype and void is allow, else false.
@@ -159,7 +159,7 @@ public class ParameterDefinitionImpl implements ParameterDefinition {
 
 	@Override
 	public String toString() {
-		return dataType.getName() + " " + name;
+		return dataType.getName() + " " + (name == null ? "" : name);
 	}
 
 }