GT-3348 fixing stale dynamic symbol names

Ghidra/Features/Base/src/test.slow/java/ghidra/program/database/symbol/SymbolManagerTest.java

@@ -22,11 +22,13 @@ import java.util.*;
 import org.junit.*;
 
 import generic.test.TestUtils;
+import ghidra.app.cmd.disassemble.DisassembleCommand;
 import ghidra.app.cmd.function.CreateFunctionCmd;
 import ghidra.program.database.ProgramBuilder;
 import ghidra.program.database.ProgramDB;
 import ghidra.program.model.address.*;
 import ghidra.program.model.data.ByteDataType;
+import ghidra.program.model.data.WordDataType;
 import ghidra.program.model.listing.*;
 import ghidra.program.model.mem.Memory;
 import ghidra.program.model.mem.MemoryBlock;
@@ -216,6 +218,124 @@ public class SymbolManagerTest extends AbstractGhidraHeadedIntegrationTest {
 		assertTrue(s[0].getSource() == SourceType.DEFAULT);
 	}
 
+	@Test
+	public void testDynamicNameChangesWhenDataApplied() throws Exception {
+		refMgr.addMemoryReference(addr(512), addr(256), RefType.FLOW, SourceType.USER_DEFINED, -1);
+		Symbol[] s = st.getSymbols(addr(256));
+		assertEquals(1, s.length);
+		assertEquals("LAB_00000100", s[0].getName());
+		program.getListing().createData(addr(256), new ByteDataType());
+		assertEquals("BYTE_00000100", s[0].getName());
+	}
+
+	@Test
+	public void testDynamicNameChangesWhenDataCleared() throws Exception {
+		refMgr.addMemoryReference(addr(512), addr(256), RefType.FLOW, SourceType.USER_DEFINED, -1);
+		program.getListing().createData(addr(256), new ByteDataType());
+		Symbol[] s = st.getSymbols(addr(256));
+		assertEquals(1, s.length);
+		assertEquals("BYTE_00000100", s[0].getName());
+
+		program.getListing().clearCodeUnits(addr(256), addr(256), false);
+		assertEquals("LAB_00000100", s[0].getName());
+	}
+
+	@Test
+	public void testDynamicOffcutNameChangesWhenSymbolCreated() throws Exception {
+		refMgr.addMemoryReference(addr(512), addr(257), RefType.FLOW, SourceType.USER_DEFINED, -1);
+		program.getListing().createData(addr(256), new WordDataType());
+
+		Symbol[] s = st.getSymbols(addr(257));
+		assertEquals(1, s.length);
+		assertEquals("WORD_00000100+1", s[0].getName());
+		st.createLabel(addr(256), "bob", SourceType.USER_DEFINED);
+		assertEquals("bob+1", s[0].getName());
+	}
+
+	@Test
+	public void testDynamicOffcutNameChangesWhenSymbolRenamed() throws Exception {
+		refMgr.addMemoryReference(addr(512), addr(257), RefType.FLOW, SourceType.USER_DEFINED, -1);
+		program.getListing().createData(addr(256), new WordDataType());
+		Symbol label = st.createLabel(addr(256), "bob", SourceType.USER_DEFINED);
+
+		Symbol[] s = st.getSymbols(addr(257));
+		assertEquals(1, s.length);
+		assertEquals("bob+1", s[0].getName());
+		label.setName("fred", SourceType.USER_DEFINED);
+		assertEquals("fred+1", s[0].getName());
+	}
+
+	@Test
+	public void testDynamicOffcutNameChangesWhenSymbolRemoved() throws Exception {
+		refMgr.addMemoryReference(addr(512), addr(257), RefType.FLOW, SourceType.USER_DEFINED, -1);
+		program.getListing().createData(addr(256), new WordDataType());
+		Symbol label = st.createLabel(addr(256), "bob", SourceType.USER_DEFINED);
+
+		Symbol[] s = st.getSymbols(addr(257));
+		assertEquals(1, s.length);
+		assertEquals("bob+1", s[0].getName());
+		label.delete();
+		assertEquals("WORD_00000100+1", s[0].getName());
+	}
+	@Test
+	public void testDynamicNameChangesWhenOffcutByInstruction() throws Exception {
+		refMgr.addMemoryReference(addr(512), addr(257), RefType.FLOW, SourceType.USER_DEFINED, -1);
+
+		Symbol[] s = st.getSymbols(addr(257));
+		assertEquals(1, s.length);
+		assertEquals("LAB_00000101", s[0].getName());
+		createInstruction(addr(256));
+		CodeUnit codeUnitAt = program.getListing().getCodeUnitAt(addr(256));
+		assertTrue(codeUnitAt instanceof Instruction);
+		assertEquals(2, codeUnitAt.getLength());
+
+		assertEquals("LAB_00000100+1", s[0].getName());
+	}
+
+	@Test
+	public void testDynamicNameChangesWhenCallRefAdded() throws Exception {
+		refMgr.addMemoryReference(addr(512), addr(256), RefType.FLOW, SourceType.USER_DEFINED, -1);
+
+		Symbol[] s = st.getSymbols(addr(256));
+		assertEquals(1, s.length);
+		assertEquals("LAB_00000100", s[0].getName());
+
+		refMgr.addMemoryReference(addr(512), addr(256), RefType.UNCONDITIONAL_CALL,
+			SourceType.USER_DEFINED, -1);
+
+		assertEquals("SUB_00000100", s[0].getName());
+	}
+
+	@Test
+	public void testDynamicNameChangesWhenCallRefRemoved() throws Exception {
+		refMgr.addMemoryReference(addr(512), addr(256), RefType.FLOW, SourceType.USER_DEFINED, -1);
+		Reference ref = refMgr.addMemoryReference(addr(516), addr(256), RefType.UNCONDITIONAL_CALL,
+			SourceType.USER_DEFINED, -1);
+
+		Symbol[] s = st.getSymbols(addr(256));
+		assertEquals(1, s.length);
+		assertEquals("SUB_00000100", s[0].getName());
+
+		refMgr.delete(ref);
+		assertEquals("LAB_00000100", s[0].getName());
+	}
+
+
+	private void createInstruction(Address addr) throws Exception {
+		int tx = program.startTransaction("test");
+		try {
+			Memory memory = program.getMemory();
+			memory.setByte(addr, (byte) 0xd9);
+			memory.setByte(addr, (byte) 0x32);
+			AddressSet set = new AddressSet(addr, addr.add(1));
+			DisassembleCommand cmd = new DisassembleCommand(set, set);
+			cmd.applyTo(program);
+		}
+		finally {
+			program.endTransaction(tx, true);
+		}
+	}
+
 	@Test
 	public void testGetDefaultFunctionSymbolByName() throws Exception {
 		Listing listing = program.getListing();

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/symbol/SymbolDB.java

@@ -45,11 +45,13 @@ public abstract class SymbolDB extends DatabaseObject implements Symbol {
 
 	private Record record;
 	private boolean isDeleting = false;
-	protected String name;
 	protected Address address;
 	protected SymbolManager symbolMgr;
 	protected Lock lock;
 
+	private String cachedName;
+	private long cachedNameModCount;
+
 	/**
 	 * Creates a Symbol that is just a placeholder for use when trying to find symbols by using
 	 * {@link Symbol#getID()}.   This is useful for locating symbols in Java collections when
@@ -83,7 +85,7 @@ public abstract class SymbolDB extends DatabaseObject implements Symbol {
 	@Override
 	public String toString() {
 		// prefer cached name for speed; it may be stale; call getName() for current value
-		String temp = name;
+		String temp = cachedName;
 		if (temp != null) {
 			return temp;
 		}
@@ -97,7 +99,6 @@ public abstract class SymbolDB extends DatabaseObject implements Symbol {
 
 	@Override
 	protected boolean refresh(Record rec) {
-		name = null;
 		if (record != null) {
 			if (rec == null) {
 				rec = symbolMgr.getSymbolRecord(key);
@@ -164,20 +165,31 @@ public abstract class SymbolDB extends DatabaseObject implements Symbol {
 	}
 
 	@Override
-	public String getName() {
+	public final String getName() {
+		String name = cachedName;
+		if (hasValidCachedName(name)) {
+			return name;
+		}
+
 		lock.acquire();
 		try {
 			checkIsValid();
-			if (name == null) {
+			cachedName = doGetName();
-				name = doGetName();
+			cachedNameModCount = symbolMgr.getProgram().getModificationNumber();
-			}
+			return cachedName;
-			return name;
 		}
 		finally {
 			lock.release();
 		}
 	}
 
+	private boolean hasValidCachedName(String name) {
+		if (name == null) {
+			return false;
+		}
+		return symbolMgr.getProgram().getModificationNumber() == cachedNameModCount;
+	}
+
 	/**
 	 * The code for creating the name content for this symbol.  This code will be called 
 	 * with the symbol's lock.
@@ -521,7 +533,6 @@ public abstract class SymbolDB extends DatabaseObject implements Symbol {
 
 		lock.acquire();
 		try {
-			name = null;
 			checkDeleted();
 			checkEditOK();
 
@@ -575,10 +586,10 @@ public abstract class SymbolDB extends DatabaseObject implements Symbol {
 
 				record.setLongValue(SymbolDatabaseAdapter.SYMBOL_PARENT_COL, newNamespace.getID());
 				record.setString(SymbolDatabaseAdapter.SYMBOL_NAME_COL, newName);
-				name = newName;
 				updateSymbolSource(record, source);
 				updateRecord();
-
+				cachedName = null;  // we can't clear it until now, since any call to getName()
+									// will cause the cached name to reset to the old name
 				if (namespaceChange) {
 					symbolMgr.symbolNamespaceChanged(this, oldNamespace);
 				}

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/symbol/SymbolDatabaseAdapterV2.java

@@ -1,6 +1,5 @@
 /* ###
  * IP: GHIDRA
- * REVIEWED: YES
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,6 +15,11 @@
  */
 package ghidra.program.database.symbol;
 
+import java.io.IOException;
+import java.util.HashSet;
+import java.util.Set;
+
+import db.*;
 import ghidra.program.database.map.*;
 import ghidra.program.database.util.DatabaseTableUtils;
 import ghidra.program.database.util.RecordFilter;
@@ -25,12 +29,6 @@ import ghidra.program.model.symbol.*;
 import ghidra.util.exception.*;
 import ghidra.util.task.TaskMonitor;
 
-import java.io.IOException;
-import java.util.HashSet;
-import java.util.Set;
-
-import db.*;
-
 /**
  * SymbolDatabaseAdapter for version 2
  */
@@ -183,13 +181,12 @@ class SymbolDatabaseAdapterV2 extends SymbolDatabaseAdapter {
 		}
 	}
 
-	/* (non-Javadoc)
-	 * @see ghidra.program.database.symbol.SymbolDatabaseAdapter#createSymbol(java.lang.String, ghidra.program.model.address.Address, long, ghidra.program.model.symbol.SymbolType, long, int, int)
-	 */
 	@Override
 	Record createSymbol(String name, Address address, long namespaceID, SymbolType symbolType,
 			long data1, int data2, String data3, SourceType source) throws IOException {
 		long nextID = symbolTable.getKey();
+
+		// avoiding key 0, because we use the negative of the address offset as keys for dynamic symbols
 		if (nextID == 0) {
 			nextID++;
 		}