GP-4534 Use replacement strategy for MemoryMapDB allAddrsSet update

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

@@ -107,7 +107,7 @@ public class MemoryBlockDB implements MemoryBlock {
 	 * @return collection of blocks which map onto this block or null if none identified
 	 */
 	Collection<MemoryBlockDB> getMappedBlocks() {
-		memMap.buildAddressSets(); // updates mappedBlocks if needed
+		memMap.buildAddressSets(false); // updates mappedBlocks if needed
 		return mappedBlocks;
 	}
 

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

@@ -52,15 +52,15 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 
 	private DataConverter defaultEndian;
 	private List<MemoryBlockDB> blocks;// sorted list of blocks
-	private SynchronizedAddressSet allAddrSet = new SynchronizedAddressSet(); // continuously updated
+	private AddressSetView allAddrSet = new AddressSetViewAdapter(); // replaced on update
 
-	private MemoryAddressSetView addrSetView;
+	private MemoryAddressSetViews addrSetViews;
 
 	/**
 	 * Address set views into program memory which are iterator safe
 	 * for public API methods.
 	 */
-	private class MemoryAddressSetView {
+	private class MemoryAddressSetViews {
 		private AddressSet initializedAndLoaded = new AddressSet();
 		private AddressSet initialized = new AddressSet();
 		private AddressSet externalBlock = new AddressSet();
@@ -103,7 +103,7 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 		adapter = MemoryMapDBAdapter.getAdapter(handle, openMode, this, monitor);
 		fileBytesAdapter = FileBytesAdapter.getAdapter(handle, openMode, monitor);
 		initializeBlocks();
-		buildAddressSets();
+		buildAddressSets(true);
 	}
 
 	// for testing
@@ -135,45 +135,62 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 		}
 	}
 
