GP-0: Tidying up Mach-O things

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/BinaryReader.java

@@ -597,6 +597,7 @@ public class BinaryReader {
 	 * 
 	 * @param length number of bytes to read
 	 * @return the US-ASCII string at the current index
+	 * @throws IOException if an IO-related error occurred
 	 */
 	public String readNextAsciiString(int length) throws IOException {
 		return readNextString(length, StandardCharsets.US_ASCII, 1);
@@ -696,6 +697,7 @@ public class BinaryReader {
 	 * Reads a byte array of <code>nElements</code>
 	 * starting at the current index and then increments the current
 	 * index by <code>SIZEOF_BYTE * nElements</code>.
+	 * @param nElements number of elements to read
 	 * @return the byte array starting at the current index
 	 * @exception IOException if an I/O error occurs
 	 */
@@ -709,6 +711,7 @@ public class BinaryReader {
 	 * Reads a short array of <code>nElements</code>
 	 * starting at the current index and then increments the current
 	 * index by <code>SIZEOF_SHORT * nElements</code>.
+	 * @param nElements number of elements to read
 	 * @return the short array starting at the current index
 	 * @exception IOException if an I/O error occurs
 	 */

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/macho/SectionAttributes.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.
@@ -61,6 +60,7 @@ public final class SectionAttributes {
 					}
 				}
 				catch (Exception e) {
+					// do nothing
 				}
 			}
 		}

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/macho/SectionTypes.java

@@ -99,6 +99,7 @@ public final class SectionTypes {
 					}
 				}
 				catch (Exception e) {
+					// do nothing
 				}
 			}
 		}

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/macho/commands/DynamicLibraryModule.java

