Merge remote-tracking branch 'origin/GP-2966_Dan_registerMappingWithAliases--SQUASHED'

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/mapping/DefaultDebuggerRegisterMapper.java

@@ -51,16 +51,30 @@ public class DefaultDebuggerRegisterMapper implements DebuggerRegisterMapper {
 		collectFilteredLanguageRegs();
 	}
 
-	protected boolean testTraceRegister(Register lReg) {
+	protected boolean includeTraceRegister(Register lReg) {
 		return lReg.isBaseRegister();
 	}
 
 	protected synchronized void collectFilteredLanguageRegs() {
 		for (Register lReg : language.getRegisters()) {
-			if (!testTraceRegister(lReg)) {
+			if (!includeTraceRegister(lReg)) {
 				continue;
 			}
-			filtLanguageRegs.put(normalizeName(lReg.getName()), lReg);
+			putLanguageRegister(filtLanguageRegs, lReg);
+		}
+	}
+
+	protected synchronized void putLanguageRegister(Map<String, Register> map, Register lReg) {
+		map.put(normalizeName(lReg.getName()), lReg);
+		for (String alias : lReg.getAliases()) {
+			map.put(normalizeName(alias), lReg);
+		}
+	}
+
+	protected synchronized void removeLanguageRegister(Map<String, Register> map, Register lReg) {
+		map.remove(normalizeName(lReg.getName()));
+		for (String alias : lReg.getAliases()) {
+			map.remove(normalizeName(alias));
 		}
 	}
 
@@ -70,7 +84,7 @@ public class DefaultDebuggerRegisterMapper implements DebuggerRegisterMapper {
 		if (lReg == null) {
 			return null;
 		}
-		languageRegs.put(name, lReg);
+		putLanguageRegister(languageRegs, lReg);
 		return lReg;
 	}
 
@@ -81,7 +95,7 @@ public class DefaultDebuggerRegisterMapper implements DebuggerRegisterMapper {
 			return null;
 		}
 		targetRegs.put(name, tReg);
-		languageRegs.put(name, lReg);
+		putLanguageRegister(languageRegs, lReg);
 		return lReg;
 	}
 
@@ -92,7 +106,7 @@ public class DefaultDebuggerRegisterMapper implements DebuggerRegisterMapper {
 			return null;
 		}
 		targetRegs.remove(name);
-		languageRegs.remove(name);
+		removeLanguageRegister(languageRegs, lReg);
 		return lReg;
 	}
 
@@ -115,7 +129,17 @@ public class DefaultDebuggerRegisterMapper implements DebuggerRegisterMapper {
 
 	@Override
 	public synchronized TargetRegister traceToTarget(Register lReg) {
-		return targetRegs.get(normalizeName(lReg.getName()));
+		TargetRegister tReg = targetRegs.get(normalizeName(lReg.getName()));
+		if (tReg != null) {
+			return tReg;
+		}
+		for (String alias : lReg.getAliases()) {
+			tReg = targetRegs.get(normalizeName(alias));
+			if (tReg != null) {
+				return tReg;
+			}
+		}
+		return null;
 	}
 
 	@Override

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/mapping/LargestSubDebuggerRegisterMapper.java

@@ -40,7 +40,7 @@ public class LargestSubDebuggerRegisterMapper extends DefaultDebuggerRegisterMap
 	}
 
 	@Override
-	protected boolean testTraceRegister(Register lReg) {
+	protected boolean includeTraceRegister(Register lReg) {
 		return true;
 	}
 
@@ -123,7 +123,7 @@ public class LargestSubDebuggerRegisterMapper extends DefaultDebuggerRegisterMap
 			return null;
 		}
 		Register lReg = subs.last(); // largest
-		return targetRegs.get(normalizeName(lReg.getName()));
+		return super.traceToTarget(lReg);
 	}
 
 	@Override
