GT-3481 - Gnu Demangler - Checkpoint 3 - Ready for review

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

@@ -212,6 +212,7 @@ public abstract class AbstractDemanglerAnalyzer extends AbstractAnalyzer {
 	protected void apply(Program program, Address address, DemangledObject demangled,
 			DemanglerOptions options, MessageLog log, TaskMonitor monitor) {
 
+		String errorMessage = null;
 		try {
 			if (demangled.applyTo(program, address, options, monitor)) {
 				return;
@@ -219,16 +220,20 @@ public abstract class AbstractDemanglerAnalyzer extends AbstractAnalyzer {
 		}
 		catch (Exception e) {
 			String message = e.getMessage();
-			message = message == null ? "" : ".  Message: " + message;
+			if (message == null) {
-			log.appendMsg(getName(),
+				message = "";
-				"Unable to demangle symbol at " + address + "; name: " +
+			}
-					demangled.getMangledName() + message);
+			errorMessage = "\n" + e.getClass().getSimpleName() + ' ' + message;
+		}
+
+		String failMessage = " (" + getName() + "/" + demangled.getClass().getName() + ")";
+		if (errorMessage != null) {
+			failMessage += errorMessage;
 		}
 
 		log.appendMsg(getName(),
 			"Failed to apply mangled symbol at " + address + "; name:  " +
-				demangled.getMangledName() + " (" +
+				demangled.getMangledName() + failMessage);
-				getName() + "/" + demangled.getClass().getName() + ")");
 	}
 
 	protected String cleanSymbol(Address address, String name) {

Ghidra/Features/GnuDemangler/src/main/java/ghidra/app/plugin/core/analysis/GnuDemanglerAnalyzer.java

@@ -51,7 +51,7 @@ public class GnuDemanglerAnalyzer extends AbstractDemanglerAnalyzer {
 		"Signals to use the deprecated demangler when the modern demangler cannot demangle a " +
 			"given string";
 
-	private static final String OPTION_NAME_DEMANGLER_PARAMETERS =
+	static final String OPTION_NAME_DEMANGLER_PARAMETERS =
 		"Use External Demangler Options";
 	private static final String OPTION_DESCRIPTION_DEMANGLER_PARAMETERS =
 		"Signals to use pass the given parameters to the demangler program";

Ghidra/Features/GnuDemangler/src/test/java/ghidra/app/plugin/core/analysis/GnuDemanglerAnalyzerTest.java

@@ -17,27 +17,38 @@ package ghidra.app.plugin.core.analysis;
 
 import static org.junit.Assert.*;
 
+import java.util.Arrays;
+
 import org.junit.Before;
 import org.junit.Test;
 
 import ghidra.app.cmd.label.AddLabelCmd;
+import ghidra.app.util.demangler.gnu.GnuDemanglerOptions;
 import ghidra.app.util.importer.MessageLog;
 import ghidra.framework.options.Options;
 import ghidra.program.database.ProgramBuilder;
 import ghidra.program.database.ProgramDB;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.listing.Program;
-import ghidra.program.model.symbol.SourceType;
+import ghidra.program.model.symbol.*;
 import ghidra.test.AbstractGhidraHeadlessIntegrationTest;
 import ghidra.test.ToyProgramBuilder;
 import ghidra.util.Msg;
-import ghidra.util.exception.RollbackException;
+import ghidra.util.StringUtilities;
 import ghidra.util.task.TaskMonitor;
 
 public class GnuDemanglerAnalyzerTest extends AbstractGhidraHeadlessIntegrationTest {
 
-	private GnuDemanglerAnalyzer analyzer = new GnuDemanglerAnalyzer();
 	private ProgramDB program;
+	private GnuDemanglerAnalyzer analyzer = new GnuDemanglerAnalyzer();
+
+	// overridden to prevent stack traces from appearing in the console
+	private MessageLog log = new MessageLog() {
+		@Override
+		public void appendException(Throwable t) {
+			appendMsg(t.toString());
+		}
+	};
 
 	@Before
 	public void setUp() throws Exception {
@@ -48,8 +59,34 @@ public class GnuDemanglerAnalyzerTest extends AbstractGhidraHeadlessIntegrationT
 		registerOptions();
 	}
 
+	@Override
+	protected void testFailed(Throwable e) {
+		Msg.error(this, "Test failed - analysis log:\n" + log);
+	}
+
 	@Test
-	public void testDeprectedDemangledString() throws Exception {
+	public void testDeprectedMangledString_WithoutDeprecatedDemangler() throws Exception {
+
+		//
+		// The below demangles to MsoDAL::VertFrame::__dt( (void))
+		// note the (void) syntax
+		//
+		// from program Microsoft Entourage
+		//
+		String mangled = "__dt__Q26MsoDAL9VertFrameFv";
+
+		Address addr = addr("0x110");
+		createSymbol(addr, mangled);
+
+		setOption(GnuDemanglerAnalyzer.OPTION_NAME_USE_DEPRECATED_DEMANGLER, false);
+
+		analyze();
+
+		assertNotDemangled(addr, "__dt");
+	}
+
+	@Test
+	public void testDeprectedMangledString_WithDeprecatedDemangler() throws Exception {
 
 		//
 		// The below demangles to MsoDAL::VertFrame::__dt( (void))
@@ -64,57 +101,214 @@ public class GnuDemanglerAnalyzerTest extends AbstractGhidraHeadlessIntegrationT
 
 		setOption(GnuDemanglerAnalyzer.OPTION_NAME_USE_DEPRECATED_DEMANGLER, true);
 
-		MessageLog log = new MessageLog();
+		analyze();
-		analyzer.added(program, program.getMemory(), TaskMonitor.DUMMY, log);
+
+		assertDemangled(addr, "__dt");
 	}
 
-	private void setOption(String optionNameUseDeprecatedDemangler, boolean b) {
+	@Test
+	public void testMangledString_WithArguments_Valid() {
 
+		//
+		// The below demangles to std::io::Read::read_to_end
+		//
+		String mangled = "_ZN3std2io4Read11read_to_end17hb85a0f6802e14499E";
+
+		Address addr = addr("0x110");
+		createSymbol(addr, mangled);
+
+		setOption(GnuDemanglerAnalyzer.OPTION_NAME_DEMANGLER_PARAMETERS, "-s rust");
+
+		analyze();
+
+		assertDemangled(addr, "read_to_end");
+	}
+
+	@Test
+	public void testMangledString_WithArguments_ValidButWrongFormat() {
+
+		//
+		// The below demangles to std::io::Read::read_to_end
+		//
+		String mangled = "_ZN3std2io4Read11read_to_end17hb85a0f6802e14499E";
+
+		Address addr = addr("0x110");
+		createSymbol(addr, mangled);
+
+		setOption(GnuDemanglerAnalyzer.OPTION_NAME_DEMANGLER_PARAMETERS, "-s dlang");
+
+		analyze();
+
+		assertNotDemangled(addr, "read_to_end");
+	}
+
+	@Test
+	public void testMangledString_WithArguments_Invalid() {
+
+		//
+		// The below demangles to std::io::Read::read_to_end
+		//
+		String mangled = "_ZN3std2io4Read11read_to_end17hb85a0f6802e14499E";
+
+		Address addr = addr("0x110");
+		createSymbol(addr, mangled);
+
+		setOption(GnuDemanglerAnalyzer.OPTION_NAME_DEMANGLER_PARAMETERS, "-s badformatname");
+
+		analyze();
+
+		assertNotDemangled(addr, "read_to_end");
+		assertMessageLogLine("java.io.IOException: Error starting demangler with command");
+		assertMessageLogLine("Invalid options", GnuDemanglerOptions.GNU_DEMANGLER_V2_33_1);
+	}
+
+	@Test
+	public void testDeprecatedMangledString_WithArguments_Invalid() {
+
+		//
+		// The below demangles to std::io::Read::read_to_end
+		//
+		String mangled = "_ZN3std2io4Read11read_to_end17hb85a0f6802e14499E";
+
+		Address addr = addr("0x110");
+		createSymbol(addr, mangled);
+
+		setOption(GnuDemanglerAnalyzer.OPTION_NAME_USE_DEPRECATED_DEMANGLER, true);
+		setOption(GnuDemanglerAnalyzer.OPTION_NAME_DEMANGLER_PARAMETERS, "-s badformatname");
+
+		analyze();
+
+		assertNotDemangled(addr, "read_to_end");
+		assertMessageLogLine("java.io.IOException: Error starting demangler with command");
+		assertMessageLogLine("Invalid options", GnuDemanglerOptions.GNU_DEMANGLER_V2_33_1);
+		assertMessageLogLine("Invalid options", GnuDemanglerOptions.GNU_DEMANGLER_V2_24);
+	}
+
+	@Test
+	public void testDeprecatedMangledString_WithArguments_InvalidModernArguments_ValidDeprecatedArguments() {
+
+		//
+		// The below demangles to std::io::Read::read_to_end
+		//
+		String mangled = "_ZN3std2io4Read11read_to_end17hb85a0f6802e14499E";
+
+		Address addr = addr("0x110");
+		createSymbol(addr, mangled);
+
+		setOption(GnuDemanglerAnalyzer.OPTION_NAME_USE_DEPRECATED_DEMANGLER, true);
+		setOption(GnuDemanglerAnalyzer.OPTION_NAME_DEMANGLER_PARAMETERS, "-s arm");
+
+		analyze();
+
+		assertNotDemangled(addr, "read_to_end");
+		assertMessageLogLine("java.io.IOException: Error starting demangler with command");
+		assertMessageLogLine("Invalid options", GnuDemanglerOptions.GNU_DEMANGLER_V2_33_1);
+		assertMessageNotInLogLine("Invalid options", GnuDemanglerOptions.GNU_DEMANGLER_V2_24);
+	}
+
+	// things missed:
+	// -demangle error case in base class...this is OK
+	// -error case in applyTo method in base class
+
+	// -use deprecated demangler case in validateOptions
+
+//==================================================================================================
+// Private Methods
+//==================================================================================================	
+
+	private void assertMessageLogLine(String... expected) {
+
+		String allMessages = log.toString();
+		String[] logLines = allMessages.split("\n");
+		for (String line : logLines) {
+			if (StringUtilities.containsAllIgnoreCase(line, expected)) {
+				return;
+			}
+		}
+
+		fail("The folllowing source text did not have a line containing:\n" +
+			Arrays.toString(expected) + "\n\nActual Text:\n" + allMessages);
+	}
+
+	private void assertMessageNotInLogLine(String... expected) {
+
+		String allMessages = log.toString();
+		String[] logLines = allMessages.split("\n");
+		for (String line : logLines) {
+			if (StringUtilities.containsAllIgnoreCase(line, expected)) {
+				fail("The folllowing source text unexpectedly has a line containing:\n" +
+					Arrays.toString(expected) + "\n\nActual Text:\n" + allMessages);
+			}
+		}
+	}
+
+	private void analyze() {
+		tx(program, () -> analyzer.added(program, program.getMemory(), TaskMonitor.DUMMY, log));
+	}
+
+	private void assertNotDemangled(Address addr, String name) {
+
+		SymbolTable st = program.getSymbolTable();
+		Symbol[] symbols = st.getSymbols(addr);
+		for (Symbol s : symbols) {
+			if (s.getName().equals(name)) {
+				fail("Symbol should not have been demangled '" + name + "'");
+			}
+		}
+	}
+
+	private void assertDemangled(Address addr, String name) {
+
+		SymbolTable st = program.getSymbolTable();
+		Symbol[] symbols = st.getSymbols(addr);
+		for (Symbol s : symbols) {
+			if (s.getName().equals(name)) {
+				return;
+			}
+		}
+
+		fail("Unable to find demangled symbol '" + name + "'");
+	}
+
+	private void setOption(String optionName, boolean doUse) {
+
+		String fullOptionName = analyzer.getName() + Options.DELIMITER_STRING + optionName;
 		Options options = program.getOptions("Analyzers");
 
 		for (String name : options.getOptionNames()) {
+			if (name.equals(fullOptionName)) {
+				tx(program, () -> options.setBoolean(optionName, doUse));
 
-			if (name.contains("Demangler GNU")) {
+				// we must call this manually, since we are not using a tool
-				Msg.out("found it: " + name);
+				analyzer.optionsChanged(options, program);
-			}
+				return;
-			else {
-				Msg.out("no it: " + name);
 			}
 		}
+
+		fail("Could not find option '" + optionName + "'");
+	}
+
+	private void setOption(String optionName, String value) {
+
+		String fullOptionName = analyzer.getName() + Options.DELIMITER_STRING + optionName;
+		Options options = program.getOptions("Analyzers");
+
+		for (String name : options.getOptionNames()) {
+			if (name.equals(fullOptionName)) {
+				tx(program, () -> options.setString(optionName, value));
+
+				// we must call this manually, since we are not using a tool
+				analyzer.optionsChanged(options, program);
+				return;
+			}
+		}
+
+		fail("Could not find option '" + optionName + "'");
 	}
 
 	private void createSymbol(Address addr, String mangled) {
-
 		AddLabelCmd cmd = new AddLabelCmd(addr, mangled, SourceType.ANALYSIS);
-		int txId = program.startTransaction(cmd.getName());
+		applyCmd(program, cmd);
-		boolean commit = true;
-		try {
-			boolean status = cmd.applyTo(program);
-			program.flushEvents();
-
-			if (!status) {
-				fail("Could not apply command: " + cmd.getStatusMsg());
-			}
-		}
-		catch (RollbackException e) {
-			commit = false;
-			throw e;
-		}
-		finally {
-			program.endTransaction(txId, commit);
-		}
-	}
-
-	@Test
-	public void testDeprectedDemangledString_WithArguments_Valid() {
-
-		fail();
-	}
-
-	@Test
-	public void testDeprectedDemangledString_WithArguments_Invalid() {
-
-		fail();
 	}
 
 	private Address addr(String addr) {
@@ -123,7 +317,6 @@ public class GnuDemanglerAnalyzerTest extends AbstractGhidraHeadlessIntegrationT
 
 	private void registerOptions() {
 		Options options = program.getOptions(Program.ANALYSIS_PROPERTIES);
-
 		Options analyzerOptions = options.getOptions(analyzer.getName());
 		analyzer.registerOptions(analyzerOptions, program);
 	}

Ghidra/Framework/Generic/src/main/java/ghidra/app/util/importer/MessageLog.java

@@ -19,15 +19,13 @@ import ghidra.util.Msg;
 import ghidra.util.exception.AssertException;
 
 /**
- * A simple class to handle logging messages and exceptions.
+ * A simple class to handle logging messages and exceptions.  A maximum message count size 
- * A maximum message count size constraint can be set to clip
+ * constraint can be set to clip messages after a certain number, but still keep incrementing
- * messages after a certain number, but still keep incrementing
  * a running total.
- *
  */
 public class MessageLog {
 	/**
-	 * The default number of messages to store before clipping.
+	 * The default number of messages to store before clipping
 	 */
 	public final static int MAX_COUNT = 500;
 
@@ -38,14 +36,14 @@ public class MessageLog {
 	private String statusMsg;
 
 	/**
-	 * Constructs a new message log using the default message count.
+	 * Constructs a new message log using the default message count
 	 */
 	public MessageLog() {
 		this(MAX_COUNT);
 	}
 
 	/**
-	 * Constructs a new message log using the sepcified message count.
+	 * Constructs a new message log using the specified message count
 	 * @param maxSize the maximum number of messages
 	 */
 	public MessageLog(int maxSize) {
@@ -54,7 +52,7 @@ public class MessageLog {
 	}
 
 	/**
-	 * Copies the contents of one message log into this one.
+	 * Copies the contents of one message log into this one
 	 * @param log the log to copy from
 	 */
 	public void copyFrom(MessageLog log) {
@@ -65,34 +63,40 @@ public class MessageLog {
 	}
 
 	/**
-	 * Appends the message to the log.
+	 * Appends the message to the log
-	 * @param msg the message
+	 * @param message the message
 	 */
-	public void appendMsg(String msg) {
+	public void appendMsg(String message) {
-		msg(msg);
+		msg(message);
 	}
 
-	public void appendMsg(String originator, String msg) {
+	/**
+	 * Appends the message to the log
+	 *
+	 * @param originator the originator of the message 
+	 * @param message the message
+	 */
+	public void appendMsg(String originator, String message) {
 		if (originator == null) {
-			msg(msg);
+			msg(message);
 		}
 		else {
-			msg(originator + "> " + msg);
+			msg(originator + "> " + message);
 		}
 	}
 
 	/**
-	 * Appends the message and line number to the log.
+	 * Appends the message and line number to the log
 	 * @param lineNum the line number that generated the message
-	 * @param msg     the message
+	 * @param message the message
 	 */
-	public void appendMsg(int lineNum, String msg) {
+	public void appendMsg(int lineNum, String message) {
-		msg("Line #" + lineNum + " - " + msg);
+		msg("Line #" + lineNum + " - " + message);
 	}
 
 	/**
-	 * Appends the exception to the log.
+	 * Appends the exception to the log
-	 * @param t the exeception to append to the log
+	 * @param t the exception to append to the log
 	 */
 	public void appendException(Throwable t) {
 		if (t instanceof NullPointerException || t instanceof AssertException) {
@@ -106,7 +110,7 @@ public class MessageLog {
 	}
 
 	/**
-	 * Returns the message count.
+	 * Returns the message count
 	 * @return the message count
 	 */
 	public int getMsgCount() {
@@ -114,8 +118,7 @@ public class MessageLog {
 	}
 
 	/**
-	 * Clears all messages from this log
+	 * Clears all messages from this log and resets the count
-	 * and resets the count.
 	 */
 	public void clear() {
 		buffer = new StringBuffer();
@@ -125,7 +128,7 @@ public class MessageLog {
 
 	/**
 	 * Stores a status message that can be used elsewhere (i.e., populate warning dialogs)
-	 * @param status
+	 * @param status the status message
 	 */
 	public void setStatus(String status) {
 		statusMsg = status;
@@ -146,9 +149,6 @@ public class MessageLog {
 		return statusMsg;
 	}
 
-	/**
-	 * @see java.lang.Object#toString()
-	 */
 	@Override
 	public String toString() {
 		if (count > maxSize) {
@@ -177,12 +177,11 @@ public class MessageLog {
 	/**
 	 * Readable method for appending error messages to the log.
 	 *
-	 * Currently does nothing different that {@link #appendMsg(String, String)}
+	 * <p>Currently does nothing different than {@link #appendMsg(String, String)}.
 	 *
 	 *
-	 * @param originator
+	 * @param originator the originator of the message 
-	 * @param message
+	 * @param message the message
-	 *
 	 */
 	public void error(String originator, String message) {
 		appendMsg(originator, message);