GP-701 fixed stack trace in expand data in selection when crossing address spaces

Ghidra/Features/Base/src/main/java/ghidra/app/util/viewer/util/OpenCloseManager.java

@@ -19,8 +19,7 @@ import java.util.*;
 
 import javax.swing.event.ChangeListener;
 
-import ghidra.program.model.address.Address;
+import ghidra.program.model.address.*;
-import ghidra.program.model.address.AddressSetView;
 import ghidra.program.model.listing.*;
 import ghidra.util.task.TaskMonitor;
 import ghidra.util.task.TaskMonitorAdapter;
@@ -254,11 +253,13 @@ public class OpenCloseManager {
 
 	private void toggleAllDataInAddresses(boolean open, Program program, AddressSetView addresses,
 			TaskMonitor monitor) {
+
+		AddressSet unprocessed = new AddressSet(addresses);
+
 		monitor.initialize(addresses.getNumAddresses());
-		Address start = addresses.getMinAddress();
 
 		Listing listing = program.getListing();
-		DataIterator iterator = listing.getData(addresses, true);
+		DataIterator iterator = listing.getDefinedData(addresses, true);
 		while (iterator.hasNext()) {
 			if (monitor.isCancelled()) {
 				return;
@@ -268,9 +269,11 @@ public class OpenCloseManager {
 
 			toggleDataRecursively(data, open, monitor);
 
-			Address max = data.getMaxAddress();
+			Address min = data.getMinAddress();
-			long progress = max.subtract(start);
 
+			unprocessed.deleteFromMin(min);
+
+			long progress = addresses.getNumAddresses() - unprocessed.getNumAddresses();
 			monitor.setProgress(progress);
 		}
 

Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/RedBlackTree.java

@@ -15,15 +15,13 @@
  */
 package ghidra.util.datastruct;
 
-import static ghidra.util.datastruct.RedBlackEntry.NodeColor.BLACK;
+import static ghidra.util.datastruct.RedBlackEntry.NodeColor.*;
-import static ghidra.util.datastruct.RedBlackEntry.NodeColor.RED;
 
 import java.util.ConcurrentModificationException;
 import java.util.ListIterator;
 
 import org.apache.commons.collections4.iterators.EmptyListIterator;
 
-
 /**
  * A RedBlack Tree implementation with K type keys and place to store V type values.
  */
@@ -206,7 +204,8 @@ public class RedBlackTree<K extends Comparable<K>, V> implements Iterable<RedBla
 
 	private RedBlackEntry<K, V> getNode(K key) {
 		RedBlackEntry<K, V> node = getEntryLessThanEqual(key);
-		if (node != null && node.getKey().equals(key)) {
+		if (node != null && node.getKey()
+				.equals(key)) {
 			return node;
 		}
 		return null;
@@ -709,6 +708,10 @@ public class RedBlackTree<K extends Comparable<K>, V> implements Iterable<RedBla
 				previousNode = forward ? getLast() : getFirst();
 			}
 			lastReturnedNode = null;
+
+			// Update the iterator modCount to match the modCount of the main class, since deletes
+			// made via the iterator should not cause a concurrent modification exception.
+			this.expectedModCount = modCount;
 		}
 
 		@Override

Ghidra/Framework/Generic/src/test/java/ghidra/util/datastruct/RedBlackTreeTest.java

@@ -17,6 +17,8 @@ package ghidra.util.datastruct;
 
 import static org.junit.Assert.*;
 
+import java.util.ListIterator;
+
 import org.junit.Before;
 import org.junit.Test;
 
@@ -29,8 +31,8 @@ public class RedBlackTreeTest extends AbstractGenericTest {
 		super();
 	}
 
-    @Before
+	@Before
-    public void setUp() throws Exception {
+	public void setUp() throws Exception {
 		tree = new RedBlackTree<Integer, String>();
 
 		tree.put(5, "five");
@@ -45,13 +47,13 @@ public class RedBlackTreeTest extends AbstractGenericTest {
 
 	}
 
-@Test
+	@Test
-    public void testSize() {
+	public void testSize() {
 		assertEquals(9, tree.size());
 	}
 
-@Test
+	@Test
-    public void testContains() {
+	public void testContains() {
 		assertTrue(tree.containsKey(1));
 		assertTrue(tree.containsKey(2));
 		assertTrue(tree.containsKey(3));
@@ -68,8 +70,8 @@ public class RedBlackTreeTest extends AbstractGenericTest {
 
 	}
 
-@Test
+	@Test
-    public void testGetFirstLast() {
+	public void testGetFirstLast() {
 		RedBlackEntry<Integer, String> node = tree.getFirst();
 		assertEquals(1, (int) node.getKey());
 
@@ -112,8 +114,8 @@ public class RedBlackTreeTest extends AbstractGenericTest {
 //
 //	}
 
-@Test
+	@Test
-    public void testGetNextNode() {
+	public void testGetNextNode() {
 		RedBlackEntry<Integer, String> node = tree.getFirst();
 		assertEquals("one", node.value);
 		node = node.getSuccessor();
@@ -137,16 +139,77 @@ public class RedBlackTreeTest extends AbstractGenericTest {
 
 	}
 
-@Test
+	@Test
-    public void testRemove() {
+	public void testRemove() {
 		assertEquals(9, tree.size());
 		tree.remove(5);
 		assertEquals(8, tree.size());
 
 	}
 
-@Test
+	@Test
-    public void testDepth() {
+	public void testIterator() {
+		ListIterator<RedBlackEntry<Integer, String>> it = tree.iterator();
+
+		RedBlackEntry<Integer, String> node = it.next();
+		assertTrue(it.hasNext());
+		assertEquals("one", node.value);
+		node = it.next();
+		assertEquals("two", node.value);
+		node = it.next();
+		assertEquals("three", node.value);
+		node = it.next();
+		assertEquals("four", node.value);
+		node = it.next();
+		assertTrue(it.hasNext());
+		assertEquals("five", node.value);
+		node = it.next();
+		assertEquals("six", node.value);
+		node = it.next();
+		assertEquals("seven", node.value);
+		node = it.next();
+		assertEquals("nine", node.value);
+		node = it.next();
+		assertEquals("ten", node.value);
+		assertFalse(it.hasNext());
+		node = it.next();
+		assertNull(node);
+
+		assertEquals(9, tree.size());
+		tree.remove(5);
+		assertEquals(8, tree.size());
+
+	}
+
+	@Test
+	public void testIteratorRemove() {
+		assertEquals(9, tree.size());
+		assertTrue(tree.containsKey(3));
+
+		ListIterator<RedBlackEntry<Integer, String>> it = tree.iterator();
+		while (it.hasNext()) {
+			RedBlackEntry<Integer, String> next = it.next();
+			if ("three".equals(next.value)) {
+				it.remove();
+			}
+		}
+		assertEquals(8, tree.size());
+		assertFalse(tree.containsKey(3));
+
+		it = tree.iterator();
+
+		RedBlackEntry<Integer, String> node = it.next();
+		assertTrue(it.hasNext());
+		assertEquals("one", node.value);
+		node = it.next();
+		assertEquals("two", node.value);
+		node = it.next();
+		assertEquals("four", node.value);
+
+	}
+
+	@Test
+	public void testDepth() {
 		tree = new RedBlackTree<Integer, String>();
 		tree.put(1, "one");
 		tree.put(2, "two");

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

@@ -1232,6 +1232,7 @@ public class AddressSet implements AddressSetView {
 	private class AddressRangeIteratorAdapter implements AddressRangeIterator {
 
 		private Iterator<RedBlackEntry<Address, Address>> iterator;
+		private AddressRange lastReturnedRange;
 
 		public AddressRangeIteratorAdapter(Iterator<RedBlackEntry<Address, Address>> iterator) {
 			this.iterator = iterator;
@@ -1248,12 +1249,14 @@ public class AddressSet implements AddressSetView {
 			if (next == null) {
 				throw new NoSuchElementException();
 			}
-			return new AddressRangeImpl(next.getKey(), next.getValue());
+			lastReturnedRange = new AddressRangeImpl(next.getKey(), next.getValue());
+			return lastReturnedRange;
 		}
 
 		@Override
 		public void remove() {
 			iterator.remove();
+			addressCount -= lastReturnedRange.getLength();
 		}
 
 		@Override

Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/model/address/AddressSetTest.java

@@ -762,6 +762,28 @@ public class AddressSetTest extends AbstractGenericTest {
 		assertTrue(!it.hasNext());
 	}
 
+	@Test
+	public void testForwardRangeIteratorRemove() {
+		AddressSet set = set(0x100, 0x110, 0x200, 0x210, 0x300, 0x305);
+		assertEquals(40, set.getNumAddresses());
+		assertEquals(3, set.getNumAddressRanges());
+
+		Iterator<AddressRange> it = set.iterator(true);
+		Assert.assertEquals(range(0x100, 0x110), it.next());
+		Assert.assertEquals(range(0x200, 0x210), it.next());
+		it.remove();
+		Assert.assertEquals(range(0x300, 0x305), it.next());
+		assertTrue(!it.hasNext());
+
+		it = set.iterator(true);
+		Assert.assertEquals(range(0x100, 0x110), it.next());
+		Assert.assertEquals(range(0x300, 0x305), it.next());
+		assertTrue(!it.hasNext());
+
+		assertEquals(23, set.getNumAddresses());
+		assertEquals(2, set.getNumAddressRanges());
+	}
+
 	@Test
 	public void testBackwardRangeIterator() {
 		AddressSet set = set(0x100, 0x110, 0x200, 0x210, 0x300, 0x305);
@@ -772,6 +794,28 @@ public class AddressSetTest extends AbstractGenericTest {
 		assertTrue(!it.hasNext());
 	}
 
+	@Test
+	public void testBackwardRangeIteratorRemove() {
+		AddressSet set = set(0x100, 0x110, 0x200, 0x210, 0x300, 0x305);
+		assertEquals(40, set.getNumAddresses());
+		assertEquals(3, set.getNumAddressRanges());
+
+		Iterator<AddressRange> it = set.iterator(false);
+		Assert.assertEquals(range(0x300, 0x305), it.next());
+		Assert.assertEquals(range(0x200, 0x210), it.next());
+		it.remove();
+		Assert.assertEquals(range(0x100, 0x110), it.next());
+		assertTrue(!it.hasNext());
+
+		it = set.iterator(false);
+		Assert.assertEquals(range(0x300, 0x305), it.next());
+		Assert.assertEquals(range(0x100, 0x110), it.next());
+		assertTrue(!it.hasNext());
+
+		assertEquals(23, set.getNumAddresses());
+		assertEquals(2, set.getNumAddressRanges());
+	}
+
 	@Test
 	public void testGetRangeContaining() {
 		AddressSet set = set(0x100, 0x110, 0x200, 0x210, 0x300, 0x305);