GP-4722: More OMF header markup

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/omf/OmfComdatExternalSymbol.java

@@ -21,12 +21,17 @@ import java.util.List;
 
 import ghidra.app.util.bin.BinaryReader;
 import ghidra.app.util.bin.format.omf.*;
+import ghidra.program.model.data.*;
+import ghidra.util.exception.DuplicateNameException;
 
 public class OmfComdatExternalSymbol extends OmfExternalSymbol {
 	
 	public record ExternalLookup(int nameIndex, int type) {}
 	protected List<ExternalLookup> externalLookups = new ArrayList<>();
 
+	private record Reference(OmfIndex nameIndex, OmfIndex typeIndex) {}
+	private List<Reference> refs = new ArrayList<>();
+
 	public OmfComdatExternalSymbol(BinaryReader reader) throws IOException {
 		super(reader, false);
 
@@ -37,6 +42,7 @@ public class OmfComdatExternalSymbol extends OmfExternalSymbol {
 		while (dataReader.getPointerIndex() < dataEnd) {
 			OmfIndex nameIndex = OmfUtils.readIndex(dataReader);
 			OmfIndex type = OmfUtils.readIndex(dataReader);
+			refs.add(new Reference(nameIndex, type));
 			externalLookups.add(new ExternalLookup(nameIndex.value(), type.value()));
 		}
 	}
@@ -47,4 +53,19 @@ public class OmfComdatExternalSymbol extends OmfExternalSymbol {
 			symbols.add(new OmfSymbol(name, ext.type, 0, 0, 0));
 		}
 	}
+
+	@Override
+	public DataType toDataType() throws DuplicateNameException, IOException {
+		StructureDataType struct = new StructureDataType(OmfRecordTypes.getName(recordType), 0);
+		struct.add(BYTE, "type", null);
+		struct.add(WORD, "length", null);
+		for (Reference ref : refs) {
+			struct.add(ref.nameIndex.toDataType(), "logical_name_index", null);
+			struct.add(ref.typeIndex.toDataType(), "type_index", null);
+		}
+		struct.add(BYTE, "checksum", null);
+
+		struct.setCategoryPath(new CategoryPath(OmfUtils.CATEGORY_PATH));
+		return struct;
+	}
 }

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/omf/OmfComdefRecord.java

@@ -16,12 +16,21 @@
 package ghidra.app.util.bin.format.omf.omf;
 
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
 
 import ghidra.app.util.bin.BinaryReader;
+import ghidra.app.util.bin.StructConverter;
 import ghidra.app.util.bin.format.omf.*;
+import ghidra.program.model.data.*;
+import ghidra.util.exception.DuplicateNameException;
 
 public class OmfComdefRecord extends OmfExternalSymbol {
 
+	private record Reference(OmfString name, OmfIndex typeIndex, OmfCommunalLength communalLength1,
+			OmfCommunalLength communalLength2) {}
+	private List<Reference> refs = new ArrayList<>();
+
 	public OmfComdefRecord(BinaryReader reader, boolean isStatic) throws IOException {
 		super(reader, isStatic);
 	}
@@ -34,38 +43,96 @@ public class OmfComdefRecord extends OmfExternalSymbol {
 			byte dataType = dataReader.readNextByte();
 			int byteLength = 0;
 			if (dataType == 0x61) {		// FAR data, reads numElements and elSize
-				int numElements = readCommunalLength(dataReader);
+				OmfCommunalLength numElements = new OmfCommunalLength(dataReader);
-				int elSize = readCommunalLength(dataReader);
+				OmfCommunalLength elSize = new OmfCommunalLength(dataReader);
-				byteLength = numElements * elSize;
+				byteLength = numElements.value * elSize.value;
+				refs.add(new Reference(name, typeIndex, numElements, elSize));
 			}
 			else {
 				// Values 1 thru 5f plus 61, read the byte length
-				byteLength = readCommunalLength(dataReader);
+				OmfCommunalLength communalLength = new OmfCommunalLength(dataReader);
+				byteLength = communalLength.value;
+				refs.add(new Reference(name, typeIndex, communalLength, null));
 			}
 			symbols.add(new OmfSymbol(name.str(), typeIndex.value(), 0, dataType, byteLength));
 		}
 	}
 