@@ -156,7 +156,7 @@ public class LargestSubDebuggerRegisterMapper extends DefaultDebuggerRegisterMap
 			Msg.warn(this, "Potential register cache aliasing: " + lReg + " vs " + subs.last());
 			/**
 			 * After testing, there is a problem with data truncation. If, e.g., EAX is sent,
-			 * followed by AX, and we expand to fill RAX, the we're going to truncate the upper 16
+			 * followed by AX, and we expand to fill RAX, then we're going to truncate the upper 16
 			 * bits of EAX :( . To avoid this, we'll only map the largest register from target to
 			 * trace. We'll still warn as a courtesy, but we'll abort the mapping.
 			 */
@@ -164,7 +164,7 @@ public class LargestSubDebuggerRegisterMapper extends DefaultDebuggerRegisterMap
 		}
 		/**
 		 * TODO: A mapping with masks may be useful in the future, but as it is, the trace database
-		 * will reject it. Furthermore, we'd still want to mark the whole base register is known,
+		 * will reject it. Furthermore, we'd still want to mark the whole base register as known,
 		 * which is not intuitive, and makes me worry about that method.
 		 */
 		// return new RegisterValue(lReg, new BigInteger(1,value)).getBaseRegisterValue();

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/record/ObjectBasedTraceRecorder.java

