GT-3260_emteere cache of address set of memory blocks that are marked

execute

Author: emteere

Ghidra/Features/Base/src/test.slow/java/ghidra/program/database/mem/MemoryManagerTest.java

@@ -296,7 +296,6 @@ public void testBlockChanges() throws Exception {
 
 		block2.setSourceName("Test");
 		assertEquals("Test", block2.getSourceName());
-
 	}
 
 	@Test
@@ -398,6 +397,90 @@ public void testGetBlockByName() throws Exception {
 		transactionID = program.startTransaction("Test");
 	}
 
+	@Test
+	public void testMemoryMapExecuteSet() throws Exception {
+		
+		AddressSetView executeSet = mem.getExecuteSet();
+		assertTrue(executeSet.isEmpty());
+		MemoryBlock block1 = createBlock("Test1", addr(100), 100);
+		executeSet = mem.getExecuteSet();
+		assertTrue(executeSet.isEmpty());
+		MemoryBlock block2 = createBlock("Test2", addr(300), 100);
+		executeSet = mem.getExecuteSet();
+		assertTrue(executeSet.isEmpty());
+
+		MemoryBlock block = mem.getBlock("Test1");
+		executeSet = mem.getExecuteSet();
+		assertTrue(executeSet.isEmpty());
+		
+		block.setExecute(false);
+		executeSet = mem.getExecuteSet();
+		assertTrue(executeSet.isEmpty());
+
+		block.setExecute(true);
+		executeSet = mem.getExecuteSet();
+		assertTrue(executeSet.isEmpty() != true);
+		Address start = block.getStart();
+		Address end = block.getEnd();
+		assertTrue(executeSet.contains(start,end));
+
+		// non-existent block
+		block = mem.getBlock("NoExist");
+		assertNull(block);
+		
+		program.endTransaction(transactionID, true);
+		transactionID = program.startTransaction("Test");	
+		
+		// now exists
+		mem.getBlock("Test1").setName("NoExist");
+		// Test1 no longer exists
+		block = mem.getBlock("NoExist");
+		executeSet = mem.getExecuteSet();
+		start = block.getStart();
+		end = block.getEnd();
+		// should be same block
+		assertTrue(executeSet.contains(start,end));
+		block.setExecute(false);
+		executeSet = mem.getExecuteSet();
+		assertTrue(executeSet.contains(start,end) == false);
+		
+		block2.setExecute(true);
+		Address start2 = block2.getStart();
+		Address end2 = block2.getEnd();
+		mem.removeBlock(block2, new TaskMonitorAdapter());
+		
+		program.endTransaction(transactionID, true);
+		
+		program.undo();
+		
+		transactionID = program.startTransaction("Test");
+
+		// should be execute set on block2, deleted, then undone
+		executeSet = mem.getExecuteSet();
+		assertTrue(executeSet.contains(start2,end2) == false);
+	
+		// undid set execute block should now be contained
+		block = mem.getBlock("Test1");
+		start = block.getStart();
+		end = block.getEnd();
+		executeSet = mem.getExecuteSet();
+		assertTrue(executeSet.contains(start,end));
+		
+		mem.split(block, addr(150));
+		block = mem.getBlock("Test1");
+		executeSet = mem.getExecuteSet();
+		assertTrue(executeSet.isEmpty() != true);
+		assertTrue(executeSet.contains(block.getStart(), block.getEnd()));
+		
+		// remove block that was split, should still be executable memory
+		start = block.getStart();
+		end = block.getEnd();
+		mem.removeBlock(block, new TaskMonitorAdapter());
+		executeSet = mem.getExecuteSet();
+		assertTrue(executeSet.isEmpty() != true);
+		assertTrue(executeSet.contains(start, end) == false);
+	}
+	
 	@Test
 	public void testSave() throws Exception {
 		MemoryBlock block1 = createBlock("Test1", addr(0), 100);

Ghidra/Framework/Project/src/main/java/ghidra/util/Lock.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.
@@ -17,27 +16,29 @@
 package ghidra.util;
 
 /**
- * Ghidra synchronization lock. This class allows creation of named locks for 
+ * Ghidra synchronization lock. This class allows creation of named locks for
  * modifying tables in the Ghidra data base. This class also creates an instance
- * of a global lock that must first be obtained when synchronizing using multiple
- * of the named locks.
+ * of a global lock that must first be obtained when synchronizing using
+ * multiple of the named locks.
  */
 public class Lock {
 	private Thread owner;
 	private int cnt = 0;
+	private int waiting = 0;
 	private String name;
 
 	/**
 	 * Creates an instance of a lock for synchronization within Ghidra.
+	 * 
 	 * @param name the name of this lock
 	 */
 	public Lock(String name) {
 		this.name = name;
 	}
 
 	/**
-	 * Acquire this synchronization lock. 
-	 * (i.e. begin synchronizing on this named lock.) 
+	 * Acquire this synchronization lock. (i.e. begin synchronizing on this named
+	 * lock.)
 	 */
 	public synchronized void acquire() {
 		Thread currThread = Thread.currentThread();
@@ -47,15 +48,16 @@ public synchronized void acquire() {
 				cnt = 1;
 				owner = currThread;
 				return;
-			}
-			else if (owner == currThread) {
+			} else if (owner == currThread) {
 				cnt++;
 				return;
 			}
 			try {
+				waiting++;
 				wait();
-			}
-			catch (InterruptedException e) {
+			} catch (InterruptedException e) {
+			} finally {
+				waiting--;
 			}
 		}
 	}
@@ -70,20 +72,24 @@ public synchronized void release() {
 		if (cnt > 0 && (owner == currThread)) {
 			if (--cnt == 0) {
 				owner = null;
-				notify();
+				// This is purely to help sample profiling.  If notify() is called the
+				// sampler can attribute time to the methods calling this erroneously.  For some reason
+				// the visualvm sampler gets a sample more often when notify() is called.
+				if (waiting != 0) {
+					notify();
+				}
 			}
-		}
-		else {
+		} else {
 			throw new IllegalStateException("Attempted to release an unowned lock: " + name);
 		}
 	}
 
 	/**
 	 * Gets the thread that currently owns the lock.
+	 * 
 	 * @return the thread that owns the lock or null.
 	 */
 	public Thread getOwner() {
 		return owner;
 	}
-
 }

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/util/PseudoDisassembler.java

