Improvement of FileBytes equals/hashcode methods and delete

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

@@ -337,23 +337,25 @@ public class FileBytes {
 
 	@Override
 	public int hashCode() {
-		return (int) id;
+		return (int) (id ^ (id >>> 32));
 	}
 
 	@Override
 	public boolean equals(Object obj) {
-		if (this == obj) {
+		if (this == obj)
 			return true;
-		}
+		if (obj == null)
-		if (obj == null) {
 			return false;
-		}
+		if (getClass() != obj.getClass())
-		if (getClass() != obj.getClass()) {
 			return false;
-		}
 		FileBytes other = (FileBytes) obj;
-
+		if (memMap != other.memMap)
-		return id == other.id;
+			return false;
+		if (id != other.id)
+			return false;
+		if (invalid != other.invalid)
+			return false;
+		return true;
 	}
 
 	MemoryMapDB getMemMap() {

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

@@ -121,8 +121,9 @@ class FileBytesAdapterV0 extends FileBytesAdapter {
 
 	@Override
 	boolean deleteFileBytes(FileBytes fileBytes) throws IOException {
-		if (table.deleteRecord(fileBytes.getId())) {
+		if (fileBytesList.remove(fileBytes)) {
-			fileBytesList.remove(fileBytes);
+			table.deleteRecord(fileBytes.getId());
+			fileBytes.invalidate();
 			return true;
 		}
 		return false;

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

@@ -164,7 +164,6 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 		}
 	}
 
-
 	private void reloadAll() throws IOException {
 		synchronized (this) {
 			fileBytesAdapter.refresh();
@@ -495,6 +494,8 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 				return newBlock;
 			}
 			catch (IOCancelledException e) {
+				// TODO: this could leave things in a bad state.  
+				// Canceling requires additional improvements (see GT-3064)
 				throw new CancelledException();
 			}
 			catch (IOException e) {
@@ -511,7 +512,7 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 	@Override
 	public MemoryBlock createInitializedBlock(String name, Address start, FileBytes fileBytes,
 			long offset, long length, boolean overlay) throws LockException, DuplicateNameException,
-			MemoryConflictException, AddressOverflowException {
+			MemoryConflictException, AddressOverflowException, IndexOutOfBoundsException {
 
 		Objects.requireNonNull(name);
 		lock.acquire();
@@ -527,9 +528,8 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 				checkRange(start, length);
 			}
 			try {
-				MemoryBlockDB newBlock =
+				MemoryBlockDB newBlock = adapter.createFileBytesBlock(name, start, length,
-					adapter.createFileBytesBlock(name, start, length, fileBytes, offset,
+					fileBytes, offset, MemoryBlock.READ);
-					MemoryBlock.READ);
 				initializeBlocks();
 				addBlockAddresses(newBlock);
 				fireBlockAdded(newBlock);
@@ -547,12 +547,13 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 	}
 
 	private void checkFileBytesRange(FileBytes fileBytes, long offset, long length) {
-		if (length < 0) {
+		if (length <= 0) {
-			throw new IllegalArgumentException("Length must be >= 0, got " + length);
+			throw new IllegalArgumentException("Length must be > 0, got " + length);
 		}
 		if (offset < 0 || offset >= fileBytes.getSize()) {
+			long limit = fileBytes.getSize() - 1;
 			throw new IndexOutOfBoundsException(
-				"Offset must be in range [0," + length + "], got " + offset);
+				"Offset must be in range [0," + limit + "], got " + offset);
 		}
 		if (offset + length > fileBytes.getSize()) {
 			throw new IndexOutOfBoundsException(
@@ -638,9 +639,8 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 			checkRange(start, length);
 			overlayAddress.addNoWrap(length - 1);// just to check if length fits in address space
 			try {
-				MemoryBlockDB newBlock =
+				MemoryBlockDB newBlock = adapter.createBlock(MemoryBlockType.BYTE_MAPPED, name,
-					adapter.createBlock(MemoryBlockType.BYTE_MAPPED, name, start, length,
+					start, length, overlayAddress, false, MemoryBlock.READ);
-						overlayAddress, false, MemoryBlock.READ);
 				initializeBlocks();
 				addBlockAddresses(newBlock);
 				fireBlockAdded(newBlock);
@@ -673,9 +673,8 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 					MemoryBlockSourceInfo info = block.getSourceInfos().get(0);
 					overlayAddr = info.getMappedRange().get().getMinAddress();
 				}
-				MemoryBlockDB newBlock =
+				MemoryBlockDB newBlock = adapter.createBlock(block.getType(), name, start, length,
-					adapter.createBlock(block.getType(), name, start, length, overlayAddr,
+					overlayAddr, block.isInitialized(), block.getPermissions());
-					block.isInitialized(), block.getPermissions());
 				initializeBlocks();
 				addBlockAddresses(newBlock);
 				fireBlockAdded(newBlock);
@@ -2005,6 +2004,7 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 		}
 		return null;
 	}
+
 	private void checkBlockSize(long newBlockLength, boolean initialized) {
 		if (newBlockLength > MAX_BLOCK_SIZE) {
 			throw new IllegalStateException(
@@ -2026,6 +2026,8 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 			throws IOException {
 		lock.acquire();
 		try {
+			// TODO: this should accept task monitor to permit cancellation although
+			// canceling requires additional improvements (see GT-3064)
 			return fileBytesAdapter.createFileBytes(filename, offset, size, is);
 		}
 		finally {
@@ -2039,17 +2041,23 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 		return Collections.unmodifiableList(allFileBytes);
 	}
 
-	@Override
+	private void checkFileBytes(FileBytes fileBytes) {
-	public boolean deleteFileBytes(FileBytes fileBytes) throws IOException {
 		if (fileBytes.getMemMap() != this) {
 			throw new IllegalArgumentException(
 				"Attempted to delete FileBytes that doesn't belong to this program");
 		}
+		fileBytes.checkValid();
+	}
+
+	@Override
+	public boolean deleteFileBytes(FileBytes fileBytes) throws IOException {
 		lock.acquire();
 		try {
+			checkFileBytes(fileBytes);
 			if (inUse(fileBytes)) {
 				return false;
 			}
+			// TODO: may need to generate a domain object event
 			return fileBytesAdapter.deleteFileBytes(fileBytes);
 		}
 		finally {

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/mem/Memory.java

@@ -136,7 +136,7 @@ public interface Memory extends AddressSetView {
 	 * Create an initialized memory block and add it to this Memory.
 	 * @param name block name
 	 * @param start start of the block
-	 * @param size block length
+	 * @param size block length (positive non-zero value required)
 	 * @param initialValue initialization value for every byte in the block.
 	 * @param monitor progress monitor, may be null.
 	 * @param overlay if true, the block will be created as an OVERLAY which means that a new
@@ -164,7 +164,7 @@ public interface Memory extends AddressSetView {
 	 * @param start starting address of the block
 	 * @param fileBytes the {@link FileBytes} object to use as the underlying source of bytes.
 	 * @param offset the offset into the FileBytes for the first byte of this memory block.
-	 * @param size block length
+	 * @param size block length (positive non-zero value required)
 	 * @param overlay if true, the block will be created as an OVERLAY which means that a new
 	 * overlay address space will be created and the block will have a starting address at the same
 	 * offset as the given start address parameter, but in the new address space.
@@ -174,8 +174,9 @@ public interface Memory extends AddressSetView {
 	 * space with the same name as this memory block 
 	 * @throws MemoryConflictException if the new block overlaps with a
 	 * previous block
-	 * @throws AddressOverflowException if the start is beyond the
+	 * @throws AddressOverflowException if the start is beyond the address space
-	 * address space
+	 * @throws IndexOutOfBoundsException if file bytes range specified by offset and size 
+	 * is out of bounds for the specified fileBytes.
 	 */
 	public MemoryBlock createInitializedBlock(String name, Address start, FileBytes fileBytes,
 			long offset, long size, boolean overlay) throws LockException, DuplicateNameException,
@@ -251,7 +252,7 @@ public interface Memory extends AddressSetView {
 			throws LockException, MemoryConflictException, AddressOverflowException;
 
 	/**
-	 * Remove the memory block
+	 * Remove the memory block.  
 	 *
 	 * @param block the block to be removed.
 	 * @param monitor monitor that is used to cancel the remove operation
@@ -743,9 +744,10 @@ public interface Memory extends AddressSetView {
 	/**
 	 * Deletes a stored sequence of file bytes.  The file bytes can only be deleted if there
 	 * are no memory block references to the file bytes.
+	 * 
 	 * @param fileBytes the FileBytes for the file bytes to be deleted.
-	 * @return true if the FileBytes was deleted.  If any memory blNoocks are reference this FileBytes,
+	 * @return true if the FileBytes was deleted.  If any memory blocks are referenced by this 
-	 * then it will not be deleted and false will be returned.
+	 * FileBytes or it is invalid then it will not be deleted and false will be returned.
 	 * @throws IOException if there was an error updating the database.
 	 */
 	public boolean deleteFileBytes(FileBytes fileBytes) throws IOException;