From c4088db81c3562b2ab6ac6a908028bc0d1c8f5b9 Mon Sep 17 00:00:00 2001 From: Scott Percival Date: Sat, 7 Jan 2023 18:08:46 +0800 Subject: [PATCH 1/2] Add OmfRecord support for CEXTDEF Make OmfLoader add dummy functions at entrypoints Fix conditional read when parsing FIXUP records Fix typo in function name --- .../format/omf/OmfComdatExternalSymbol.java | 66 +++++++++++++++++++ .../util/bin/format/omf/OmfFileHeader.java | 4 ++ .../util/bin/format/omf/OmfFixupRecord.java | 8 +-- .../util/bin/format/omf/OmfLibraryRecord.java | 4 +- .../app/util/bin/format/omf/OmfRecord.java | 3 +- .../ghidra/app/util/opinion/OmfLoader.java | 25 ++++++- .../omf/OmfArchiveFileSystemFactory.java | 2 +- 7 files changed, 100 insertions(+), 12 deletions(-) create mode 100644 Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfComdatExternalSymbol.java diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfComdatExternalSymbol.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfComdatExternalSymbol.java new file mode 100644 index 0000000000..1dd27bc5dc --- /dev/null +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfComdatExternalSymbol.java @@ -0,0 +1,66 @@ +/* ### + * IP: GHIDRA + * + * 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.omf; + +import ghidra.app.util.bin.BinaryReader; + +import java.io.IOException; +import java.util.ArrayList; + +public class OmfComdatExternalSymbol extends OmfExternalSymbol { + public static class ExternalLookup { + public int nameIndex; + public int type; + + public ExternalLookup(int ni, int t) { + this.nameIndex = ni; + this.type = t; + } + + } + + protected ArrayList externalLookups; + protected OmfSymbol[] symbol; + + public OmfComdatExternalSymbol(BinaryReader reader) throws IOException { + super(false); + readRecordHeader(reader); + long max = reader.getPointerIndex() + getRecordLength() - 1; + this.externalLookups = new ArrayList(); + + while(reader.getPointerIndex() < max) { + int nameIndex = OmfRecord.readIndex(reader); + int type = OmfRecord.readIndex(reader); + this.externalLookups.add(new ExternalLookup(nameIndex, type)); + } + + readCheckSumByte(reader); + } + + public void loadNames(ArrayList namelist) { + ArrayList symbollist = new ArrayList(); + for (ExternalLookup ext : this.externalLookups) { + String name = namelist.get(ext.nameIndex-1); + symbollist.add(new OmfSymbol(name, ext.type, 0, 0, 0)); + } + this.symbol = new OmfSymbol[symbollist.size()]; + symbollist.toArray(this.symbol); + } + + public OmfSymbol[] getSymbols() { + return symbol; + } +} diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfFileHeader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfFileHeader.java index 9b8aae7bda..b3743eb03d 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfFileHeader.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfFileHeader.java @@ -332,6 +332,10 @@ public class OmfFileHeader extends OmfRecord { header.evaluateComdef(comdef); header.externsymbols.add((OmfExternalSymbol) record); } + else if (record instanceof OmfComdatExternalSymbol comdat) { + ((OmfComdatExternalSymbol)record).loadNames(header.nameList); + header.externsymbols.add((OmfExternalSymbol)record); + } else if (record instanceof OmfExternalSymbol external) { header.externsymbols.add(external); } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfFixupRecord.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfFixupRecord.java index 4ae4bd0560..63ab06eaf4 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfFixupRecord.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfFixupRecord.java @@ -147,12 +147,10 @@ public class OmfFixupRecord extends OmfRecord { ThreadSubrecord thread = new ThreadSubrecord(); thread.type = reader.readNextByte(); int method = thread.getMethod(); - if (method < 4) { - thread.index = OmfRecord.readInt1Or2(reader, hasBigFields); - } - else { + if ((method >= 4) && thread.isFrameThread()) thread.index = -1; - } + else + thread.index = OmfRecord.readInt1Or2(reader, hasBigFields); return thread; } } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfLibraryRecord.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfLibraryRecord.java index 3988470e2a..f92cf6949b 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfLibraryRecord.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfLibraryRecord.java @@ -52,8 +52,8 @@ public class OmfLibraryRecord extends OmfRecord { public ArrayList getMemberHeaders() { return members; } - - public static boolean checkMagicNumer(BinaryReader reader) throws IOException { + + public static boolean checkMagicNumber(BinaryReader reader) throws IOException { byte type = reader.readNextByte(); if (type != (byte) 0xF0) { return false; diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfRecord.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfRecord.java index 2604fc88c1..cf7122ba1f 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfRecord.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfRecord.java @@ -173,6 +173,8 @@ public abstract class OmfRecord { yield new OmfSymbolRecord(reader, true); case LCOMDEF: yield new OmfComdefRecord(reader, true); + case CEXTDEF: + yield new OmfComdatExternalSymbol(reader); case RHEADR: case REGINT: case REDATA: @@ -192,7 +194,6 @@ public abstract class OmfRecord { yield new OmfObsoleteRecord(reader); case LOCSYM: case TYPDEF: - case CEXTDEF: case COMDAT: case LINSYM: case ALIAS: diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/OmfLoader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/OmfLoader.java index 1be2ff7bc4..a8b2cbed50 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/OmfLoader.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/OmfLoader.java @@ -25,8 +25,11 @@ import ghidra.app.util.bin.ByteProvider; import ghidra.app.util.bin.format.omf.*; import ghidra.app.util.bin.format.omf.OmfFixupRecord.Subrecord; import ghidra.app.util.importer.MessageLog; +import ghidra.program.database.function.OverlappingFunctionException; import ghidra.program.model.address.Address; +import ghidra.program.model.address.AddressOutOfBoundsException; import ghidra.program.model.address.AddressOverflowException; +import ghidra.program.model.address.AddressSet; import ghidra.program.model.data.DataType; import ghidra.program.model.data.Undefined; import ghidra.program.model.lang.Language; @@ -390,12 +393,14 @@ public class OmfLoader extends AbstractProgramWrapperLoader { break; } Address addrBase = null; + boolean tagFunction = false; if (symbolrec.getSegmentIndex() != 0) { // TODO: What does it mean if both the segment and group index are non-zero? // Is the segment index group relative? // For now we assume if a segment index is present, we don't need the group index OmfSegmentHeader baseSegment = segments.get(symbolrec.getSegmentIndex() - 1); addrBase = baseSegment.getAddress(language); + tagFunction = baseSegment.isCode(); } else if (symbolrec.getGroupIndex() != 0) { OmfGroupRecord baseGroup = groups.get(symbolrec.getGroupIndex() - 1); @@ -409,10 +414,24 @@ public class OmfLoader extends AbstractProgramWrapperLoader { int numSymbols = symbolrec.numSymbols(); for (int i = 0; i < numSymbols; ++i) { OmfSymbol symbol = symbolrec.getSymbol(i); - Address address = addrBase.add(symbol.getOffset()); - symbol.setAddress(address); + try { + Address address = addrBase.add(symbol.getOffset()); + symbol.setAddress(address); - createSymbol(symbol, address, symbolTable, log); + createSymbol(symbol, address, symbolTable, log); + if (tagFunction) { + // Create a dummy function so that EntryPointAnalyzer will disassemble it + try { + program.getFunctionManager().createFunction(symbol.getName(), address, new AddressSet(address), SourceType.IMPORTED); + } catch (OverlappingFunctionException e) { + log.appendMsg("Function already exists at address " + address.toString() + ": " + e.toString()); + } catch (InvalidInputException e) { + log.appendMsg("Unable to create function with invalid name " + symbol.getName() + ": " + e.toString()); + } + } + } catch (AddressOutOfBoundsException e) { + log.appendMsg("Unable to create symbol " + symbol.getName() + ": " + e.toString()); + } } } } diff --git a/Ghidra/Features/FileFormats/src/main/java/ghidra/file/formats/omf/OmfArchiveFileSystemFactory.java b/Ghidra/Features/FileFormats/src/main/java/ghidra/file/formats/omf/OmfArchiveFileSystemFactory.java index 4e2e6d4d05..da3e5840a7 100644 --- a/Ghidra/Features/FileFormats/src/main/java/ghidra/file/formats/omf/OmfArchiveFileSystemFactory.java +++ b/Ghidra/Features/FileFormats/src/main/java/ghidra/file/formats/omf/OmfArchiveFileSystemFactory.java @@ -52,7 +52,7 @@ public class OmfArchiveFileSystemFactory implements try { BinaryReader reader = OmfFileHeader.createReader(byteProvider); - return OmfLibraryRecord.checkMagicNumer(reader); + return OmfLibraryRecord.checkMagicNumber(reader); } catch (IOException e) { return false; From 2a84e11dec5058b0def4ef32d90ad84ad46f6cf7 Mon Sep 17 00:00:00 2001 From: Ryan Kurtz Date: Tue, 21 Feb 2023 07:15:32 -0500 Subject: [PATCH 2/2] GP-3117: Code cleanup --- .../format/omf/OmfComdatExternalSymbol.java | 43 ++++++------------- .../util/bin/format/omf/OmfComdefRecord.java | 9 +--- .../bin/format/omf/OmfExternalSymbol.java | 16 +++---- .../util/bin/format/omf/OmfFileHeader.java | 7 ++- .../util/bin/format/omf/OmfFixupRecord.java | 6 ++- .../util/bin/format/omf/OmfLibraryRecord.java | 2 +- .../ghidra/app/util/opinion/OmfLoader.java | 30 +++++++------ 7 files changed, 47 insertions(+), 66 deletions(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfComdatExternalSymbol.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfComdatExternalSymbol.java index 1dd27bc5dc..9da0cdff2d 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfComdatExternalSymbol.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfComdatExternalSymbol.java @@ -15,52 +15,35 @@ */ package ghidra.app.util.bin.format.omf; -import ghidra.app.util.bin.BinaryReader; - import java.io.IOException; import java.util.ArrayList; +import java.util.List; + +import ghidra.app.util.bin.BinaryReader; public class OmfComdatExternalSymbol extends OmfExternalSymbol { - public static class ExternalLookup { - public int nameIndex; - public int type; - - public ExternalLookup(int ni, int t) { - this.nameIndex = ni; - this.type = t; - } - - } - protected ArrayList externalLookups; - protected OmfSymbol[] symbol; + public record ExternalLookup(int nameIndex, int type) {} + protected List externalLookups = new ArrayList<>(); public OmfComdatExternalSymbol(BinaryReader reader) throws IOException { super(false); readRecordHeader(reader); + long max = reader.getPointerIndex() + getRecordLength() - 1; - this.externalLookups = new ArrayList(); - - while(reader.getPointerIndex() < max) { + while (reader.getPointerIndex() < max) { int nameIndex = OmfRecord.readIndex(reader); int type = OmfRecord.readIndex(reader); - this.externalLookups.add(new ExternalLookup(nameIndex, type)); + externalLookups.add(new ExternalLookup(nameIndex, type)); } - + readCheckSumByte(reader); } - public void loadNames(ArrayList namelist) { - ArrayList symbollist = new ArrayList(); - for (ExternalLookup ext : this.externalLookups) { - String name = namelist.get(ext.nameIndex-1); - symbollist.add(new OmfSymbol(name, ext.type, 0, 0, 0)); + public void loadNames(List nameList) { + for (ExternalLookup ext : externalLookups) { + String name = nameList.get(ext.nameIndex - 1); + symbols.add(new OmfSymbol(name, ext.type, 0, 0, 0)); } - this.symbol = new OmfSymbol[symbollist.size()]; - symbollist.toArray(this.symbol); - } - - public OmfSymbol[] getSymbols() { - return symbol; } } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfComdefRecord.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfComdefRecord.java index 4f681d3189..af7beea63d 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfComdefRecord.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfComdefRecord.java @@ -16,7 +16,6 @@ package ghidra.app.util.bin.format.omf; import java.io.IOException; -import java.util.ArrayList; import ghidra.app.util.bin.BinaryReader; @@ -25,9 +24,8 @@ public class OmfComdefRecord extends OmfExternalSymbol { public OmfComdefRecord(BinaryReader reader, boolean isStatic) throws IOException, OmfException { super(isStatic); readRecordHeader(reader); - long max = reader.getPointerIndex() + getRecordLength() - 1; - ArrayList symbollist = new ArrayList(); + long max = reader.getPointerIndex() + getRecordLength() - 1; while (reader.getPointerIndex() < max) { String name = OmfRecord.readString(reader); int typeIndex = OmfRecord.readIndex(reader); @@ -42,12 +40,9 @@ public class OmfComdefRecord extends OmfExternalSymbol { // Values 1 thru 5f plus 61, read the byte length byteLength = readCommunalLength(reader); } - OmfSymbol sym = new OmfSymbol(name, typeIndex, 0, dataType, byteLength); - symbollist.add(sym); + symbols.add(new OmfSymbol(name, typeIndex, 0, dataType, byteLength)); } readCheckSumByte(reader); - symbol = new OmfSymbol[symbollist.size()]; - symbollist.toArray(symbol); } private static int readCommunalLength(BinaryReader reader) throws OmfException, IOException { diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfExternalSymbol.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfExternalSymbol.java index b7b61db970..50b6e4cb57 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfExternalSymbol.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfExternalSymbol.java @@ -17,12 +17,14 @@ package ghidra.app.util.bin.format.omf; import java.io.IOException; import java.util.ArrayList; +import java.util.List; import ghidra.app.util.bin.BinaryReader; public class OmfExternalSymbol extends OmfRecord { + private boolean isStatic; - protected OmfSymbol[] symbol; + protected List symbols = new ArrayList<>(); protected OmfExternalSymbol(boolean isStatic) { this.isStatic = isStatic; @@ -31,23 +33,19 @@ public class OmfExternalSymbol extends OmfRecord { public OmfExternalSymbol(BinaryReader reader, boolean isStatic) throws IOException { this.isStatic = isStatic; readRecordHeader(reader); - long max = reader.getPointerIndex() + getRecordLength() - 1; - ArrayList symbollist = new ArrayList(); + long max = reader.getPointerIndex() + getRecordLength() - 1; while (reader.getPointerIndex() < max) { String name = OmfRecord.readString(reader); int type = OmfRecord.readIndex(reader); - OmfSymbol subrec = new OmfSymbol(name, type, 0, 0, 0); - symbollist.add(subrec); + symbols.add(new OmfSymbol(name, type, 0, 0, 0)); } readCheckSumByte(reader); - symbol = new OmfSymbol[symbollist.size()]; - symbollist.toArray(symbol); } - public OmfSymbol[] getSymbols() { - return symbol; + public List getSymbols() { + return symbols; } public boolean isStatic() { diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfFileHeader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfFileHeader.java index b3743eb03d..b2154f0ae6 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfFileHeader.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfFileHeader.java @@ -235,8 +235,7 @@ public class OmfFileHeader extends OmfRecord { } private void evaluateComdef(OmfComdefRecord comdef) { - OmfSymbol[] coms = comdef.getSymbols(); - for (OmfSymbol sym : coms) { + for (OmfSymbol sym : comdef.getSymbols()) { int dt = sym.getDataType(); if (dt > 0 && dt < 0x60) { // A special borland segment symbol int count = (extraSeg == null) ? 1 : extraSeg.size() + 1; @@ -333,8 +332,8 @@ public class OmfFileHeader extends OmfRecord { header.externsymbols.add((OmfExternalSymbol) record); } else if (record instanceof OmfComdatExternalSymbol comdat) { - ((OmfComdatExternalSymbol)record).loadNames(header.nameList); - header.externsymbols.add((OmfExternalSymbol)record); + comdat.loadNames(header.nameList); + header.externsymbols.add(comdat); } else if (record instanceof OmfExternalSymbol external) { header.externsymbols.add(external); diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfFixupRecord.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfFixupRecord.java index 63ab06eaf4..6bf157cdb7 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfFixupRecord.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfFixupRecord.java @@ -147,10 +147,12 @@ public class OmfFixupRecord extends OmfRecord { ThreadSubrecord thread = new ThreadSubrecord(); thread.type = reader.readNextByte(); int method = thread.getMethod(); - if ((method >= 4) && thread.isFrameThread()) + if (method >= 4 && thread.isFrameThread()) { thread.index = -1; - else + } + else { thread.index = OmfRecord.readInt1Or2(reader, hasBigFields); + } return thread; } } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfLibraryRecord.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfLibraryRecord.java index f92cf6949b..2b30114adf 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfLibraryRecord.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfLibraryRecord.java @@ -52,7 +52,7 @@ public class OmfLibraryRecord extends OmfRecord { public ArrayList getMemberHeaders() { return members; } - + public static boolean checkMagicNumber(BinaryReader reader) throws IOException { byte type = reader.readNextByte(); if (type != (byte) 0xF0) { diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/OmfLoader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/OmfLoader.java index a8b2cbed50..596b9efaa5 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/OmfLoader.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/OmfLoader.java @@ -26,10 +26,7 @@ import ghidra.app.util.bin.format.omf.*; import ghidra.app.util.bin.format.omf.OmfFixupRecord.Subrecord; import ghidra.app.util.importer.MessageLog; import ghidra.program.database.function.OverlappingFunctionException; -import ghidra.program.model.address.Address; -import ghidra.program.model.address.AddressOutOfBoundsException; -import ghidra.program.model.address.AddressOverflowException; -import ghidra.program.model.address.AddressSet; +import ghidra.program.model.address.*; import ghidra.program.model.data.DataType; import ghidra.program.model.data.Undefined; import ghidra.program.model.lang.Language; @@ -422,15 +419,23 @@ public class OmfLoader extends AbstractProgramWrapperLoader { if (tagFunction) { // Create a dummy function so that EntryPointAnalyzer will disassemble it try { - program.getFunctionManager().createFunction(symbol.getName(), address, new AddressSet(address), SourceType.IMPORTED); - } catch (OverlappingFunctionException e) { - log.appendMsg("Function already exists at address " + address.toString() + ": " + e.toString()); - } catch (InvalidInputException e) { - log.appendMsg("Unable to create function with invalid name " + symbol.getName() + ": " + e.toString()); + program.getFunctionManager() + .createFunction(symbol.getName(), address, + new AddressSet(address), SourceType.IMPORTED); + } + catch (OverlappingFunctionException e) { + log.appendMsg("Function already exists at address " + address + ": " + + e.getMessage()); + } + catch (InvalidInputException e) { + log.appendMsg("Unable to create function with invalid name " + + symbol.getName() + ": " + e.getMessage()); } } - } catch (AddressOutOfBoundsException e) { - log.appendMsg("Unable to create symbol " + symbol.getName() + ": " + e.toString()); + } + catch (AddressOutOfBoundsException e) { + log.appendMsg( + "Unable to create symbol " + symbol.getName() + ": " + e.getMessage()); } } } @@ -496,9 +501,8 @@ public class OmfLoader extends AbstractProgramWrapperLoader { monitor.setMessage("Creating External Symbols"); for (OmfExternalSymbol symbolrec : symbolrecs) { - OmfSymbol[] symbols = symbolrec.getSymbols(); // TODO: Check instanceof OmfComdefRecord - for (OmfSymbol symbol : symbols) { + for (OmfSymbol symbol : symbolrec.getSymbols()) { if (monitor.isCancelled()) { break; }