diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/mem/FileBytes.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/mem/FileBytes.java index 18c364b8d6..8b4777043c 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/mem/FileBytes.java +++ b/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; - - return id == other.id; + if (memMap != other.memMap) + return false; + if (id != other.id) + return false; + if (invalid != other.invalid) + return false; + return true; } MemoryMapDB getMemMap() { diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/mem/FileBytesAdapterV0.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/mem/FileBytesAdapterV0.java index fb82ab0073..3dc050b024 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/mem/FileBytesAdapterV0.java +++ b/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())) { - fileBytesList.remove(fileBytes); + if (fileBytesList.remove(fileBytes)) { + table.deleteRecord(fileBytes.getId()); + fileBytes.invalidate(); return true; } return false; diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/mem/MemoryMapDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/mem/MemoryMapDB.java index 587ee3e00d..bc9a2c7c17 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/mem/MemoryMapDB.java +++ b/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(); @@ -261,7 +260,7 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener { void checkMemoryWrite(MemoryBlockDB block, Address start, long length) throws MemoryAccessException { checkRangeForInstructions(start, start.add(length - 1)); - + Set overlappingBlocks = getPotentialOverlappingBlocks(); ByteSourceRangeList changeingByteSource = block.getByteSourceRangeList(start, length); if (overlappingBlocks.contains(block)) { @@ -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 = - adapter.createFileBytesBlock(name, start, length, fileBytes, offset, - MemoryBlock.READ); + MemoryBlockDB newBlock = adapter.createFileBytesBlock(name, start, length, + fileBytes, offset, 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) { - throw new IllegalArgumentException("Length must be >= 0, got " + length); + if (length <= 0) { + 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 = - adapter.createBlock(MemoryBlockType.BYTE_MAPPED, name, start, length, - overlayAddress, false, MemoryBlock.READ); + MemoryBlockDB newBlock = adapter.createBlock(MemoryBlockType.BYTE_MAPPED, name, + start, length, 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 = - adapter.createBlock(block.getType(), name, start, length, overlayAddr, - block.isInitialized(), block.getPermissions()); + MemoryBlockDB newBlock = adapter.createBlock(block.getType(), name, start, length, + overlayAddr, 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 - public boolean deleteFileBytes(FileBytes fileBytes) throws IOException { + private void checkFileBytes(FileBytes fileBytes) { 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 { diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/mem/Memory.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/mem/Memory.java index 98ecef9df7..c5a60d60a2 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/mem/Memory.java +++ b/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 - * address space + * @throws AddressOverflowException if the start is beyond the 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, - * then it will not be deleted and false will be returned. + * @return true if the FileBytes was deleted. If any memory blocks are referenced by this + * 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;