@@ -99,17 +99,22 @@ public class DynamicLibraryModule implements StructConverter {
 	public int getExternalRelocationCount() {
 		return nextrel;
 	}
+	
 	/**
-	 * low 16 bits are the index into the init section, 
+	 * Low 16 bits are the index into the init section, high 16 bits are the index into the term 
-	 * high 16 bits are the index into the term section
+	 * section
+	 * 
+	 * @return The init term index
 	 */
 	public int getInitTermIndex() {
 		return iinit_iterm;
 	}
+	
 	/**
-	 * low 16 bits are the number of init section entries, 
+	 * Low 16 bits are the number of init section entries, high 16 bits are the number of term 
-	 * high 16 bits are the number of term section entries
+	 * section entries
-	 * @return
+	 * 
+	 * @return The init term count
 	 */
 	public int getInitTermCount() {
 		return ninit_nterm;
@@ -121,6 +126,7 @@ public class DynamicLibraryModule implements StructConverter {
 		return objc_module_info_addr;
 	}
 
+	@Override
 	public DataType toDataType() throws DuplicateNameException, IOException {
 	    StructureDataType struct = new StructureDataType("dylib_module", 0);
 	    struct.add(DWORD, "module_name", "the module name (index into string table)");

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/macho/commands/LoadCommand.java

@@ -140,7 +140,7 @@ public abstract class LoadCommand implements StructConverter {
 			return;
 		}
 		String comment = getContextualName(source, additionalDescription);
-		program.getListing().setComment(address, CodeUnit.PLATE_COMMENT, comment);
+		program.getListing().setComment(address, CommentType.PLATE, comment);
 	}
 
 	/**

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/macho/commands/ObsoleteCommand.java

@@ -29,6 +29,7 @@ public abstract class ObsoleteCommand extends LoadCommand {
 		throw new ObsoleteException();
 	}
 
+	@Override
 	public DataType toDataType() throws DuplicateNameException, IOException {
 		StructureDataType struct = new StructureDataType(getCommandName(), 0);
 		struct.add(DWORD, "cmd", null);

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/macho/commands/Util.java

@@ -1,21 +0,0 @@
-/* ###
- * 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.
- * 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.app.util.bin.format.macho.commands;
-
-public final class Util {
-
-}

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/macho/commands/chained/DyldChainedStartsInSegment.java

@@ -75,7 +75,7 @@ public class DyldChainedStartsInSegment implements StructConverter {
 	public void markup(Program program, Address address, MachHeader header, TaskMonitor monitor,
 			MessageLog log) throws CancelledException {
 		try {
-
+			// TODO?
 		}
 		catch (Exception e) {
 			log.appendMsg(DyldChainedStartsInSegment.class.getSimpleName(),

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/macho/commands/codesignature/CodeSignatureCodeDirectory.java

@@ -23,7 +23,7 @@ import ghidra.app.util.bin.format.macho.MachHeader;
 import ghidra.app.util.importer.MessageLog;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.data.*;
-import ghidra.program.model.listing.CodeUnit;
+import ghidra.program.model.listing.CommentType;
 import ghidra.program.model.listing.Program;
 import ghidra.util.exception.CancelledException;
 import ghidra.util.exception.DuplicateNameException;
@@ -123,15 +123,14 @@ public class CodeSignatureCodeDirectory extends CodeSignatureGenericBlob {
 				DataUtilities.createData(program, identAddr, STRING, -1,
 					DataUtilities.ClearDataMode.CHECK_FOR_SPACE);
 				program.getListing()
-						.setComment(identAddr, CodeUnit.PRE_COMMENT, "CS_CodeDirectory identifer");
+						.setComment(identAddr, CommentType.PRE, "CS_CodeDirectory identifer");
 			}
 			if (teamOffset != 0) {
 				Address teamAddr = addr.add(teamOffset);
 				DataUtilities.createData(program, teamAddr, STRING, -1,
 					DataUtilities.ClearDataMode.CHECK_FOR_SPACE);
 				program.getListing()
-						.setComment(teamAddr, CodeUnit.PRE_COMMENT,
+						.setComment(teamAddr, CommentType.PRE, "CS_CodeDirectory team identifier");
-							"CS_CodeDirectory team identifier");
 			}
 			if (hashOffset != 0 && hashSize != 0) {
 				Address hashAddr = addr.add(hashOffset);
@@ -140,7 +139,7 @@ public class CodeSignatureCodeDirectory extends CodeSignatureGenericBlob {
 				DataUtilities.createData(program, hashAddr, hasheArrayArrayDt, -1,
 					DataUtilities.ClearDataMode.CHECK_FOR_SPACE);
 				program.getListing()
-						.setComment(hashAddr, CodeUnit.PRE_COMMENT, "CS_CodeDirectory hashes");
+						.setComment(hashAddr, CommentType.PRE, "CS_CodeDirectory hashes");
 			}
 		}
 		catch (Exception e) {

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/macho/commands/codesignature/CodeSignatureGenericBlob.java

@@ -24,7 +24,7 @@ import ghidra.app.util.bin.format.macho.MachHeader;
 import ghidra.app.util.importer.MessageLog;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.data.*;
-import ghidra.program.model.listing.CodeUnit;
+import ghidra.program.model.listing.CommentType;
 import ghidra.program.model.listing.Program;
 import ghidra.util.exception.CancelledException;
 import ghidra.util.exception.DuplicateNameException;
@@ -88,7 +88,7 @@ public class CodeSignatureGenericBlob implements StructConverter {
 			Address hashAddr = address.add(toDataType().getLength());
 			DataUtilities.createData(program, hashAddr, dt, -1,
 				DataUtilities.ClearDataMode.CHECK_FOR_SPACE);
-			program.getListing().setComment(hashAddr, CodeUnit.PRE_COMMENT, "CS_GenericBlob hash");
+			program.getListing().setComment(hashAddr, CommentType.PRE, "CS_GenericBlob hash");
 		}
 		catch (Exception e) {
 			log.appendMsg(CodeSignatureGenericBlob.class.getSimpleName(),

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/macho/commands/dyld/AbstractClassicProcessor.java

@@ -68,7 +68,7 @@ public abstract class AbstractClassicProcessor {
 			return;
 		}
 
-		listing.setComment(symbol.getAddress(), CodeUnit.PLATE_COMMENT, fromDylib);
+		listing.setComment(symbol.getAddress(), CommentType.PLATE, fromDylib);
 
 		long offset = symbol.getAddress().getOffset();
 
@@ -156,8 +156,9 @@ public abstract class AbstractClassicProcessor {
 	}
 
 	/**
-	 * Return the Symbol for the specified NList.
+	 * {@return the Symbol for the specified NList. Looks in the global namespace first.}
-	 * Looks in the global namespace first.
+	 * 
+	 * @param nList The NList
 	 */
 	protected Symbol getSymbol(NList nList) {
 		SymbolTable symbolTable = program.getSymbolTable();
@@ -204,10 +205,8 @@ public abstract class AbstractClassicProcessor {
 	}
 
 	/**
-	 * Returns the relocation base.
+	 * {@return the relocation base. If the program is 64-bit (x86 or PowerPC), then return the VM 
-	 * If the program is 64-bit (x86 or PowerPC), then
+	 * address of the first segment with W bit. Otherwise, just return first segment VM address.}
-	 * return the VM address of the first segment with W bit.
-	 * Otherwise, just return first segment VM address.
 	 */
 	protected long getRelocationBase() {
 		List<SegmentCommand> segments = header.getLoadCommands(SegmentCommand.class);

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/macho/dyld/DyldCacheAccelerateInfo.java

@@ -314,7 +314,7 @@ public class DyldCacheAccelerateInfo implements StructConverter {
 			DataType dt = new ArrayDataType(WORD, reExportCount, WORD.getLength());
 			DataUtilities.createData(program, addr, dt, -1,
 				DataUtilities.ClearDataMode.CHECK_FOR_SPACE);
-			program.getListing().setComment(addr, CodeUnit.EOL_COMMENT, "re-exports");
+			program.getListing().setComment(addr, CommentType.EOL, "re-exports");
 			monitor.incrementProgress(1);
 		}
 		catch (CodeUnitInsertionException e) {
@@ -332,7 +332,7 @@ public class DyldCacheAccelerateInfo implements StructConverter {
 			DataType dt = new ArrayDataType(WORD, depListCount, WORD.getLength());
 			DataUtilities.createData(program, addr, dt, -1,
 				DataUtilities.ClearDataMode.CHECK_FOR_SPACE);
-			program.getListing().setComment(addr, CodeUnit.EOL_COMMENT, "dependencies");
+			program.getListing().setComment(addr, CommentType.EOL, "dependencies");
 			monitor.incrementProgress(1);
 		}
 		catch (CodeUnitInsertionException e) {

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/macho/dyld/DyldCacheHeader.java

@@ -1430,7 +1430,7 @@ public class DyldCacheHeader implements StructConverter {
 			for (DyldCacheImageInfo imageInfo : imageInfoList) {
 				Data d = DataUtilities.createData(program, addr, imageInfo.toDataType(), -1,
 					DataUtilities.ClearDataMode.CHECK_FOR_SPACE);
-				program.getListing().setComment(addr, CodeUnit.EOL_COMMENT, imageInfo.getPath());
+				program.getListing().setComment(addr, CommentType.EOL, imageInfo.getPath());
 				addr = addr.add(d.getLength());
 				monitor.checkCancelled();
 				monitor.incrementProgress(1);
@@ -1450,7 +1450,7 @@ public class DyldCacheHeader implements StructConverter {
 			String size = "0x" + Long.toHexString(codeSignatureSize);
 			program.getListing()
 					.setComment(fileOffsetToAddr(codeSignatureOffset, program, space),
-						CodeUnit.PLATE_COMMENT, "Code Signature (" + size + " bytes)");
+						CommentType.PLATE, "Code Signature (" + size + " bytes)");
 			monitor.incrementProgress(1);
 		}
 		catch (IllegalArgumentException e) {
@@ -1550,8 +1550,7 @@ public class DyldCacheHeader implements StructConverter {
 			for (DyldCacheImageTextInfo imageTextInfo : imageTextInfoList) {
 				Data d = DataUtilities.createData(program, addr, imageTextInfo.toDataType(), -1,
 					DataUtilities.ClearDataMode.CHECK_FOR_SPACE);
-				program.getListing()
+				program.getListing().setComment(addr, CommentType.EOL, imageTextInfo.getPath());
-						.setComment(addr, CodeUnit.EOL_COMMENT, imageTextInfo.getPath());
 				addr = addr.add(d.getLength());
 				monitor.checkCancelled();
 				monitor.incrementProgress(1);

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/macho/threadcommand/FloatStateX86_32.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.
@@ -17,5 +16,5 @@
 package ghidra.app.util.bin.format.macho.threadcommand;
 
 public class FloatStateX86_32 {
-
+	// Empty
 }

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/macho/threadcommand/ThreadStateARM.java

@@ -20,7 +20,6 @@ import java.io.IOException;
 import ghidra.app.util.bin.BinaryReader;
 import ghidra.app.util.bin.format.macho.MachConstants;
 import ghidra.program.model.data.*;
-import ghidra.util.Conv;
 import ghidra.util.exception.DuplicateNameException;
 
 /**
@@ -75,7 +74,7 @@ public class ThreadStateARM extends ThreadState {
 
 	@Override
 	public long getInstructionPointer() {
-		return Conv.intToLong(pc);
+		return Integer.toUnsignedLong(pc);
 	}
 
 	@Override

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/macho/threadcommand/ThreadStateX86_32.java

@@ -20,7 +20,6 @@ import java.io.IOException;
 import ghidra.app.util.bin.BinaryReader;
 import ghidra.app.util.bin.format.macho.MachConstants;
 import ghidra.program.model.data.*;
-import ghidra.util.Conv;
 import ghidra.util.exception.DuplicateNameException;
 
 /**
@@ -67,7 +66,7 @@ public class ThreadStateX86_32 extends ThreadStateX86 {
 
     @Override
     public long getInstructionPointer() {
-		return Conv.intToLong(eip);
+		return Integer.toUnsignedLong(eip);
     }
 
 	@Override

Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/DyldCacheProgramBuilder.java

@@ -482,7 +482,7 @@ public class DyldCacheProgramBuilder extends MachoProgramBuilder {
 			DyldCacheProgramBuilder.this.markupHeaders(header, headerAddr);
 
 			if (!name.isEmpty()) {
-				listing.setComment(headerAddr, CodeUnit.PLATE_COMMENT, path);
+				listing.setComment(headerAddr, CommentType.PLATE, path);
 			}
 		}
 

Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/MachoPrelinkProgramBuilder.java

@@ -234,7 +234,7 @@ public class MachoPrelinkProgramBuilder extends MachoProgramBuilder {
 			MachoPrelinkProgramBuilder.this.markupHeaders(header, headerAddr);
 
 			if (!name.isEmpty()) {
-				listing.setComment(headerAddr, CodeUnit.PLATE_COMMENT, name);
+				listing.setComment(headerAddr, CommentType.PLATE, name);
 			}
 		}
 

Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/MachoProgramBuilder.java

@@ -1463,7 +1463,7 @@ public class MachoProgramBuilder {
 	}
 
 	private void addLibrary(String library) {
-		library = library.replaceAll(" ", "_");
+		library = SymbolUtilities.replaceInvalidChars(library, true);
 		try {
 			program.getExternalManager().addExternalLibraryName(library, SourceType.IMPORTED);
 		}
@@ -1867,7 +1867,8 @@ public class MachoProgramBuilder {
 			throw new Exception(
 				"Library ordinal '%d' outside of expected range".formatted(libraryOrdinal));
 		}
-		String libraryName = libraryPaths.get(libraryIndex).replaceAll(" ", "_");
+		String libraryName =
+			SymbolUtilities.replaceInvalidChars(libraryPaths.get(libraryIndex), true);
 		Library library = extManager.getExternalLibrary(libraryName);
 		if (library == null) {
 			throw new Exception(
@@ -1880,7 +1881,7 @@ public class MachoProgramBuilder {
 				loc.setName(library, symbol, SourceType.IMPORTED);
 			}
 			catch (InvalidInputException e) {
-				throw new Exception("Symbol name contains illegal characters");
+				throw new Exception(e.getMessage());
 			}
 		}
 	}