@@ -543,9 +543,9 @@ public class ObjectBasedTraceRecorder implements TraceRecorder {
 	}
 
 	protected TargetRegisterBank isExactRegisterOnTarget(TracePlatform platform,
-			TargetRegisterContainer regContainer, String name) {
+			TargetRegisterContainer regContainer, Register register) {
 		PathMatcher matcher =
-			platform.getConventionalRegisterPath(regContainer.getSchema(), List.of(), name);
+			platform.getConventionalRegisterPath(regContainer.getSchema(), List.of(), register);
 		for (TargetObject targetObject : matcher.getCachedSuccessors(regContainer).values()) {
 			if (!(targetObject instanceof TargetRegister targetRegister)) {
 				continue;
@@ -568,28 +568,7 @@ public class ObjectBasedTraceRecorder implements TraceRecorder {
 		if (regContainer == null) {
 			return null;
 		}
-		TargetRegisterBank result;
-		String name = platform.getConventionalRegisterObjectName(register);
-		result = isExactRegisterOnTarget(platform, regContainer, name);
-		if (result != null) {
-			return result;
-		}
-		// Not totally case insensitive, but the sane cases
-		String upperName = name.toUpperCase();
-		if (!name.equals(upperName)) {
-			result = isExactRegisterOnTarget(platform, regContainer, upperName);
-			if (result != null) {
-				return result;
-			}
-		}
-		String lowerName = name.toLowerCase();
-		if (!name.equals(lowerName)) {
-			result = isExactRegisterOnTarget(platform, regContainer, lowerName);
-			if (result != null) {
-				return result;
-			}
-		}
-		return null;
+		return isExactRegisterOnTarget(platform, regContainer, register);
 	}
 
 	@Override

Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/mapping/DefaultDebuggerRegisterMapperTest.java

@@ -0,0 +1,132 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package ghidra.app.plugin.core.debug.mapping;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import java.util.Objects;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import ghidra.app.plugin.core.debug.gui.AbstractGhidraHeadedDebuggerGUITest;
+import ghidra.app.services.ActionSource;
+import ghidra.app.services.TraceRecorder;
+import ghidra.dbg.model.TestTargetRegister;
+import ghidra.dbg.model.TestTargetRegisterValue;
+import ghidra.dbg.target.TargetObject;
+import ghidra.dbg.target.TargetRegister;
+import ghidra.program.model.lang.*;
+import ghidra.test.ToyProgramBuilder;
+import ghidra.trace.model.thread.TraceThread;
+
+public class DefaultDebuggerRegisterMapperTest extends AbstractGhidraHeadedDebuggerGUITest {
+
+	static class TestTargetMapper extends DefaultDebuggerTargetTraceMapper {
+		public TestTargetMapper(TargetObject target)
+				throws LanguageNotFoundException, CompilerSpecNotFoundException {
+			super(target, new LanguageID(ToyProgramBuilder._TOY64_BE),
+				new CompilerSpecID("default"), Set.of());
+		}
+	}
+
+	protected static void assertSameRegister(TargetRegister expected, TargetObject actual) {
+		if (actual instanceof TestTargetRegister tr) {
+			assertEquals(expected, tr);
+		}
+		else if (actual instanceof TestTargetRegisterValue rv) {
+			assertEquals(expected, rv.desc);
+		}
+		else {
+			fail();
+		}
+	}
+
+	@Before
+	public void setUpMapperTest() throws Throwable {
+		createTestModel();
+		mb.createTestProcessesAndThreads();
+		mb.createTestThreadRegisterBanks();
+	}
+
+	protected DebuggerRegisterMapper doGetRegisterMapper(String toConfirm)
+			throws Throwable {
+		TraceRecorder recorder = modelService.recordTarget(mb.testProcess1,
+			new TestTargetMapper(mb.testProcess1), ActionSource.AUTOMATIC);
+		TraceThread thread1 = waitForValue(() -> recorder.getTraceThread(mb.testThread1));
+		DebuggerRegisterMapper rm = waitForValue(() -> recorder.getRegisterMapper(thread1));
+		waitForValue(() -> rm.getTargetRegister(toConfirm));
+		return rm;
+	}
+
+	protected DebuggerRegisterMapper getRegisterMapper() throws Throwable {
+		mb.testProcess1.regs.addRegistersFromLanguage(getToyBE64Language(), r -> true);
+		return doGetRegisterMapper("r0");
+	}
+
+	protected DebuggerRegisterMapper getRegisterMapperAliased() throws Throwable {
+		Register r0 = getToyBE64Language().getRegister("r0");
+		mb.testProcess1.regs.addRegistersFromLanguage(getToyBE64Language(), r -> r != r0);
+		mb.testProcess1.regs.addRegister("a0", r0);
+		return doGetRegisterMapper("r1");
+	}
+
+	@Test
+	public void testTraceToTargetRegCanonical() throws Throwable {
+		DebuggerRegisterMapper rm = getRegisterMapper();
+		TestTargetRegister tR0 =
+			Objects.requireNonNull(mb.testProcess1.regs.getCachedElements().get("r0"));
+		Register lR0 = Objects.requireNonNull(getToyBE64Language().getRegister("r0"));
+
+		TargetRegister tReg = waitForValue(() -> rm.traceToTarget(lR0));
+		assertSameRegister(tR0, tReg);
+	}
+
+	@Test
+	public void testTargetToTraceRegCanonical() throws Throwable {
+		DebuggerRegisterMapper rm = getRegisterMapper();
+		TestTargetRegister tR0 =
+			Objects.requireNonNull(mb.testProcess1.regs.getCachedElements().get("r0"));
+		Register lR0 = Objects.requireNonNull(getToyBE64Language().getRegister("r0"));
+
+		Register lReg = waitForValue(() -> rm.targetToTrace(tR0));
+		assertEquals(lR0, lReg);
+	}
+
+	@Test
+	public void testTraceToTargetRegAlias() throws Throwable {
+		DebuggerRegisterMapper rm = getRegisterMapperAliased();
+		TestTargetRegister tA0 =
+			Objects.requireNonNull(mb.testProcess1.regs.getCachedElements().get("a0"));
+		Register lR0 = Objects.requireNonNull(getToyBE64Language().getRegister("r0"));
+
+		TargetRegister tReg = waitForValue(() -> rm.traceToTarget(lR0));
+		assertSameRegister(tA0, tReg);
+	}
+
+	@Test
+	public void testTargetToTraceRegAlias() throws Throwable {
+		DebuggerRegisterMapper rm = getRegisterMapperAliased();
+		TestTargetRegister tA0 =
+			Objects.requireNonNull(mb.testProcess1.regs.getCachedElements().get("a0"));
+		Register lR0 = Objects.requireNonNull(getToyBE64Language().getRegister("r0"));
+
+		Register lReg = waitForValue(() -> rm.targetToTrace(tA0));
+		assertEquals(lR0, lReg);
+	}
+}

Ghidra/Debug/Framework-Debugging/src/main/java/ghidra/dbg/util/PathMatcher.java

@@ -33,6 +33,10 @@ public class PathMatcher implements PathPredicates {
 		patterns.add(pattern);
 	}
 
+	public void addAll(PathMatcher matcher) {
+		patterns.addAll(matcher.patterns);
+	}
+
 	@Override
 	public String toString() {
 		return String.format("<PathMatcher\n  %s\n>", StringUtils.join(patterns, "\n  "));

Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/model/TestDebuggerObjectModel.java

@@ -156,6 +156,11 @@ public class TestDebuggerObjectModel extends EmptyDebuggerObjectModel {
 		return TestTargetRegister.fromLanguageRegister(container, register);
 	}
 
+	protected TestTargetRegister newTestTargetRegister(TestTargetRegisterContainer container,
+			String index, Register register) {
+		return TestTargetRegister.fromLanguageRegister(container, index, register);
+	}
+
 	protected TestTargetThreadContainer newTestTargetThreadContainer(TestTargetProcess process) {
 		return new TestTargetThreadContainer(process);
 	}

Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/model/TestTargetRegister.java

@@ -27,10 +27,15 @@ public class TestTargetRegister
 		extends DefaultTestTargetObject<TestTargetObject, TestTargetRegisterContainer>
 		implements TargetRegister {
 
+	public static TestTargetRegister fromLanguageRegister(TestTargetRegisterContainer parent,
+			String index, Register register) {
+		return new TestTargetRegister(parent, PathUtils.makeKey(index),
+			(register.getBitLength() + 7) / 8, register.isProgramCounter());
+	}
+
 	public static TestTargetRegister fromLanguageRegister(
 			TestTargetRegisterContainer parent, Register register) {
-		return new TestTargetRegister(parent, PathUtils.makeKey(register.getName()),
-			(register.getBitLength() + 7) / 8, register.isProgramCounter());
+		return fromLanguageRegister(parent, register.getName(), register);
 	}
 
 	protected final int byteLength;

Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/model/TestTargetRegisterContainer.java

@@ -61,6 +61,11 @@ public class TestTargetRegisterContainer
 	public TestTargetRegister addRegister(Register register) {
 		TestTargetRegister tr =
 			getModel().newTestTargetRegister(this, Objects.requireNonNull(register));
+		doAddRegister(tr, register);
+		return tr;
+	}
+
+	protected void doAddRegister(TestTargetRegister tr, Register register) {
 		String reason = "Added " + register + " from Ghidra language";
 		changeElements(List.of(), List.of(tr), reason);
 		List<AbstractTestTargetRegisterBank<?>> banks;
@@ -70,6 +75,12 @@ public class TestTargetRegisterContainer
 		for (AbstractTestTargetRegisterBank<?> bank : banks) {
 			bank.addRegisterDescs(List.of(tr), reason);
 		}
+	}
+
+	public TestTargetRegister addRegister(String index, Register register) {
+		TestTargetRegister tr =
+			getModel().newTestTargetRegister(this, index, Objects.requireNonNull(register));
+		doAddRegister(tr, register);
 		return tr;
 	}
 

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/guest/InternalTracePlatform.java

@@ -15,8 +15,7 @@
  */
 package ghidra.trace.database.guest;
 
-import java.util.Collection;
-import java.util.List;
+import java.util.*;
 
 import ghidra.dbg.target.TargetObject;
 import ghidra.dbg.target.TargetRegister;
@@ -80,41 +79,58 @@ public interface InternalTracePlatform extends TracePlatform {
 		return result;
 	}
 
+	default List<String> listRegNames(Register register) {
+		Set<String> result = new LinkedHashSet<>();
+		result.add(register.getName());
+		result.add(register.getName().toUpperCase());
+		result.add(register.getName().toLowerCase());
+		for (String alias : register.getAliases()) {
+			result.add(alias);
+			result.add(alias.toUpperCase());
+			result.add(alias.toLowerCase());
+		}
+		return List.copyOf(result);
+	}
+
 	@Override
-	default String getConventionalRegisterObjectName(Register register) {
+	default Collection<String> getConventionalRegisterObjectNames(Register register) {
 		Address pmin = mapGuestToHost(register.getAddress());
 		if (pmin == null) {
-			return register.getName();
+			return listRegNames(register);
 		}
 		TraceSymbolManager symbolManager = getTrace().getSymbolManager();
 		TraceNamespaceSymbol nsRegMap = symbolManager.namespaces().getGlobalNamed(regMap(register));
-		Collection<? extends TraceLabelSymbol> labels = symbolManager.labels()
+		Collection<String> labels = symbolManager.labels()
 				.getAt(0, null, pmin, false)
 				.stream()
 				.filter(s -> s.getParentNamespace() == nsRegMap)
+				.map(TraceSymbol::getName)
 				.toList();
-		if (labels.isEmpty()) {
-			return register.getName();
+		if (!labels.isEmpty()) {
+			return labels;
 		}
-		// primary is listed first, so take it
-		return labels.iterator().next().getName();
+		return listRegNames(register);
 	}
 
 	@Override
 	default PathMatcher getConventionalRegisterPath(TargetObjectSchema schema, List<String> path,
-			String name) {
+			Collection<String> names) {
 		PathMatcher matcher = schema.searchFor(TargetRegister.class, path, true);
 		if (matcher.isEmpty()) {
 			return matcher;
 		}
-		return matcher.applyKeys(Align.RIGHT, List.of(name));
+		PathMatcher result = new PathMatcher();
+		for (String name:names) {
+			result.addAll(matcher.applyKeys(Align.RIGHT, List.of(name)));
+		}
+		return result;
 	}
 
 	@Override
 	default PathMatcher getConventionalRegisterPath(TargetObjectSchema schema, List<String> path,
 			Register register) {
 		return getConventionalRegisterPath(schema, path,
-			getConventionalRegisterObjectName(register));
+			getConventionalRegisterObjectNames(register));
 	}
 
 	@Override
@@ -156,6 +172,11 @@ public interface InternalTracePlatform extends TracePlatform {
 			if (nsRegMap == null) {
 				nsRegMap = namespaces.add(regMap, globals, SourceType.USER_DEFINED);
 			}
+			TraceLabelSymbol exists = symbolManager.labels()
+					.getChildWithNameAt(objectName, getIntKey(), null, hostAddr, nsRegMap);
+			if (exists != null) {
+				return exists;
+			}
 			return symbolManager.labels()
 					.create(0, null, hostAddr, objectName, nsRegMap, SourceType.USER_DEFINED);
 		}

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/guest/TracePlatform.java

@@ -15,6 +15,7 @@
  */
 package ghidra.trace.model.guest;
 
+import java.util.Collection;
 import java.util.List;
 
 import ghidra.dbg.target.TargetObject;
@@ -171,27 +172,29 @@ public interface TracePlatform {
 	AddressRange getConventionalRegisterRange(AddressSpace overlay, Register register);
 
 	/**
-	 * Get the name or index of the register object for the given platform register
+	 * Get the names or indices of the register object for the given platform register
 	 * 
 	 * <p>
 	 * This will check for a label in the host physical space, allowing a mapper to specify an
-	 * alternative register object name. See {@link #addRegisterMapOverride(Register, String)}.
+	 * alternative register object name. See {@link #addRegisterMapOverride(Register, String)}. If
+	 * one exists, then only that name is returned. Otherwise, the given register's names and
+	 * aliases are all returned as defined and in all-upper and all-lower case.
 	 * 
 	 * @param register the platform register
 	 * @return the mapped name
 	 */
-	String getConventionalRegisterObjectName(Register register);
+	Collection<String> getConventionalRegisterObjectNames(Register register);
 
 	/**
 	 * Get the expected path where an object defining the register value would be
 	 * 
 	 * @param schema the schema of the register container
 	 * @param path the path to the register container
-	 * @param name the name of the register on the target
+	 * @param names the possible names of the register on the target
 	 * @return the path matcher, possibly empty
 	 */
 	PathMatcher getConventionalRegisterPath(TargetObjectSchema schema, List<String> path,
-			String name);
+			Collection<String> names);
 
 	/**
 	 * Get the expected path where an object defining the register value would be

Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/database/guest/DBTraceObjectRegisterSupportTest.java

@@ -459,4 +459,23 @@ public class DBTraceObjectRegisterSupportTest extends AbstractGhidraHeadlessInte
 		assertMatches("Targets[0].Threads[0].Registers.User[r0]",
 			b.host.getConventionalRegisterPath(overlay, b.reg("r0")));
 	}
+
+	@Test
+	public void testPlatformGetConventionalRegisterPathAlias() throws Throwable {
+		AddressSpace registers = b.trace.getBaseAddressFactory().getRegisterSpace();
+		AddressSpace overlay;
+		Register r0;
+		try (UndoableTransaction tid = b.startTransaction()) {
+			root = manager.createRootObject(ctx.getSchema(new SchemaName("Session"))).getChild();
+			r0 = b.language.getRegister("r0");
+			overlay = b.trace.getMemoryManager()
+					.createOverlayAddressSpace("Targets[0].Threads[0].Registers", registers);
+		}
+
+		PathMatcher matcher = b.host.getConventionalRegisterPath(overlay, r0);
+		assertMatches("Targets[0].Threads[0].Registers.User[r0]", matcher);
+		assertMatches("Targets[0].Threads[0].Registers.User[a0]", matcher);
+		assertMatches("Targets[0].Threads[0].Registers.User[R0]", matcher);
+		assertMatches("Targets[0].Threads[0].Registers.User[A0]", matcher);
+	}
 }

Ghidra/Processors/Toy/certification.manifest

@@ -12,7 +12,7 @@ data/languages/old/v01stuff/toyInstructions.sinc||GHIDRA||||END|
 data/languages/old/v01stuff/toyPosStack.cspec||GHIDRA||||END|
 data/languages/toy.cspec||GHIDRA||||END|
 data/languages/toy.ldefs||GHIDRA||||END|
-data/languages/toy.pspec||GHIDRA||reviewed||END|
+data/languages/toy.pspec||GHIDRA||||END|
 data/languages/toy.sinc||GHIDRA||||END|
 data/languages/toy64-long8.cspec||GHIDRA||||END|
 data/languages/toy64.cspec||GHIDRA||||END|

Ghidra/Processors/Toy/data/languages/toy.pspec

@@ -2,4 +2,22 @@
 
 <processor_spec>
   <programcounter register="pc"/>
+  <register_data>
+    <register name="r0" alias="a0"/>
+    <register name="r1" alias="a1"/>
+    <register name="r2" alias="a2"/>
+    <register name="r3" alias="a3"/>
+    <register name="r4" alias="a4"/>
+    <register name="r5" alias="a5"/>
+    <register name="r6" alias="a6"/>
+    <register name="r7" alias="a7"/>
+    <register name="r8" alias="a8"/>
+    <register name="r9" alias="a9"/>
+    <register name="r10" alias="a10"/>
+    <register name="r11" alias="a11"/>
+    <register name="r12" alias="a12"/>
+    <register name="sp" alias="a13"/>
+    <register name="lr" alias="a14"/>
+    <register name="pc" alias="a15"/>
+  </register_data>
 </processor_spec>