GP-2198 correct ELF Loader issues which failed to some non-loaded memory blocks. Improve use of task monitor during ELF import and cancellation. Also corrected bug in memory map table sort for Byte Source column.

2024-11-23 12:49:45 +00:00 · 2022-06-21 12:07:57 -04:00 · 2022-06-21 12:07:57 -04:00 · e681b3dc4b
commit e681b3dc4b
parent 0e3fe30c67
10 changed files with 189 additions and 68 deletions
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/memory/MemoryMapModel.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/memory/MemoryMapModel.java
@ -535,6 +535,9 @@ class MemoryMapModel extends AbstractSortedTableModel<MemoryBlock> {

 	@Override
 	protected Comparator<MemoryBlock> createSortComparator(int columnIndex) {
+		if (columnIndex == BYTE_SOURCE) {
+			return super.createSortComparator(columnIndex);
+		}
 		return new MemoryMapComparator(columnIndex);
 	}

@ -581,6 +584,9 @@ class MemoryMapModel extends AbstractSortedTableModel<MemoryBlock> {
 					int b1init = (b1.isInitialized() ? 1 : -1);
 					int b2init = (b2.isInitialized() ? 1 : -1);
 					return (b1init - b2init);
+
+				//case BYTE_SOURCE: - handled by default comparator
+
 				case SOURCE:
 					String b1src = b1.getSourceName();
 					String b2src = b2.getSourceName();
@ -608,7 +614,7 @@ class MemoryMapModel extends AbstractSortedTableModel<MemoryBlock> {
 					String bt2 = b2.getType().toString();
 					return bt1.compareToIgnoreCase(bt2);
 				default:
-					return 0;
+					throw new RuntimeException("Unimplemented column comparator: " + sortColumn);
 			}
 		}
 	}
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/MemoryBlockUtils.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/MemoryBlockUtils.java
@ -329,6 +329,7 @@ public class MemoryBlockUtils {
 	 * @return the newly created FileBytes object.
 	 * @param monitor the monitor for canceling this potentially long running operation.
 	 * @throws IOException if an IOException occurred.
+	 * @throws CancelledException if the user cancelled the operation
 	 */
 	public static FileBytes createFileBytes(Program program, ByteProvider provider,
 			TaskMonitor monitor) throws IOException, CancelledException {
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/ElfProgramBuilder.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/ElfProgramBuilder.java
@ -119,29 +119,22 @@ class ElfProgramBuilder extends MemorySectionResolver implements ElfLoadHelper {
 		int id = program.startTransaction("Load ELF program");
 		boolean success = false;
 		try {
-			monitor.setIndeterminate(true);
-
 			addProgramProperties(monitor);

 			setImageBase();
 			program.setExecutableFormat(ElfLoader.ELF_NAME);

-			// resolve segment/sections and create program memory blocks
 			ByteProvider byteProvider = elf.getByteProvider();
-			try (InputStream fileIn = byteProvider.getInputStream(0)) {
-				fileBytes = program.getMemory()
-						.createFileBytes(byteProvider.getName(), 0, byteProvider.length(), fileIn,
-							monitor);
-			}

-			adjustSegmentAndSectionFileAllocations(byteProvider);
+			createFileBytes(byteProvider, monitor);
+
+			adjustSegmentAndSectionFileAllocations(byteProvider, monitor);

 			// process headers and define "section" within memory elfProgramBuilder
 			processProgramHeaders(monitor);
 			processSectionHeaders(monitor);

-			monitor.setIndeterminate(false);
-
+			// resolve segment/sections and create program memory blocks
 			resolve(monitor);

 			if (elf.getSectionHeaderCount() == 0) {
@ -159,23 +152,30 @@ class ElfProgramBuilder extends MemorySectionResolver implements ElfLoadHelper {
 			markupProgramHeaders(monitor);
 			markupSectionHeaders(monitor);

-			monitor.setProgress(0);
 			monitor.setIndeterminate(true);

 			markupDynamicTable(monitor);
 			markupInterpreter(monitor);

+			monitor.setIndeterminate(false);
+
 			processStringTables(monitor);

 			processSymbolTables(monitor);

+			monitor.setIndeterminate(true);
+
 			elf.getLoadAdapter().processElf(this, monitor);

 			processEntryPoints(monitor);

+			monitor.setIndeterminate(false);
+
 			processRelocations(monitor);
 			processImports(monitor);

+			monitor.setIndeterminate(true);
+
 			monitor.setMessage("Processing PLT/GOT ...");
 			elf.getLoadAdapter().processGotPlt(this, monitor);

@ -194,9 +194,22 @@ class ElfProgramBuilder extends MemorySectionResolver implements ElfLoadHelper {
 		}
 	}

-	private void adjustSegmentAndSectionFileAllocations(
-			ByteProvider byteProvider)
-			throws IOException {
+	private void createFileBytes(ByteProvider byteProvider,
+			TaskMonitor monitor) throws IOException, CancelledException {
+		monitor.setMessage("Loading FileBytes...");
+		try (InputStream fileIn = byteProvider.getInputStream(0);
+				MonitoredInputStream mis = new MonitoredInputStream(fileIn, monitor)) {
+			// Indicate that cleanup is not neccessary for cancelled import operation.
+			mis.setCleanupOnCancel(false);
+			fileBytes = program.getMemory()
+					.createFileBytes(byteProvider.getName(), 0, byteProvider.length(), mis,
+						monitor);
+		}
+	}
+
+	private void adjustSegmentAndSectionFileAllocations(ByteProvider byteProvider,
+			TaskMonitor monitor)
+			throws IOException, CancelledException {

 		// Identify file ranges not allocated to segments or sections
 		RangeMap fileMap = new RangeMap();
@ -205,7 +218,12 @@ class ElfProgramBuilder extends MemorySectionResolver implements ElfLoadHelper {
 		ElfProgramHeader[] segments = elf.getProgramHeaders();
 		ElfSectionHeader[] sections = elf.getSections();

+		monitor.setMessage("Examining file allocations...");
+		monitor.initialize(segments.length + sections.length);
+
 		for (ElfProgramHeader segment : segments) {
+			monitor.checkCanceled();
+			monitor.incrementProgress(1);
 			if (segment.getType() == ElfProgramHeaderConstants.PT_NULL) {
 				continue;
 			}
@ -217,6 +235,8 @@ class ElfProgramBuilder extends MemorySectionResolver implements ElfLoadHelper {
 		}

 		for (ElfSectionHeader section : sections) {
+			monitor.checkCanceled();
+			monitor.incrementProgress(1);
 			if (section.getType() == ElfSectionHeaderConstants.SHT_NULL ||
 				section.getType() == ElfSectionHeaderConstants.SHT_NOBITS) {
 				continue;
@ -243,9 +263,14 @@ class ElfProgramBuilder extends MemorySectionResolver implements ElfLoadHelper {
 		}

 		// Add unallocated non-zero file regions as OTHER blocks
+		monitor.setMessage("Identify unallocated file regions...");
+		monitor.initialize(fileMap.getNumRanges());
+
 		IndexRangeIterator rangeIterator = fileMap.getIndexRangeIterator(0);
 		int unallocatedIndex = 0;
 		while (rangeIterator.hasNext()) {
+			monitor.checkCanceled();
+			monitor.incrementProgress(1);
 			IndexRange range = rangeIterator.next();
 			int value = fileMap.getValue(range.getStart());
 			if (value != -1) {
@ -1058,7 +1083,7 @@ class ElfProgramBuilder extends MemorySectionResolver implements ElfLoadHelper {
 			}

 			ElfProgramHeader[] programHeaders = elf.getProgramHeaders();
-			monitor.setMaximum(programHeaders.length);
+			monitor.initialize(programHeaders.length);
 			int vaddrFieldIndex = elf.is64Bit() ? 3 : 2; // p_vaddr structure element index
 			for (int i = 0; i < programHeaders.length; i++) {
 				monitor.checkCanceled();
@ -1125,7 +1150,7 @@ class ElfProgramBuilder extends MemorySectionResolver implements ElfLoadHelper {
 			}

 			ElfSectionHeader[] sections = elf.getSections();
-			monitor.setMaximum(sections.length);
+			monitor.initialize(sections.length);
 			for (int i = 0; i < sections.length; i++) {
 				monitor.checkCanceled();
 				monitor.incrementProgress(1);
@ -2772,10 +2797,10 @@ class ElfProgramBuilder extends MemorySectionResolver implements ElfLoadHelper {
 	 */
 	private void expandProgramHeaderBlocks(TaskMonitor monitor) throws CancelledException {

-		monitor.setMessage("Exapanding Program Segments...");
-
 		ElfProgramHeader[] elfProgramHeaders = elf.getProgramHeaders();
-		monitor.setMaximum(elfProgramHeaders.length);
+
+		monitor.setMessage("Exapanding Program Segments...");
+		monitor.initialize(elfProgramHeaders.length);
 		for (int i = 0; i < elfProgramHeaders.length; ++i) {
 			monitor.checkCanceled();
 			monitor.incrementProgress(1);
@ -2887,13 +2912,15 @@ class ElfProgramBuilder extends MemorySectionResolver implements ElfLoadHelper {
 			return;
 		}

-		monitor.setMessage("Processing program headers...");
-
 		boolean includeOtherBlocks = ElfLoaderOptionsFactory.includeOtherBlocks(options);

 		ElfProgramHeader[] elfProgramHeaders = elf.getProgramHeaders();
+
+		monitor.setMessage("Processing program headers...");
+		monitor.initialize(elfProgramHeaders.length);
 		for (int i = 0; i < elfProgramHeaders.length; ++i) {
 			monitor.checkCanceled();
+			monitor.incrementProgress(1);
 			ElfProgramHeader elfProgramHeader = elfProgramHeaders[i];
 			if (elfProgramHeader.getType() == ElfProgramHeaderConstants.PT_NULL) {
 				continue;
@ -2955,9 +2982,14 @@ class ElfProgramBuilder extends MemorySectionResolver implements ElfLoadHelper {
 		boolean maintainExecuteBit = elf.getSectionHeaderCount() == 0;

 		if (fullSizeBytes <= 0) {
-			log("Skipping zero-length segment [" + segmentNumber + "," +
-				elfProgramHeader.getDescription() + "] at address " + address.toString(true));
-			return;
+			if (!space.isLoadedMemorySpace() && loadSizeBytes > 0) {
+				fullSizeBytes = loadSizeBytes;
+			}
+			else {
+				log("Skipping zero-length segment [" + segmentNumber + "," +
+					elfProgramHeader.getDescription() + "] at address " + address.toString(true));
+				return;
+			}
 		}

 		if (!space.isValidRange(address.getOffset(), fullSizeBytes)) {
@ -2976,7 +3008,7 @@ class ElfProgramBuilder extends MemorySectionResolver implements ElfLoadHelper {
 				comment += " (disabled execute bit)";
 			}

-			String blockName = String.format("%s%d", SEGMENT_NAME_PREFIX, segmentNumber);
+			String blockName = getSegmentName(elfProgramHeader, segmentNumber);
 			if (loadSizeBytes != 0) {
 				addInitializedMemorySection(elfProgramHeader, elfProgramHeader.getOffset(),
 					loadSizeBytes, address, blockName, elfProgramHeader.isRead(),
@ -2995,6 +3027,14 @@ class ElfProgramBuilder extends MemorySectionResolver implements ElfLoadHelper {
 		}
 	}

+	private String getSegmentName(ElfProgramHeader elfProgramHeader, int segmentNumber) {
+		int headerType = elfProgramHeader.getType();
+		if (headerType == ElfProgramHeaderConstants.PT_NOTE) {
+			return "_elfNote";
+		}
+		return String.format("%s%d", SEGMENT_NAME_PREFIX, segmentNumber);
+	}
+
 	private String getSectionComment(long addr, long byteSize, int addressableUnitSize,
 			String description, boolean loaded) {
 		StringBuilder buf = new StringBuilder();
@ -3099,8 +3139,12 @@ class ElfProgramBuilder extends MemorySectionResolver implements ElfLoadHelper {
 		}

 		ElfSectionHeader[] sections = elf.getSections();
+
+		monitor.setMessage("Processing section headers...");
+		monitor.initialize(sections.length);
 		for (ElfSectionHeader elfSectionToLoad : sections) {
 			monitor.checkCanceled();
+			monitor.incrementProgress(1);
 			int type = elfSectionToLoad.getType();
 			if (type != ElfSectionHeaderConstants.SHT_NULL &&
 				(includeOtherBlocks || elfSectionToLoad.isAlloc())) {
--- a/Ghidra/Framework/Generic/src/main/java/ghidra/util/MonitoredInputStream.java
+++ b/Ghidra/Framework/Generic/src/main/java/ghidra/util/MonitoredInputStream.java
@ -23,7 +23,8 @@ import ghidra.util.task.TaskMonitor;

 /**
 * An InputStream which utilizes a TaskMonitor to indicate input progress and
- * allows the operation to be cancelled via the TaskMonitor.
+ * allows the operation to be cancelled via the TaskMonitor.  If monitor is
+ * cancelled any susequent read will generate a {@link IOCancelledException}.
 */
 public class MonitoredInputStream extends InputStream {

@ -32,7 +33,8 @@ public class MonitoredInputStream extends InputStream {
 	protected InputStream in;
 	private TaskMonitor monitor;
 	private int smallCount = 0;
-	private int count = 0;
+	private long count = 0;
+	private boolean cleanupOnCancel;

 	public MonitoredInputStream(InputStream in, TaskMonitor monitor) {
 		this.in = in;
@ -40,10 +42,19 @@ public class MonitoredInputStream extends InputStream {
 	}

 	/**
-	 * Reset the current progress count to the specified value.
+	 * Get task monitor associated within this input stream.
+	 * @return task monitor
 	 */
-	public void setProgress(int count) {
-		this.count = count;
+	public TaskMonitor getTaskMonitor() {
+		return monitor;
+	}
+
+	/**
+	 * Reset the current progress count to the specified value.
+	 * @param progress current progress 
+	 */
+	public void setProgress(long progress) {
+		this.count = progress;
 	}

 	/**
@ -245,4 +256,25 @@ public class MonitoredInputStream extends InputStream {
 	public boolean markSupported() {
 		return in.markSupported();
 	}
+
+	/**
+	 * Convey to byte stream consumer if cleanup of any artifacts produced is recommended, when 
+	 * applicable, if {@link IOCancelledException} is thrown by this input stream.
+	 * @param enable true if cleanup recommended, false if no cleanup neccessary (default).
+	 * @return this instance
+	 */
+	public MonitoredInputStream setCleanupOnCancel(boolean enable) {
+		cleanupOnCancel = enable;
+		return this;
+	}
+
+	/**
+	 * Determine if artifact cleanup is recommended when possible following cancellation
+	 * of this input stream (i.e., {@link IOCancelledException} has been caught).
+	 * @return true if cleanup recommended, false if no cleanup required.
+	 */
+	public boolean cleanupOnCancel() {
+		return cleanupOnCancel;
+	}
+
 }
--- a/Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/RangeMap.java
+++ b/Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/RangeMap.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.
@ -20,8 +19,6 @@ import ghidra.util.LongIterator;
 import ghidra.util.exception.NoValueException;
 import ghidra.util.prop.IntPropertySet;

-import java.io.Serializable;
-
 /**
 * Stores ranges of int values throughout "long" space. Every "long" index has
 * an associated int value (initially 0). Users can paint (set) ranges of
@ -34,8 +31,7 @@ import java.io.Serializable;
 * value stored there, then the value stored at the nearest previous index
 * that contains a value.
 */
-public class RangeMap implements Serializable {
-    private final static long serialVersionUID = 1;
+public class RangeMap {
 	
 	IntPropertySet map;
 	int defaultValue;
@ -57,6 +53,14 @@ public class RangeMap implements Serializable {
 		map.putInt(0, defaultValue);		
 	}
 	
+	/**
+	 * Get the total number of ranges in map.
+	 * @return number of ranges
+	 */
+	public int getNumRanges() {
+		return map.getSize();
+	}
+
 	/**
 	 * Clears all current values from the range map and resets the default value.
 	 */
@ -175,6 +179,4 @@ public class RangeMap implements Serializable {
 	public LongIterator getChangePointIterator(long start, long end) {
 		return map.getPropertyIterator(start, end);	
 	}
-
-
 }
--- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/mem/FileBytesAdapter.java
+++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/mem/FileBytesAdapter.java
@ -20,6 +20,8 @@ import java.io.InputStream;
 import java.util.List;

 import db.*;
+import ghidra.util.MonitoredInputStream;
+import ghidra.util.exception.IOCancelledException;
 import ghidra.util.exception.VersionException;
 import ghidra.util.task.TaskMonitor;

@ -75,8 +77,21 @@ abstract class FileBytesAdapter {
 		return new FileBytesAdapterV0(handle, true);
 	}

-	abstract FileBytes createFileBytes(String filename, long offset, long size, InputStream is)
-			throws IOException;
+	/**
+	 * Create {@link FileBytes} from specified input stream
+	 * @param filename name of original file
+	 * @param offset position of input stream within original file or 0 if no file
+	 * @param size number of bytes to be read from input stream for stored file bytes
+	 * @param is input stream
+	 * @param monitor task monitor for progress and to allow cancellation.  This will be ignored if 
+	 * input stream is a {@link MonitoredInputStream}.  Monitor may be reinitialized and progress
+	 * updated while reading from input stream.
+	 * @return new file bytes
+	 * @throws IOException if error occurs reading input stream or writing file bytes to database
+	 * @throws IOCancelledException if operation was cancelled
+	 */
+	abstract FileBytes createFileBytes(String filename, long offset, long size, InputStream is,
+			TaskMonitor monitor) throws IOException;

 	/**
 	 * Returns a DBBuffer object for the given database buffer id
--- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/mem/FileBytesAdapterNoTable.java
+++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/mem/FileBytesAdapterNoTable.java
@ -21,6 +21,7 @@ import java.util.List;

 import db.DBBuffer;
 import db.DBHandle;
+import ghidra.util.task.TaskMonitor;

 /**
 * Version of the FileBytesAdapter used to access older databases for read-only and upgrade purposes.
@ -32,7 +33,8 @@ class FileBytesAdapterNoTable extends FileBytesAdapter {
 	}

 	@Override
-	FileBytes createFileBytes(String filename, long offset, long size, InputStream is) {
+	FileBytes createFileBytes(String filename, long offset, long size, InputStream is,
+			TaskMonitor monitor) {
 		throw new UnsupportedOperationException();
 	}

--- 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
@ -20,8 +20,10 @@ import java.io.InputStream;
 import java.util.*;

 import db.*;
+import ghidra.util.MonitoredInputStream;
 import ghidra.util.exception.IOCancelledException;
 import ghidra.util.exception.VersionException;
+import ghidra.util.task.TaskMonitor;

 /**
 * Initial version of the FileBytesAdapter
@ -71,9 +73,9 @@ class FileBytesAdapterV0 extends FileBytesAdapter {
 	}

 	@Override
-	FileBytes createFileBytes(String filename, long offset, long size, InputStream is)
-			throws IOException {
-		DBBuffer[] buffers = createBuffers(size, is);
+	FileBytes createFileBytes(String filename, long offset, long size, InputStream is,
+			TaskMonitor monitor) throws IOException {
+		DBBuffer[] buffers = createBuffers(size, is, monitor);
 		DBBuffer[] layeredBuffers = createLayeredBuffers(buffers);
 		int[] bufIds = getIds(buffers);
 		int[] layeredBufIds = getIds(layeredBuffers);
@ -152,7 +154,24 @@ class FileBytesAdapterV0 extends FileBytesAdapter {
 		return layeredBuffers;
 	}

-	private DBBuffer[] createBuffers(long size, InputStream is) throws IOException {
+	@SuppressWarnings("resource")
+	private DBBuffer[] createBuffers(long size, InputStream is, TaskMonitor monitor)
+			throws IOException {
+
+		if (monitor == null) {
+			monitor = TaskMonitor.DUMMY;
+		}
+		MonitoredInputStream mis;
+		if (is instanceof MonitoredInputStream) {
+			mis = (MonitoredInputStream) is;
+			mis.getTaskMonitor().initialize(size);
+		}
+		else {
+			// caller responsible for closing input stream provided
+			mis = new MonitoredInputStream(is, monitor).setCleanupOnCancel(true);
+			monitor.initialize(size);
+		}
+
 		int maxBufSize = getMaxBufferSize();
 		int bufCount = (int) (size / maxBufSize);
 		int sizeLastBuf = (int) (size % maxBufSize);
@ -174,12 +193,21 @@ class FileBytesAdapterV0 extends FileBytesAdapter {

 		try {
 			for (DBBuffer buffer : buffers) {
-				buffer.fill(is);
+				buffer.fill(mis);
 			}
 		}
 		catch (IOCancelledException e) {
-			for (DBBuffer buffer : buffers) {
-				buffer.delete();
+			if (mis.cleanupOnCancel()) {
+				// Optional cleanup which can be avoided during import where entire program
+				// will get removed on cancel.
+				monitor.initialize(buffers.length);
+				monitor.setMessage("Cancelling...");
+				monitor.setCancelEnabled(false);
+				for (DBBuffer buffer : buffers) {
+					buffer.delete();
+					monitor.incrementProgress(1);
+				}
+				monitor.setIndeterminate(true);
 			}
 			throw e;
 		}
--- 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
@ -2196,11 +2196,7 @@ public class MemoryMapDB implements Memory, ManagerDB, LiveMemoryListener {
 		}
 		lock.acquire();
 		try {
-			if (monitor != null && is != null) {
-				is = new MonitoredInputStream(is, monitor);
-				monitor.initialize(size);
-			}
-			return fileBytesAdapter.createFileBytes(filename, offset, size, is);
+			return fileBytesAdapter.createFileBytes(filename, offset, size, is, monitor);
 		}
 		catch (IOCancelledException e) {
 			throw new CancelledException();
--- 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
@ -140,8 +140,7 @@ public interface Memory extends AddressSetView {
 	 * @throws LockException if exclusive lock not in place (see haveLock())
 	 * @throws MemoryConflictException if the new block overlaps with a
 	 * previous block
-	 * @throws AddressOverflowException if the start is beyond the
-	 * address space
+	 * @throws AddressOverflowException if block specification exceeds bounds of address space
 	 * @throws CancelledException user cancelled operation
 	 * @throws IllegalArgumentException if invalid block name specified
 	 */
@ -165,8 +164,7 @@ public interface Memory extends AddressSetView {
 	 * @throws LockException if exclusive lock not in place (see haveLock())
 	 * @throws MemoryConflictException if the new block overlaps with a
 	 * previous block
-	 * @throws AddressOverflowException if the start is beyond the
-	 * address space
+	 * @throws AddressOverflowException if block specification exceeds bounds of address space
 	 * @throws IllegalArgumentException if invalid block name specified
 	 * @throws CancelledException user cancelled operation
 	 */
@ -191,7 +189,7 @@ public interface Memory extends AddressSetView {
 	 * @throws LockException if exclusive lock not in place (see haveLock())
 	 * @throws MemoryConflictException if the new block overlaps with a
 	 * previous block
-	 * @throws AddressOverflowException if the start is beyond the address space
+	 * @throws AddressOverflowException if block specification exceeds bounds of address space
 	 * @throws IndexOutOfBoundsException if file bytes range specified by offset and size 
 	 * is out of bounds for the specified fileBytes.
 	 * @throws IllegalArgumentException if invalid block name specified
@ -213,8 +211,7 @@ public interface Memory extends AddressSetView {
 	 * @throws LockException if exclusive lock not in place (see haveLock())
 	 * @throws MemoryConflictException if the new block overlaps with a
 	 * previous block
-	 * @throws AddressOverflowException if the start is beyond the
-	 * address space
+	 * @throws AddressOverflowException if block specification exceeds bounds of address space
 	 * @throws IllegalArgumentException if invalid block name specified
 	 */
 	public MemoryBlock createUninitializedBlock(String name, Address start, long size,
@ -305,8 +302,7 @@ public interface Memory extends AddressSetView {
 	 * @return new block
 	 * @throws LockException if exclusive lock not in place (see haveLock())
 	 * @throws MemoryConflictException if block specification conflicts with an existing block
-	 * @throws AddressOverflowException if the new memory block would extend
-	 * beyond the end of the address space.
+	 * @throws AddressOverflowException if block specification exceeds bounds of address space
 	 * @throws IllegalArgumentException if invalid block name specifiede
 	 */
 	public MemoryBlock createBlock(MemoryBlock block, String name, Address start, long length)
@ -357,8 +353,7 @@ public interface Memory extends AddressSetView {
 	 * @throws MemoryConflictException if move would cause
 	 * blocks to overlap.
 	 * @throws MemoryBlockException if block movement is not permitted
-	 * @throws AddressOverflowException if new start address +
-	 * block.getSize() would cause the Address to wrap around.
+	 * @throws AddressOverflowException if block movement would violate bounds of address space
 	 * @throws NotFoundException if memoryBlock does not exist in
 	 *   this memory.
 	 */
@ -376,8 +371,7 @@ public interface Memory extends AddressSetView {
 	 * @throws NotFoundException thrown if block does not exist
 	 * in memory
 	 * @throws MemoryBlockException memory split not permitted
-	 * @throws AddressOutOfBoundsException thrown if address is
-	 * not in the block
+	 * @throws AddressOutOfBoundsException thrown if address is not in the block
 	 */
 	public void split(MemoryBlock block, Address addr)
 			throws MemoryBlockException, LockException, NotFoundException;
@ -790,7 +784,8 @@ public interface Memory extends AddressSetView {
 	 * @param offset the offset into the file for the first byte in the input stream.
 	 * @param size the number of bytes to store from the input stream.
 	 * @param is the input stream that will supply the bytes to store in the program.
-	 * @param monitor 
+	 * Caller is responsible for closing input stream upon return.
+	 * @param monitor task monitor
 	 * @return a FileBytes that was created to access the bytes.
 	 * @throws IOException if there was an IOException saving the bytes to the program database.
 	 * @throws CancelledException if the user cancelled this operation. Note: the database will