-	MemoryAddressSetView buildAddressSets() {
+	/**
-		if (addrSetView != null) {
+	 * Get the address set views {@code addrSetView} and rebuild if needed and optionally
-			return addrSetView;
+	 * rebuild {@code allAddrSet}.  This method also updates mapped-block details when
+	 * rebuilding the views.
+	 * <br>
+	 * NOTE: The {@link #initializeBlocks()} method is used to invalidate the {code addrSetViews}
+	 * without affecting {@code allAddrSet}, while {@link #reloadAll()} will force a complete
+	 * rebuild of all addresss sets.
+	 * 
+	 * @param rebuildAllAddrSets if true all address sets will be rebuilt before returning the
+	 * address set view object.
+	 * @return the address set view object
+	 */
+	MemoryAddressSetViews buildAddressSets(boolean rebuildAllAddrSets) {
+		MemoryAddressSetViews localAddrSetViews = addrSetViews;
+		if (!rebuildAllAddrSets && localAddrSetViews != null) {
+			return localAddrSetViews;
 		}
 		lock.acquire();
 		try {
 			// have to try and get it again, another thread may have already filled it out
-			if (addrSetView != null) {
+			if (!rebuildAllAddrSets && addrSetViews != null) {
-				return addrSetView;
+				return addrSetViews;
 			}
-			
-			// set cached addressSet view to null, so other threads will end up here waiting
-			// on the above lock if they try to access the cached address sets
-			addrSetView = null;
-			MemoryAddressSetView newAddrSetView = new MemoryAddressSetView();
 
-			// The allAddrSet instance is generally maintained with all memory
+			// Begin rebuilding a complete set of address set views
-			// block addresses and need only be updated if currently empty.
+			MemoryAddressSetViews newAddrSetViews = new MemoryAddressSetViews();
-			// Any time allAddrSet is modified addrSetView is set to null to 
+
-			// signal it becoming stale.
+			// The allAddrSet instance is generally kept up-to-date with all memory
-			boolean addToAll = allAddrSet.isEmpty();
+			// block addresses and need only be rebuilt under certain conditions as
+			// signaled by the rebuildAllAddrs parameter.
+			AddressSet newAllAddrs = null;
+			if (rebuildAllAddrSets) {
+				newAllAddrs = new AddressSet();
+			}
 
 			// we have to process the non-mapped blocks first because to process the mapped
 			// blocks we need the address sets for the non-mapped blocks to be complete
 			for (MemoryBlockDB block : blocks) {
 				block.clearMappedBlockList();
 				if (!block.isMapped()) {
-					addBlockAddresses(newAddrSetView, block, addToAll);
+					addBlockAddresses(block, newAddrSetViews, newAllAddrs);
 				}
 			}
 			// process all mapped blocks after non-mapped-blocks above
 			for (MemoryBlockDB block : blocks) {
 				if (block.isMapped()) {
-					addBlockAddresses(newAddrSetView, block, addToAll);
+					addBlockAddresses(block, newAddrSetViews, newAllAddrs);
 				}
 			}
-			addrSetView = newAddrSetView;
+			if (newAllAddrs != null) {
-			
+				// replace allAddrSet with fully updated address set
-			return addrSetView;
+				allAddrSet = new AddressSetViewAdapter(newAllAddrs);
+			}
+			addrSetViews = newAddrSetViews;
+			return addrSetViews;
 		}
 		finally {
 			lock.release();
@@ -181,23 +198,26 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 	}
 
 	/**
-	 * Update the <code>addrSetView</code> address sets with relevant addresses from the 
+	 * Update address sets with relevant addresses from the specified memory block. 
-	 * specified memory block. In addition, allAddrSet will be updated if addToAll parameter is true.
+	 * In addition, allAddrSet will be updated if addToAll parameter is true.
 	 * 
-	 * @param block memory block
+	 * @param block memory block to be added
-	 * @param addToAll if true the allAddrSet should be updated with the specified block's address range
+	 * @param newAddrSetViews address set views which should be built-up
+	 * @param newAllAddrs if not null this set will be updated with the specified block's address range,
+	 * otherwise only the {@code addrSetView} sets will be updated.
 	 */
-	private void addBlockAddresses(MemoryAddressSetView newSet, MemoryBlockDB block, boolean addToAll) {
+	private void addBlockAddresses(MemoryBlockDB block, MemoryAddressSetViews newAddrSetViews,
+			AddressSet newAllAddrs) {
 		Address start = block.getStart();
 		Address end = block.getEnd();
-		if (addToAll) {
+		if (newAllAddrs != null) {
-			allAddrSet.add(start, end);
+			newAllAddrs.add(start, end);
 		}
 		if (block.isExternalBlock()) {
-			newSet.externalBlock.add(start, end);
+			newAddrSetViews.externalBlock.add(start, end);
 		}
 		else if (block.isExecute()) {
-			newSet.execute.add(start, end);
+			newAddrSetViews.execute.add(start, end);
 		}
 		if (block.isMapped()) {
 			// Identify source-blocks which block maps onto and add as a mapped-block to each of these
@@ -208,26 +228,37 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 					b.addMappedBlock(block);
 				}
 			}
-			AddressSet mappedSet = getMappedIntersection(block, newSet.initialized);
+			AddressSet mappedSet = getMappedIntersection(block, newAddrSetViews.initialized);
-			newSet.initialized.add(mappedSet);
+			newAddrSetViews.initialized.add(mappedSet);
-			newSet.initializedAndLoaded
+			newAddrSetViews.initializedAndLoaded
-					.add(getMappedIntersection(block, newSet.initializedAndLoaded));
+					.add(getMappedIntersection(block, newAddrSetViews.initializedAndLoaded));
 		}
 		else if (block.isInitialized()) {
-			newSet.initialized.add(block.getStart(), block.getEnd());
+			newAddrSetViews.initialized.add(block.getStart(), block.getEnd());
 			if (block.isLoaded()) {
-				newSet.initializedAndLoaded.add(block.getStart(), block.getEnd());
+				newAddrSetViews.initializedAndLoaded.add(block.getStart(), block.getEnd());
 			}
 		}
 	}
 
+	private void addToAllAddressSet(Address minAddr, Address maxAddr) {
+		AddressSet updatedAllAddrSet = new AddressSet(allAddrSet);
+		updatedAllAddrSet.add(minAddr, maxAddr);
+		allAddrSet = new AddressSetViewAdapter(updatedAllAddrSet);
+	}
+
+	private void removeFromAllAddressSet(Address minAddr, Address maxAddr) {
+		AddressSet updatedAllAddrSet = new AddressSet(allAddrSet);
+		updatedAllAddrSet.delete(minAddr, maxAddr);
+		allAddrSet = new AddressSetViewAdapter(updatedAllAddrSet);
+	}
+
 	private void reloadAll() throws IOException {
 		synchronized (this) {
 			fileBytesAdapter.refresh();
 			adapter.refreshMemory();
-			allAddrSet = new SynchronizedAddressSet(); // buildAddressSets() will populate
 			initializeBlocks();
-			buildAddressSets();
+			buildAddressSets(true);
 		}
 		if (liveMemory != null) {
 			liveMemory.clearCache();
@@ -239,7 +270,7 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 		blocks = adapter.getMemoryBlocks();
 		lastBlock = null;
 		nameBlockMap = new HashMap<>();
-		addrSetView = null; // signal stale view
+		addrSetViews = null; // signal stale views
 		addrMap.memoryMapChanged(this);
 		if (program != null) {
 			program.getAddressFactory().invalidateOverlayCache();
@@ -248,18 +279,18 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 
 	void blockExecuteChanged(MemoryBlockDB block) {
 		// lock must be active
-		if (addrSetView == null) {
+		if (addrSetViews == null) {
 			return;
 		}
 		// copy must be made to remain iterator safe
-		AddressSet set = new AddressSet(addrSetView.execute);
+		AddressSet set = new AddressSet(addrSetViews.execute);
 		if (block.isExecute()) {
 			set.addRange(block.getStart(), block.getEnd());
 		}
 		else {
 			set.deleteRange(block.getStart(), block.getEnd());
 		}
-		addrSetView.execute = set;
+		addrSetViews.execute = set;
 	}
 
 	public void setLanguage(Language newLanguage) {
@@ -322,8 +353,8 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 
 	@Override
 	public AddressSetView getAllInitializedAddressSet() {
-		MemoryAddressSetView localAddrSetView = buildAddressSets();
+		MemoryAddressSetViews localAddrSetViews = buildAddressSets(false);
-		return new AddressSetViewAdapter(localAddrSetView.initialized);
+		return new AddressSetViewAdapter(localAddrSetViews.initialized);
 	}
 
 	@Override
@@ -331,21 +362,21 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 		if (liveMemory != null) {
 			return this; // all memory is initialized!
 		}
-		
+
-		MemoryAddressSetView localAddrSetView = buildAddressSets();
+		MemoryAddressSetViews localAddrSetViews = buildAddressSets(false);
-		return new AddressSetViewAdapter(localAddrSetView.initializedAndLoaded);
+		return new AddressSetViewAdapter(localAddrSetViews.initializedAndLoaded);
 	}
-	
+
 	@Override
 	public boolean isExternalBlockAddress(Address addr) {
-		MemoryAddressSetView localAddrSetView = buildAddressSets();
+		MemoryAddressSetViews localAddrSetViews = buildAddressSets(false);
-		return localAddrSetView.externalBlock.contains(addr);
+		return localAddrSetViews.externalBlock.contains(addr);
 	}
-	
+
 	@Override
 	public AddressSetView getExecuteSet() {
-		MemoryAddressSetView localAddrSetView = buildAddressSets();
+		MemoryAddressSetViews localAddrSetViews = buildAddressSets(false);
-		return new AddressSetViewAdapter(localAddrSetView.execute);
+		return new AddressSetViewAdapter(localAddrSetViews.execute);
 	}
 
 	void checkMemoryWrite(MemoryBlockDB block, Address start, long length)
@@ -489,7 +520,7 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 		return null;
 	}
 
-	void fireBlockAdded(MemoryBlock newBlock) {
+	private void fireBlockAdded(MemoryBlock newBlock) {
 		AddressRange range = new AddressRangeImpl(newBlock.getStart(), newBlock.getEnd());
 		program.getTreeManager().addMemoryBlock(newBlock.getName(), range);
 		program.setChanged(ProgramEvent.MEMORY_BLOCK_ADDED, newBlock.getStart(), newBlock.getEnd(),
@@ -497,16 +528,17 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 		program.fireEvent(new DomainObjectChangeRecord(DomainObjectEvent.RESTORED));
 	}
 
-	void fireBlockSplit() {
+	private void fireBlockSplit(MemoryBlockDB originalBlock, MemoryBlockDB newBlock) {
+		program.setChanged(ProgramEvent.MEMORY_BLOCK_SPLIT, null, null, originalBlock, newBlock);
 		program.fireEvent(new DomainObjectChangeRecord(DomainObjectEvent.RESTORED));
 	}
 
-	void fireBlockRemoved(Address blockStartAddr) {
+	private void fireBlockRemoved(Address blockStartAddr) {
 		program.setChanged(ProgramEvent.MEMORY_BLOCK_REMOVED, blockStartAddr, null);
 		program.fireEvent(new DomainObjectChangeRecord(DomainObjectEvent.RESTORED));
 	}
 
-	void fireBlockMoved(MemoryBlockDB block, Address oldStartAddr) {
+	private void fireBlockMoved(MemoryBlockDB block, Address oldStartAddr) {
 		program.setChanged(ProgramEvent.MEMORY_BLOCKS_JOINED, oldStartAddr, block);
 		program.fireEvent(new DomainObjectChangeRecord(DomainObjectEvent.RESTORED));
 	}
@@ -518,14 +550,10 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 	 * @param newBlock new joined memory block
 	 * @param oldBlockStartAddr original start address of affected block
 	 */
-	void fireBlocksJoined(MemoryBlock newBlock, Address oldBlockStartAddr) {
+	private void fireBlocksJoined(MemoryBlock newBlock, Address oldBlockStartAddr) {
 		program.setChanged(ProgramEvent.MEMORY_BLOCKS_JOINED, oldBlockStartAddr, newBlock);
 	}
 
-	void fireBlockSplit(MemoryBlockDB originalBlock, MemoryBlockDB newBlock) {
-		program.setChanged(ProgramEvent.MEMORY_BLOCK_SPLIT, null, null, originalBlock, newBlock);
-	}
-
 	void fireBlockChanged(MemoryBlock block) {
 		if (program != null) {
 			program.setChanged(ProgramEvent.MEMORY_BLOCK_CHANGED, block, null);
@@ -685,7 +713,7 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 			try {
 				MemoryBlockDB newBlock =
 					adapter.createInitializedBlock(name, start, is, length, MemoryBlock.READ);
-				allAddrSet.add(newBlock.getStart(), newBlock.getEnd());
+				addToAllAddressSet(newBlock.getStart(), newBlock.getEnd());
 				initializeBlocks();
 				fireBlockAdded(newBlock);
 				return newBlock;
@@ -729,7 +757,7 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 			try {
 				MemoryBlockDB newBlock = adapter.createFileBytesBlock(name, start, length,
 					fileBytes, offset, MemoryBlock.READ);
-				allAddrSet.add(newBlock.getStart(), newBlock.getEnd());
+				addToAllAddressSet(newBlock.getStart(), newBlock.getEnd());
 				initializeBlocks();
 				fireBlockAdded(newBlock);
 				return newBlock;
@@ -782,7 +810,7 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 			try {
 				MemoryBlockDB newBlock = adapter.createBlock(MemoryBlockType.DEFAULT, name, start,
 					size, null, false, MemoryBlock.READ, 0);
-				allAddrSet.add(newBlock.getStart(), newBlock.getEnd());
+				addToAllAddressSet(newBlock.getStart(), newBlock.getEnd());
 				initializeBlocks();
 				fireBlockAdded(newBlock);
 				return newBlock;
@@ -817,7 +845,7 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 			try {
 				MemoryBlockDB newBlock = adapter.createBlock(MemoryBlockType.BIT_MAPPED, name,
 					start, length, mappedAddress, false, MemoryBlock.READ, 0);
-				allAddrSet.add(newBlock.getStart(), newBlock.getEnd());
+				addToAllAddressSet(newBlock.getStart(), newBlock.getEnd());
 				initializeBlocks();
 				fireBlockAdded(newBlock);
 				return newBlock;
@@ -861,7 +889,7 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 			try {
 				MemoryBlockDB newBlock = adapter.createBlock(MemoryBlockType.BYTE_MAPPED, name,
 					start, length, mappedAddress, false, MemoryBlock.READ, mappingScheme);
-				allAddrSet.add(newBlock.getStart(), newBlock.getEnd());
+				addToAllAddressSet(newBlock.getStart(), newBlock.getEnd());
 				initializeBlocks();
 				fireBlockAdded(newBlock);
 				return newBlock;
@@ -909,7 +937,7 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 				}
 				MemoryBlockDB newBlock = adapter.createBlock(block.getType(), name, start, length,
 					mappedAddr, block.isInitialized(), block.getFlags(), mappingScheme);
-				allAddrSet.add(newBlock.getStart(), newBlock.getEnd());
+				addToAllAddressSet(newBlock.getStart(), newBlock.getEnd());
 				initializeBlocks();
 				fireBlockAdded(newBlock);
 				return newBlock;
@@ -1031,9 +1059,9 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 				throw new IllegalArgumentException("Split cannot be done on a bit mapped block");
 			}
 			try {
-				memBlock.split(addr);
+				MemoryBlockDB newBlock = memBlock.split(addr);
 				initializeBlocks();
-				fireBlockSplit();
+				fireBlockSplit(memBlock, newBlock);
 			}
 			catch (IOException e) {
 				program.dbError(e);
@@ -1919,7 +1947,7 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 			try {
 				program.deleteAddressRange(startAddress, endAddress, monitor);
 				memBlock.delete();
-				allAddrSet.delete(startAddress, endAddress);
+				removeFromAllAddressSet(startAddress, endAddress);
 				initializeBlocks();
 			}
 			catch (IOException e) {

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

@@ -1,122 +0,0 @@
-/* ###
- * 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.program.database.mem;
-
-import java.util.ConcurrentModificationException;
-import java.util.Iterator;
-
-import ghidra.program.model.address.*;
-
-/**
- * <code>RecoverableAddressIterator</code> provides the ability to iterator over an {@link AddressSet}
- * which is getting modified concurrent with the iteration of Addresses contained within it.  Do to 
- * multiple levels of prefetch caching, the results returned may be stale relative to the actual
- * {@link AddressSet} at any point in time.  The primary intent is to return addresses in proper order
- * and avoid throwing a {@link ConcurrentModificationException} which the standard iterators are
- * subject to.
- * <p>
- * NOTES:
- * <ol>
- * <li>The iterator methods are not symchronized but could be made so if restricted to 
- * use in conjunction with the {@link SynchronizedAddressSet} where it would synchronize on 
- * the set itself.</li>
- * <li>This class and {@link SynchronizedAddressSet} could be made public alongside {@link AddressSet}
- * if so desired in the future.  Its current use has been limited until proven to be thread-safe
- * and useful.</li>
- * </ol>
- */
-class RecoverableAddressIterator implements AddressIterator {
-
-	private SynchronizedAddressSet synchSet;
-	private AddressSet internalSet;
-	private boolean forward;
-	private AddressIterator iterator;
-	private Address next;
-	private int changeID;
-
-	/**
-	 * Construct iterator
-	 * @param set address set
-	 * @param start address to start iterating at in the address set or null for all addresses
-	 * @param forward if true address are return from lowest to highest, else from highest to lowest
-	 */
-	RecoverableAddressIterator(SynchronizedAddressSet set, Address start, boolean forward) {
-		this.synchSet = set;
-		this.internalSet = set.getInternalSet();
-		changeID = set.getModificationID();
-		this.forward = forward;
-		initIterator(start);
-		this.next = iterator.next();
-	}
-
-	private void initIterator(Address start) {
-		if (start == null) {
-			iterator = internalSet.getAddresses(forward);
-		}
-		else {
-			iterator = internalSet.getAddresses(start, forward);
-		}
-	}
-
-	@Override
-	public Iterator<Address> iterator() {
-		return this;
-	}
-
-	@Override
-	public Address next() {
-		Address addr = next;
-		if (addr != null) {
-			try {
-				if (synchSet.hasChanged(changeID)) {
-					next = recoverNext(addr);
-				} else {
-					next = iterator.next();
-				}
-			}
-			catch (ConcurrentModificationException e) {
-				// will never happen, set in hand is never changed
-				next = recoverNext(addr);
-			}
-		}
-		return addr;
-	}
-
-	private Address recoverNext(Address lastAddr) {
-		changeID = synchSet.getModificationID();
-		internalSet = synchSet.getInternalSet();
-		while (true) {
-			try {
-				initIterator(lastAddr);
-				Address a = iterator.next();
-				if (a != null && a.equals(lastAddr)) {
-					a = iterator.next();
-				}
-				return a;
-			}
-			catch (ConcurrentModificationException e) {
-				// will never happen, set in hand is never changed
-				// set must have changed - try re-initializing again
-			}
-		}
-	}
-
-	@Override
-	public boolean hasNext() {
-		return next != null;
-	}
-
-}

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

@@ -1,142 +0,0 @@
-/* ###
- * 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.program.database.mem;
-
-import java.util.*;
-
-import ghidra.program.model.address.*;
-
-/**
- * <code>RecoverableAddressRangeIterator</code> provides the ability to iterator over an {@link AddressSet}
- * which is getting modified concurrent with the iteration of {@link AddressRange}es contained within it.  Do to 
- * multiple levels of prefetch caching, the results returned may be stale relative to the actual
- * {@link AddressSet} at any point in time.  The primary intent is to return address ranges in proper order
- * and avoid throwing a {@link ConcurrentModificationException} which the standard iterators are
- * subject to.
- * <p>
- * NOTES:
- * <ol>
- * <li>The iterator methods are not symchronized but could be made so if restricted to 
- * use in conjunction with the {@link SynchronizedAddressSet} where it would synchronize on 
- * the set itself.</li>
- * <li>This class and {@link SynchronizedAddressSet} could be made public alongside {@link AddressSet}
- * if so desired in the future.  Its current use has been limited until proven to be thread-safe
- * and useful.</li>
- * </ol>
- */
-class RecoverableAddressRangeIterator implements AddressRangeIterator {
-
-	private SynchronizedAddressSet synchSet;
-	private AddressSet internalSet;
-	private boolean forward;
-	private AddressRangeIterator iterator;
-	private AddressRange next;
-	private int changeID;
-
-	/**
-	 * Construct iterator
-	 * @param set address set
-	 * @param start the address the the first range should contain.
-	 * @param forward true iterators forward, false backwards
-	 */
-	RecoverableAddressRangeIterator(SynchronizedAddressSet set, Address start, boolean forward) {
-		this.synchSet = set;
-		this.internalSet = set.getInternalSet();
-		changeID = set.getModificationID();
-		this.forward = forward;
-		initIterator(start);
-		try {
-			this.next = iterator.next();
-		}
-		catch (NoSuchElementException e) {
-			this.next = null;
-		}
-	}
-
-	private void initIterator(Address start) {
-		if (start == null) {
-			iterator = internalSet.getAddressRanges(forward);
-		}
-		else {
-			iterator = internalSet.getAddressRanges(start, forward);
-		}
-	}
-
-	@Override
-	public Iterator<AddressRange> iterator() {
-		return this;
-	}
-
-	@Override
-	public AddressRange next() throws NoSuchElementException {
-		AddressRange range = next;
-		if (range == null) {
-			throw new NoSuchElementException();
-		}
-		try {
-			if (synchSet.hasChanged(changeID)) {
-				next = recoverNext(range);
-			} else {
-				next = iterator.next();
-			}
-		}
-		catch (ConcurrentModificationException e) {
-			// will never happen, set in hand is never changed
-			next = recoverNext(range);
-		}
-		catch (NoSuchElementException e) {
-			next = null;
-		}
-		return range;
-	}
-
-	private AddressRange recoverNext(AddressRange lastRange) {
-		changeID = synchSet.getModificationID();
-		internalSet = synchSet.getInternalSet();
-		while (true) {
-			try {
-				Address lastAddr = forward ? lastRange.getMaxAddress() : lastRange.getMinAddress();
-				initIterator(lastAddr);
-				AddressRange r = iterator.next();
-				if (!r.intersects(lastRange)) {
-					return r;
-				}
-				if (forward) {
-					if (r.getMaxAddress().compareTo(lastAddr) > 0) {
-						return new AddressRangeImpl(lastAddr.next(), r.getMaxAddress());
-					}
-				}
-				else if (r.getMinAddress().compareTo(lastAddr) < 0) { // reverse
-					return new AddressRangeImpl(r.getMinAddress(), lastAddr.previous());
-				}
-				return iterator.next(); // skip range and return next
-			}
-			catch (ConcurrentModificationException e) {
-				// will never happen, set in hand is never changed
-				// set must have changed - try re-initializing again
-			}
-			catch (NoSuchElementException e) {
-				return null;
-			}
-		}
-	}
-
-	@Override
-	public boolean hasNext() {
-		return next != null;
-	}
-
-}

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

@@ -1,253 +0,0 @@
-/* ###
- * 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.program.database.mem;
-
-import java.util.Iterator;
-
-import ghidra.program.model.address.*;
-
-/**
- * <code>SynchronizedAddressSet</code> provides a synchronized address set which
- * implements the {@link AddressSetView} interface.  Iterators returned by this
- * implementation will recover from concurrent modification of this address set.
- * See {@link RecoverableAddressRangeIterator} and {@link RecoverableAddressIterator}.
- */
-class SynchronizedAddressSet implements AddressSetView {
-
-	private AddressSet set;
-	private int modificationID = 1; // updated if set above ever replaced
-
-	SynchronizedAddressSet() {
-		set = new AddressSet();
-	}
-	
-	/**
-	 * get the modification number of the internal address set
-	 * If the underlying set if ever replaced, modification id is changed
-	 * @return current modification id
-	 */
-	int getModificationID() {
-		return modificationID;
-	}
-
-	/**
-	 * Add all addresses of the given AddressSet to this set.
-	 * @param addrSet set of addresses to add.
-	 * @see AddressSet#add(AddressSetView)
-	 */
-	synchronized void add(AddressSet addrSet) {
-		AddressSet newSet = new AddressSet(set);
-		newSet.add(addrSet);
-		set = newSet;
-		modificationID++;
-	}
-
-	/**
-	 * Adds the range to this set
-	 * @param start the start address of the range to add
-	 * @param end the end address of the range to add
-	 * @see AddressSet#add(Address, Address)
-	 */
-	synchronized void add(Address start, Address end) {
-		AddressSet newSet = new AddressSet(set);
-		newSet.add(start, end);
-		set = newSet;
-		modificationID++;
-	}
-
-	/**
-	 * Deletes a range of addresses from this set
-	 * @param start the starting address of the range to be removed
-	 * @param end the ending address of the range to be removed (inclusive)
-	 * @see AddressSet#delete(Address, Address)
-	 */
-	synchronized void delete(Address start, Address end) {
-		AddressSet newSet = new AddressSet(set);
-		newSet.delete(start, end);
-		set = newSet;
-		modificationID++;
-	}
-	
-	synchronized AddressSet getInternalSet() {
-		return set;
-	}
-	
-	/**
-	 * Check if the internal set has been modified
-	 * If the mod id is different, then the set has changed.
-	 * 
-	 * @param modID modification id to check
-	 * @return true if internal mod id is different
-	 */
-	public boolean hasChanged(int modID) {
-		return modID != modificationID;
-	}
-
-	@Override
-	public boolean contains(Address addr) {
-		return set.contains(addr);
-	}
-
-	@Override
-	public boolean contains(Address start, Address end) {
-		return set.contains(start, end);
-	}
-
-	@Override
-	public boolean contains(AddressSetView addrSet) {
-		return set.contains(addrSet);
-	}
-
-	@Override
-	public boolean isEmpty() {
-		return set.isEmpty();
-	}
-
-	@Override
-	public Address getMinAddress() {
-		return set.getMinAddress();
-	}
-
-	@Override
-	public Address getMaxAddress() {
-		return set.getMaxAddress();
-	}
-
-	@Override
-	public int getNumAddressRanges() {
-		return set.getNumAddressRanges();
-	}
-
-	@Override
-	public AddressRangeIterator getAddressRanges() {
-		return set.getAddressRanges();
-	}
-
-	@Override
-	public AddressRangeIterator getAddressRanges(boolean forward) {
-		return set.getAddressRanges(forward);
-	}
-
-	@Override
-	public synchronized AddressRangeIterator getAddressRanges(Address start, boolean forward) {
-		return new RecoverableAddressRangeIterator(this, start, forward);
-	}
-
-	@Override
-	public synchronized Iterator<AddressRange> iterator() {
-		return set.getAddressRanges();
-	}
-
-	@Override
-	public Iterator<AddressRange> iterator(boolean forward) {
-		return set.getAddressRanges(forward);
-	}
-
-	@Override
-	public Iterator<AddressRange> iterator(Address start, boolean forward) {
-		return set.getAddressRanges(start, forward);
-	}
-
-	@Override
-	public long getNumAddresses() {
-		return set.getNumAddresses();
-	}
-
-	@Override
-	public synchronized AddressIterator getAddresses(boolean forward) {
-		return new RecoverableAddressIterator(this, null, forward);
-	}
-
-	@Override
-	public synchronized AddressIterator getAddresses(Address start, boolean forward) {
-		return new RecoverableAddressIterator(this, start, forward);
-	}
-
-	@Override
-	public boolean intersects(AddressSetView addrSet) {
-		return set.intersects(addrSet);
-	}
-
-	@Override
-	public boolean intersects(Address start, Address end) {
-		return set.intersects(start, end);
-	}
-
-	@Override
-	public AddressSet intersect(AddressSetView addrSet) {
-		return set.intersect(addrSet);
-	}
-
-	@Override
-	public AddressSet intersectRange(Address start, Address end) {
-		return set.intersectRange(start, end);
-	}
-
-	@Override
-	public AddressSet union(AddressSetView addrSet) {
-		return set.union(addrSet);
-	}
-
-	@Override
-	public AddressSet subtract(AddressSetView addrSet) {
-		return set.subtract(addrSet);
-	}
-
-	@Override
-	public AddressSet xor(AddressSetView addrSet) {
-		return set.xor(addrSet);
-	}
-
-	@Override
-	public boolean hasSameAddresses(AddressSetView addrSet) {
-		return set.hasSameAddresses(addrSet);
-	}
-
-	@Override
-	public AddressRange getFirstRange() {
-		return set.getFirstRange();
-	}
-
-	@Override
-	public AddressRange getLastRange() {
-		return set.getLastRange();
-	}
-
-	@Override
-	public AddressRange getRangeContaining(Address address) {
-		return set.getRangeContaining(address);
-	}
-
-	@Override
-	public Address findFirstAddressInCommon(AddressSetView addrSet) {
-		return set.findFirstAddressInCommon(addrSet);
-	}
-
-	@Override
-	public int hashCode() {
-		return set.hashCode();
-	}
-
-	@Override
-	public boolean equals(Object obj) {
-		return set.equals(obj);
-	}
-
-	@Override
-	public String toString() {
-		return set.toString();
-	}
-}

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/address/AddressSetViewAdapter.java

@@ -33,6 +33,13 @@ public class AddressSetViewAdapter implements AddressSetView {
 		this.set = set;
 	}
 
+	/**
+	 * Construct an empty AddressSetViewAdapter.
+	 */
+	public AddressSetViewAdapter() {
+		this.set = new AddressSet();
+	}
+
 	@Override
 	public boolean contains(Address addr) {
 		return set.contains(addr);

Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/database/mem/RecoverableAddressIteratorTest.java

@@ -1,174 +0,0 @@
-/* ###
- * 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.program.database.mem;
-
-import static org.junit.Assert.*;
-
-import org.junit.Before;
-import org.junit.Test;
-
-import generic.test.AbstractGenericTest;
-import ghidra.program.model.address.*;
-
-public class RecoverableAddressIteratorTest extends AbstractGenericTest {
-
-	private AddressSpace space;
-	private AddressSet set;
-
-	@Before
-	public void setUp() throws Exception {
-
-		space = new GenericAddressSpace("xx", 32, AddressSpace.TYPE_RAM, 0);
-
-		set = new AddressSet();
-		set.add(range(0x100, 0x200));
-		set.add(range(0x250, 0x250));
-		set.add(range(0x300, 0x400));
-	}
-
-	private Address addr(long offset) {
-		return space.getAddress(offset);
-	}
-
-	private AddressRange range(long start, long end) {
-		return new AddressRangeImpl(addr(start), addr(end));
-	}
-
-	private void assertHasAddressRange(AddressRange range, boolean forward, AddressIterator it) {
-		Address nextAddr, endAddr;
-		if (forward) {
-			nextAddr = range.getMinAddress();
-			endAddr = range.getMaxAddress();
-		}
-		else {
-			nextAddr = range.getMaxAddress();
-			endAddr = range.getMinAddress();
-		}
-		while (true) {
-			assertEquals(nextAddr, it.next());
-			if (nextAddr.equals(endAddr)) {
-				break;
-			}
-			nextAddr = forward ? nextAddr.next() : nextAddr.previous();
-		}
-	}
-
-	@Test
-	public void test1Forward() {
-
-		AddressIterator it = new RecoverableAddressIterator(set, addr(0x150), true);
-
-		assertTrue(it.hasNext());
-
-		set.add(range(0x350, 0x500));
-
-		assertHasAddressRange(range(0x150, 0x200), true, it);
-
-		set.add(addr(0x220)); // will get skipped due to underlying iterator prefetch
-
-		assertHasAddressRange(range(0x250, 0x250), true, it);
-
-		assertHasAddressRange(range(0x300, 0x400), true, it);
-
-		assertHasAddressRange(range(0x401, 0x500), true, it);
-
-		assertFalse(it.hasNext());
-		assertNull(it.next());
-	}
-
-	@Test
-	public void test2Forward() {
-
-		AddressIterator it = new RecoverableAddressIterator(set, addr(0x150), true);
-
-		assertTrue(it.hasNext());
-
-		set.add(range(0x210, 0x215));
-
-		assertHasAddressRange(range(0x150, 0x200), true, it);
-
-		set.add(range(0x220, 0x500));
-
-		assertHasAddressRange(range(0x210, 0x215), true, it);
-
-		assertHasAddressRange(range(0x220, 0x500), true, it);
-
-		assertFalse(it.hasNext());
-	}
-
-	@Test
-	public void test3Forward() {
-
-		AddressIterator it = new RecoverableAddressIterator(set, addr(0x150), true);
-
-		assertTrue(it.hasNext());
-
-		set.add(range(0x210, 0x215));
-
-		assertHasAddressRange(range(0x150, 0x200), true, it);
-
-		set.delete(range(0x220, 0x500));
-
-		assertHasAddressRange(range(0x210, 0x215), true, it);
-
-		assertFalse(it.hasNext());
-	}
-
-	@Test
-	public void test1Reverse() {
-
-		AddressIterator it = new RecoverableAddressIterator(set, addr(0x350), false);
-
-		assertTrue(it.hasNext());
-
-		set.add(addr(0x240));
-
-		assertHasAddressRange(range(0x300, 0x350), false, it);
-
-		set.add(range(0x50, 0x150));
-
-		assertHasAddressRange(range(0x250, 0x250), false, it);
-
-		set.add(range(0x50, 0x150));
-
-		assertHasAddressRange(range(0x240, 0x240), false, it);
-
-		assertHasAddressRange(range(0x50, 0x200), false, it);
-
-		assertFalse(it.hasNext());
-	}
-
-	@Test
-	public void test2Reverse() {
-
-		AddressIterator it = new RecoverableAddressIterator(set, addr(0x350), false);
-
-		assertTrue(it.hasNext());
-
-		set.add(addr(0x240));
-
-		assertHasAddressRange(range(0x300, 0x350), false, it);
-
-		set.delete(range(0x100, 0x200));
-
-		assertHasAddressRange(range(0x250, 0x250), false, it);
-
-		assertHasAddressRange(range(0x240, 0x240), false, it);
-
-		assertFalse(it.hasNext());
-	}
-
-}

Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/database/mem/RecoverableAddressRangeIteratorTest.java

@@ -1,164 +0,0 @@
-/* ###
- * 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.program.database.mem;
-
-import static org.junit.Assert.*;
-
-import java.util.NoSuchElementException;
-
-import org.junit.Before;
-import org.junit.Test;
-
-import generic.test.AbstractGenericTest;
-import ghidra.program.model.address.*;
-
-public class RecoverableAddressRangeIteratorTest extends AbstractGenericTest {
-
-	private AddressSpace space;
-	private AddressSet set;
-
-	@Before
-	public void setUp() throws Exception {
-
-		space = new GenericAddressSpace("xx", 32, AddressSpace.TYPE_RAM, 0);
-
-		set = new AddressSet();
-		set.add(range(0x100, 0x200));
-		set.add(range(0x250, 0x250));
-		set.add(range(0x300, 0x400));
-	}
-
-	private Address addr(long offset) {
-		return space.getAddress(offset);
-	}
-
-	private AddressRange range(long start, long end) {
-		return new AddressRangeImpl(addr(start), addr(end));
-	}
-
-	@Test
-	public void test1Forward() {
-
-		AddressRangeIterator it = new RecoverableAddressRangeIterator(set, addr(150), true);
-
-		assertTrue(it.hasNext());
-		assertEquals(range(0x100, 0x200), it.next()); // triggers prefetch of range(0x250, 0x250)
-
-		set.add(addr(0x220)); // will get skipped due to underlying iterator prefetch
-
-		assertEquals(range(0x250, 0x250), it.next()); // triggers prefetch of range(0x300, 0x400)
-
-		set.add(range(0x350, 0x500)); // modifies existing RedBlackEntry node - no iterator recovery triggered
-
-		assertEquals(range(0x300, 0x400), it.next()); // triggers prefetch of END
-
-		assertFalse(it.hasNext());
-
-		try {
-			it.next();
-			fail("Expected NoSuchElementException");
-		}
-		catch (NoSuchElementException e) {
-			// expected
-		}
-	}
-
-	@Test
-	public void test2Forward() {
-
-		AddressRangeIterator it = new RecoverableAddressRangeIterator(set, addr(150), true);
-
-		assertTrue(it.hasNext());
-		assertEquals(range(0x100, 0x200), it.next()); // triggers prefetch of range(0x250, 0x250)
-
-		set.add(range(0x220, 0x400));
-
-		assertEquals(range(0x250, 0x250), it.next()); // triggers recovery prefetch of partial range(0x251, 0x400) 
-
-		assertEquals(range(0x251, 0x400), it.next()); // triggers prefetch of END
-
-		assertFalse(it.hasNext());
-	}
-
-	@Test
-	public void test3Forward() {
-
-		AddressRangeIterator it = new RecoverableAddressRangeIterator(set, addr(150), true);
-
-		assertTrue(it.hasNext());
-		assertEquals(range(0x100, 0x200), it.next()); // triggers prefetch of range(0x250, 0x250)
-
-		set.delete(range(0x220, 0x400));
-
-		assertEquals(range(0x250, 0x250), it.next()); // triggers recovery prefetch of END
-
-		assertFalse(it.hasNext());
-	}
-
-	@Test
-	public void test1Reverse() {
-
-		AddressRangeIterator it = new RecoverableAddressRangeIterator(set, addr(0x350), false);
-
-		assertTrue(it.hasNext());
-		assertEquals(range(0x300, 0x400), it.next()); // triggers prefetch of range(0x250, 0x250)
-
-		set.add(addr(0x220));
-
-		assertEquals(range(0x250, 0x250), it.next()); // triggers recovery prefetch of range(0x220, 0x220)
-
-		assertEquals(range(0x220, 0x220), it.next()); // triggers prefetch of range(0x100, 0x200)
-
-		assertEquals(range(0x100, 0x200), it.next()); // triggers prefetch of END
-
-		assertFalse(it.hasNext());
-	}
-
-	@Test
-	public void test2Reverse() {
-
-		AddressRangeIterator it = new RecoverableAddressRangeIterator(set, addr(0x350), false);
-
-		assertTrue(it.hasNext());
-		assertEquals(range(0x300, 0x400), it.next()); // triggers prefetch of range(0x250, 0x250)
-
-		set.add(range(0x220, 0x380));
-
-		assertEquals(range(0x250, 0x250), it.next()); // triggers recovery prefetch of partial range(0x220, 0x24f) 
-
-		assertEquals(range(0x220, 0x24f), it.next()); // triggers prefetch of range(0x100, 0x200)
-
-		assertEquals(range(0x100, 0x200), it.next()); // triggers prefetch of END
-
-		assertFalse(it.hasNext());
-	}
-
-	@Test
-	public void test3Reverse() {
-
-		AddressRangeIterator it = new RecoverableAddressRangeIterator(set, addr(0x350), false);
-
-		assertTrue(it.hasNext());
-		assertEquals(range(0x300, 0x400), it.next()); // triggers prefetch of range(0x250, 0x250)
-
-		set.delete(range(0x100, 0x220));
-
-		assertEquals(range(0x250, 0x250), it.next()); // triggers recovery prefetch of END
-
-		assertFalse(it.hasNext());
-	}
-
-}