-	private static int readCommunalLength(BinaryReader reader) throws OmfException, IOException {
+	@Override
-		int val = reader.readNextByte() & 0xff;
+	public DataType toDataType() throws DuplicateNameException, IOException {
-		if (val <= 128) {
+		StructureDataType struct = new StructureDataType(OmfRecordTypes.getName(recordType), 0);
-			return val;
+		struct.add(BYTE, "type", null);
+		struct.add(WORD, "length", null);
+		for (Reference ref : refs) {
+			struct.add(ref.name.toDataType(), "name", null);
+			struct.add(ref.typeIndex.toDataType(), "type_index", null);
+			struct.add(BYTE, "data_type", null);
+			struct.add(ref.communalLength1.toDataType(), "communal_length", null);
+			if (ref.communalLength2 != null) {
+				struct.add(ref.communalLength2.toDataType(), "communal_length", null);
 			}
-		if (val == 0x81) {
-			val = reader.readNextShort() & 0xffff;
 		}
-		else if (val == 0x84) {
+		struct.add(BYTE, "checksum", null);
-			val = reader.readNextShort() & 0xffff;
+
-			int hithird = reader.readNextByte() & 0xff;
+		struct.setCategoryPath(new CategoryPath(OmfUtils.CATEGORY_PATH));
-			val += (hithird << 16);
+		return struct;
 	}
-		else if (val == 0x88) {
+
-			val = reader.readNextInt();
+	/**
+	 * A OMF COMDEF "communal length"
+	 */
+	private static class OmfCommunalLength implements StructConverter {
+
+		private int numBytes;
+		private int value;
+
+		public OmfCommunalLength(BinaryReader reader) throws OmfException, IOException {
+			long origIndex = reader.getPointerIndex();
+			int b = reader.readNextUnsignedByte();
+			if (b <= 128) {
+				value = b;
+			}
+			else if (b == 0x81) {
+				value = reader.readNextUnsignedShort();
+			}
+			else if (b == 0x84) {
+				value = reader.readNextUnsignedShort();
+				int hithird = reader.readNextUnsignedByte();
+				value += (hithird << 16);
+			}
+			else if (b == 0x88) {
+				value = reader.readNextInt();
 			}
 			else {
 				throw new OmfException("Illegal communal length encoding");
 			}
-		return val;
+			numBytes = (int)(reader.getPointerIndex() - origIndex);
 		}
 
+		@Override
+		public DataType toDataType() throws DuplicateNameException, IOException {
+			StructureDataType struct =
+				new StructureDataType(OmfCommunalLength.class.getSimpleName(), 0);
+			switch (numBytes) {
+				case 1:
+					struct.add(BYTE, "value", null);
+					break;
+				case 3:
+					struct.add(BYTE, "type", null);
+					struct.add(WORD, "value", null);
+					break;
+				case 4:
+					struct.add(BYTE, "type", null);
+					struct.add(Integer3DataType.dataType, "value", null);
+					break;
+				case 5:
+					struct.add(BYTE, "type", null);
+					struct.add(DWORD, "value", null);
+					break;
+
+			}
+			struct.setCategoryPath(new CategoryPath(OmfUtils.CATEGORY_PATH));
+			return struct;
+		}
+	}
 }

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/omf/OmfExternalSymbol.java

@@ -29,8 +29,7 @@ public class OmfExternalSymbol extends OmfRecord {
 	private boolean isStatic;
 	protected List<OmfSymbol> symbols = new ArrayList<>();
 	
-	private record Reference(OmfString name, OmfIndex type) {}
+	private record Reference(OmfString name, OmfIndex typeIndex) {}
-
 	private List<Reference> refs = new ArrayList<>();
 
 	public OmfExternalSymbol(BinaryReader reader, boolean isStatic) throws IOException {
@@ -42,9 +41,9 @@ public class OmfExternalSymbol extends OmfRecord {
 	public void parseData() throws IOException, OmfException {
 		while (dataReader.getPointerIndex() < dataEnd) {
 			OmfString name = OmfUtils.readString(dataReader);
-			OmfIndex type = OmfUtils.readIndex(dataReader);
+			OmfIndex typeIndex = OmfUtils.readIndex(dataReader);
-			refs.add(new Reference(name, type));
+			refs.add(new Reference(name, typeIndex));
-			symbols.add(new OmfSymbol(name.str(), type.value(), 0, 0, 0));
+			symbols.add(new OmfSymbol(name.str(), typeIndex.value(), 0, 0, 0));
 		}
 	}
 
@@ -62,8 +61,8 @@ public class OmfExternalSymbol extends OmfRecord {
 		struct.add(BYTE, "type", null);
 		struct.add(WORD, "length", null);
 		for (Reference ref : refs) {
-			struct.add(ref.name.toDataType(), "name", null);
+			struct.add(ref.name.toDataType(), "external_name", null);
-			struct.add(ref.type.toDataType(), "type", null);
+			struct.add(ref.typeIndex.toDataType(), "type_index", null);
 		}
 		struct.add(BYTE, "checksum", null);
 

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/omf/OmfFileHeader.java

@@ -324,19 +324,22 @@ public class OmfFileHeader extends OmfRecord {
 	 * @throws IOException for problems reading data
 	 * @throws OmfException for malformed records
 	 */
-	public static OmfFileHeader parse(AbstractOmfRecordFactory factory, TaskMonitor monitor, MessageLog log)
+	public static OmfFileHeader parse(AbstractOmfRecordFactory factory, TaskMonitor monitor,
-			throws IOException, OmfException {
+			MessageLog log) throws IOException, OmfException {
 		OmfRecord record = factory.readNextRecord();
 		if (!(record instanceof OmfFileHeader header)) {
 			throw new OmfException("Object file does not start with proper header");
 		}
+		if (!record.validCheckSum()) {
+			logRecord("Invalid checksum", record, log);
+		}
 		header.records.add(header);
 		OmfData lastDataBlock = null;
 
 		while (true) {
 			record = factory.readNextRecord();
 			if (!record.validCheckSum()) {
-				throw new OmfException("Invalid checksum!");
+				logRecord("Invalid checksum", record, log);
 			}
 			header.records.add(record);
 
@@ -358,7 +361,7 @@ public class OmfFileHeader extends OmfRecord {
 			}
 			else if (record instanceof OmfComdefRecord comdef) {
 				header.evaluateComdef(comdef);
-				header.externsymbols.add((OmfExternalSymbol) record);
+				header.externsymbols.add(comdef);
 			}
 			else if (record instanceof OmfComdatExternalSymbol comdat) {
 				comdat.loadNames(header.nameList);
@@ -494,7 +497,8 @@ public class OmfFileHeader extends OmfRecord {
 	}
 
 	private static void logRecord(String description, OmfRecord record, MessageLog log) {
-		log.appendMsg(description + " (" + record + ")");
+		log.appendMsg("%s (0x%x - %s @ 0x%x)".formatted(description, record.getRecordType(),
+			OmfRecordTypes.getName(record.getRecordType()), record.getRecordOffset()));
 	}
 
 	@Override

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

@@ -68,6 +68,7 @@ public class OmfLoader extends AbstractProgramWrapperLoader {
 			case String s when s.startsWith("CodeGear") -> "codegearcpp";
 			case String s when s.equals("MS C") -> "windows";
 			case String s when s.startsWith("Watcom") -> "watcom";
+			case null -> null;
 			default -> null;
 		};
 	}
@@ -120,9 +121,9 @@ public class OmfLoader extends AbstractProgramWrapperLoader {
 			header.sortSegmentDataBlocks();
 			OmfFileHeader.doLinking(IMAGE_BASE, header.getSegments(), header.getGroups());
 		}
-		catch (OmfException ex) {
+		catch (OmfException e) {
 			if (header == null) {
-				throw new IOException("OMF File header was corrupted");
+				throw new IOException("OMF File header was corrupted. " + e.getMessage());
 			}
 			log.appendMsg("File was corrupted - leaving partial program " + provider.getName());
 		}