@@ -95,8 +95,6 @@ public class PseudoDisassembler {
 
 	private boolean respectExecuteFlag = false;
 
-	private AddressSetView executeSet;
-
 	/**
 	 * Create a pseudo disassembler for the given program.
 	 */
@@ -111,18 +109,6 @@ public PseudoDisassembler(Program program) {
 
 		this.programContext = program.getProgramContext();
 	}
-	
-	/**
-	 * @return cached addressSet of executable memory blocks
-	 */
-	private AddressSetView getExecuteSet() {
-		if (executeSet != null) {
-			return executeSet;
-		}
-		
-		executeSet = memory.getExecuteSet();
-		return executeSet;
-	}
 
 	/**
 	 * Set the maximum number of instructions to check
@@ -617,6 +603,7 @@ public boolean checkValidSubroutine(Address entryPoint, PseudoDisassemblerContex
 			boolean allowExistingInstructions, boolean mustTerminate) {
 		AddressSet body = new AddressSet();
 		AddressSet instrStarts = new AddressSet();
+		AddressSetView execSet = memory.getExecuteSet();
 
 		if (hasLowBitCodeModeInAddrValues(program)) {
 			entryPoint = setTargeContextForDisassembly(procContext, entryPoint);
@@ -801,7 +788,6 @@ else if (flowType.isComputed()) {
 								}
 							}
 							// if respecting execute flag on memory, test to make sure we did flow into non-execute memory
-							AddressSetView execSet = getExecuteSet();
 							if (respectExecuteFlag && !execSet.isEmpty() && !execSet.contains(flows[j])) {
 								if (!flows[j].isExternalAddress()) {
 									MemoryBlock block = memory.getBlock(flows[j]);
@@ -902,8 +888,8 @@ private boolean checkPseudoBody(Address entry, AddressSet body, AddressSet start
 		}
 
 		// check that body does not wander into non-executable memory
-		AddressSetView execSet = getExecuteSet();
-		if (respectExecuteFlag && !execSet.isEmpty() && !body.subtract(execSet).isEmpty()) {
+		AddressSetView execSet = memory.getExecuteSet();
+		if (respectExecuteFlag && !execSet.isEmpty() && !execSet.contains(body)) {
 			return false;
 		}
 
@@ -914,8 +900,9 @@ private boolean checkPseudoBody(Address entry, AddressSet body, AddressSet start
 			return false;
 		}
 
+		boolean canHaveOffcutEntry = hasLowBitCodeModeInAddrValues(program);
 		AddressSet strictlyBody = body.subtract(starts);
-		if (hasLowBitCodeModeInAddrValues(program)) {
+		if (canHaveOffcutEntry) {
 			strictlyBody.deleteRange(entry, entry.add(1));
 		}
 		AddressIterator addrIter =

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/mem/MemoryMapDB.java

@@ -57,6 +57,8 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 	private AddressSet addrSet = new AddressSet();
 	private AddressSet initializedLoadedAddrSet = new AddressSet();
 	private AddressSet allInitializedAddrSet = new AddressSet();
+	private AddressSetView executeSet = new AddressSet();
+
 	private MemoryBlock lastBlock;// the last accessed block
 	private LiveMemoryHandler liveMemory;
 
@@ -187,6 +189,7 @@ private synchronized void initializeBlocks() {
 		blocks = newBlocks;
 		addrMap.memoryMapChanged(this);
 		nameBlockMap = new HashMap<>();
+		executeSet = null;
 	}
 
 	public void setLanguage(Language newLanguage) {
@@ -396,6 +399,12 @@ void fireBlockChanged(MemoryBlock block) {
 
 		// name could have changed
 		nameBlockMap = new HashMap<>();
+		
+		// execute state could have changed.  check if in set and shouldn't be or vice/versa
+		if (executeSet != null && executeSet.contains(block.getStart(), block.getEnd()) != block.isExecute()) {
+			// don't regenerate now, do lazily later if needed
+			executeSet = null;
+		}
 	}
 
 	void fireBytesChanged(Address addr, int count) {
@@ -1972,13 +1981,17 @@ public int hashCode() {
 	 */
 	@Override
 	public AddressSetView getExecuteSet() {
+		if (executeSet != null) {
+			return executeSet;
+		}
 		AddressSet set = new AddressSet();
 		for (MemoryBlock block : blocks) {
 			if (block.isExecute()) {
 				set.addRange(block.getStart(), block.getEnd());
 			}
 		}
-		return set;
+		executeSet = new AddressSetViewAdapter(set);
+		return executeSet;
 	}
 
 	@Override