OMF module reorganization

src/common.rs

@@ -771,6 +771,18 @@ pub enum RelocationFlags {
         /// Relocation type (the `R_WASM_*` constant).
         r_type: u8,
     },
+    /// OMF relocation metadata.
+    #[cfg(feature = "omf")]
+    Omf {
+        /// The location field describing what bytes are being fixed up.
+        location: crate::omf::FixupLocation,
+        /// Whether the relocation is applied segment-relative (`M = 1`) or self-relative (`M = 0`).
+        mode: crate::omf::FixupMode,
+        /// The frame datum used to establish the base reference for the relocation.
+        frame: crate::omf::FixupFrame,
+        /// The target datum identifying the entity being referenced.
+        target: crate::omf::FixupTarget,
+    },
 }
 
 /// Wrapper to print as `[..]` without a manual `Debug` implementation, rather than dumping an

src/omf.rs

@@ -92,6 +92,94 @@ pub mod record_type {
     pub const VENDEXT: u8 = 0xCE;
 }
 
+/// Check if a byte is a valid OMF record type
+pub fn is_omf_record_type(byte: u8) -> bool {
+    use crate::omf::record_type::*;
+    matches!(
+        byte,
+        THEADR
+            | LHEADR
+            | COMENT
+            | MODEND
+            | MODEND32
+            | EXTDEF
+            | TYPDEF
+            | PUBDEF
+            | PUBDEF32
+            | LINNUM
+            | LINNUM32
+            | LNAMES
+            | SEGDEF
+            | SEGDEF32
+            | GRPDEF
+            | FIXUPP
+            | FIXUPP32
+            | LEDATA
+            | LEDATA32
+            | LIDATA
+            | LIDATA32
+            | COMDEF
+            | BAKPAT
+            | BAKPAT32
+            | LEXTDEF
+            | LEXTDEF32
+            | LPUBDEF
+            | LPUBDEF32
+            | LCOMDEF
+            | CEXTDEF
+            | COMDAT
+            | COMDAT32
+            | LINSYM
+            | LINSYM32
+            | ALIAS
+            | NBKPAT
+            | NBKPAT32
+            | LLNAMES
+            | VERNUM
+            | VENDEXT
+    )
+}
+
+/// The addressing mode for an OMF relocation.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+#[repr(u8)]
+pub enum FixupMode {
+    /// Segment-relative relocation (`M = 1`).
+    SegmentRelative = 0,
+    /// Self-relative relocation (`M = 0`).
+    SelfRelative = 1,
+}
+
+/// Frame datum variants as defined by the OMF specification.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub enum FixupFrame {
+    /// Segment frame datum referencing a 1-based segment index.
+    Segment(u16),
+    /// Group frame datum referencing a 1-based group index.
+    Group(u16),
+    /// External frame datum referencing a 1-based entry in the external-name table.
+    External(u16),
+    /// Explicit frame number datum.
+    FrameNumber(u16),
+    /// Use the location of the fixup as the frame datum.
+    Location,
+    /// Use the target's frame datum.
+    Target,
+}
+
+/// Target datum variants as defined by the OMF specification.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub enum FixupTarget {
+    /// Segment target datum referencing a 1-based segment index.
+    Segment(u16),
+    /// Group target datum referencing a 1-based group index.
+    Group(u16),
+    /// External target datum referencing a 1-based entry in the external-name table.
+    External(u16),
+    /// Explicit frame number datum.
+    FrameNumber(u16),
+}
+
 /// OMF record header
 #[derive(Debug, Clone, Copy)]
 #[repr(C)]
@@ -139,7 +227,7 @@ pub enum SegmentCombination {
 }
 
 /// Fixup location types
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 #[repr(u8)]
 pub enum FixupLocation {
     /// Low-order byte
@@ -161,158 +249,3 @@ pub enum FixupLocation {
     /// 32-bit loader-resolved offset
     LoaderOffset32 = 13,
 }
-
-/// Target method types for fixups
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-#[repr(u8)]
-pub enum TargetMethod {
-    /// Segment index
-    SegmentIndex = 0,
-    /// Group index
-    GroupIndex = 1,
-    /// External index
-    ExternalIndex = 2,
-    /// Frame number (absolute)
-    FrameNumber = 3,
-}
-
-/// Frame method types for fixups
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-#[repr(u8)]
-pub enum FrameMethod {
-    /// Segment index
-    SegmentIndex = 0,
-    /// Group index
-    GroupIndex = 1,
-    /// External index
-    ExternalIndex = 2,
-    /// Frame number (absolute)
-    FrameNumber = 3,
-    /// Location (use fixup location)
-    Location = 4,
-    /// Target (use target's frame)
-    Target = 5,
-}
-
-/// Check if a byte is a valid OMF record type
-pub(crate) fn is_omf_record_type(byte: u8) -> bool {
-    use record_type::*;
-    matches!(
-        byte,
-        THEADR
-            | LHEADR
-            | COMENT
-            | MODEND
-            | MODEND32
-            | EXTDEF
-            | TYPDEF
-            | PUBDEF
-            | PUBDEF32
-            | LINNUM
-            | LINNUM32
-            | LNAMES
-            | SEGDEF
-            | SEGDEF32
-            | GRPDEF
-            | FIXUPP
-            | FIXUPP32
-            | LEDATA
-            | LEDATA32
-            | LIDATA
-            | LIDATA32
-            | COMDEF
-            | BAKPAT
-            | BAKPAT32
-            | LEXTDEF
-            | LEXTDEF32
-            | LPUBDEF
-            | LPUBDEF32
-            | LCOMDEF
-            | CEXTDEF
-            | COMDAT
-            | COMDAT32
-            | LINSYM
-            | LINSYM32
-            | ALIAS
-            | NBKPAT
-            | NBKPAT32
-            | LLNAMES
-            | VERNUM
-            | VENDEXT
-    )
-}
-
-/// Helper to read an OMF index (1 or 2 bytes)
-pub(crate) fn read_index(data: &[u8]) -> Option<(u16, usize)> {
-    if data.is_empty() {
-        return None;
-    }
-
-    let first_byte = data[0];
-    if first_byte & 0x80 == 0 {
-        // 1-byte index
-        Some((first_byte as u16, 1))
-    } else if data.len() >= 2 {
-        // 2-byte index
-        let high = (first_byte & 0x7F) as u16;
-        let low = data[1] as u16;
-        Some((high << 8 | low, 2))
-    } else {
-        None
-    }
-}
-
-/// Helper to read a counted string (length byte followed by string)
-pub(crate) fn read_counted_string(data: &[u8]) -> Option<(&[u8], usize)> {
-    if data.is_empty() {
-        return None;
-    }
-
-    let length = data[0] as usize;
-    if data.len() > length {
-        Some((&data[1..1 + length], 1 + length))
-    } else {
-        None
-    }
-}
-
-/// Read an encoded value (used in LIDATA for repeat counts and block counts)
-/// Returns the value and number of bytes consumed
-pub(crate) fn read_encoded_value(data: &[u8]) -> Option<(u32, usize)> {
-    if data.is_empty() {
-        return None;
-    }
-
-    let first_byte = data[0];
-    if first_byte < 0x80 {
-        // Single byte value (0-127)
-        Some((first_byte as u32, 1))
-    } else if first_byte == 0x81 {
-        // Two byte value: 0x81 followed by 16-bit little-endian value
-        if data.len() >= 3 {
-            let value = u16::from_le_bytes([data[1], data[2]]) as u32;
-            Some((value, 3))
-        } else {
-            None
-        }
-    } else if first_byte == 0x84 {
-        // Three byte value: 0x84 followed by 24-bit little-endian value
-        if data.len() >= 4 {
-            let value = u32::from_le_bytes([data[1], data[2], data[3], 0]);
-            Some((value, 4))
-        } else {
-            None
-        }
-    } else if first_byte == 0x88 {
-        // Four byte value: 0x88 followed by 32-bit little-endian value
-        if data.len() >= 5 {
-            let value = u32::from_le_bytes([data[1], data[2], data[3], data[4]]);
-            Some((value, 5))
-        } else {
-            None
-        }
-    } else {
-        // Unknown encoding
-        None
-    }
-}

src/read/mod.rs

@@ -355,6 +355,8 @@ impl FileKind {
             #[cfg(feature = "omf")]
             [0x80, ..] | [0x82, ..] => {
                 // Check if it's a valid OMF record type
+                // TODO this is tautological, 0x80 and 0x82 are valid OMF record types
+                // how can we check better?
                 if crate::omf::is_omf_record_type(magic[0]) {
                     FileKind::Omf
                 } else {

src/read/omf/comdat.rs

@@ -0,0 +1,128 @@
+use crate::read::{self, Error, Result};
+use crate::{omf, ComdatKind, ObjectComdat, ReadRef, SectionIndex, SymbolIndex};
+
+use super::OmfFile;
+
+/// A COMDAT (communal data) section
+#[derive(Debug, Clone)]
+pub(super) struct OmfComdatData<'data> {
+    /// Symbol name
+    pub(super) name: &'data [u8],
+    /// Segment index where this COMDAT belongs
+    pub(super) segment_index: u16,
+    /// Selection/allocation method
+    pub(super) selection: OmfComdatSelection,
+    /// Alignment
+    #[allow(unused)]
+    pub(super) alignment: omf::SegmentAlignment,
+    /// Data
+    #[allow(unused)]
+    pub(super) data: &'data [u8],
+}
+
+/// COMDAT selection methods
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub(super) enum OmfComdatSelection {
+    /// Explicit: may not be combined, produce error if multiple definitions
+    Explicit = 0,
+    /// Use any: pick any instance
+    UseAny = 1,
+    /// Same size: all instances must be same size
+    SameSize = 2,
+    /// Exact match: all instances must have identical content
+    ExactMatch = 3,
+}
+
+/// A COMDAT section in an OMF file.
+#[derive(Debug)]
+pub struct OmfComdat<'data, 'file, R: ReadRef<'data>> {
+    file: &'file OmfFile<'data, R>,
+    index: usize,
+    _phantom: core::marker::PhantomData<&'data ()>,
+}
+
+impl<'data, 'file, R: ReadRef<'data>> read::private::Sealed for OmfComdat<'data, 'file, R> {}
+
+impl<'data, 'file, R: ReadRef<'data>> ObjectComdat<'data> for OmfComdat<'data, 'file, R> {
+    type SectionIterator = OmfComdatSectionIterator<'data, 'file, R>;
+
+    fn kind(&self) -> ComdatKind {
+        let comdat = &self.file.comdats[self.index];
+        match comdat.selection {
+            OmfComdatSelection::Explicit => ComdatKind::NoDuplicates,
+            OmfComdatSelection::UseAny => ComdatKind::Any,
+            OmfComdatSelection::SameSize => ComdatKind::SameSize,
+            OmfComdatSelection::ExactMatch => ComdatKind::ExactMatch,
+        }
+    }
+
+    fn symbol(&self) -> SymbolIndex {
+        // COMDAT symbols don't have a direct symbol index in OMF
+        SymbolIndex(usize::MAX)
+    }
+
+    fn name_bytes(&self) -> Result<&'data [u8]> {
+        let comdat = &self.file.comdats[self.index];
+        Ok(comdat.name)
+    }
+
+    fn name(&self) -> Result<&'data str> {
+        let comdat = &self.file.comdats[self.index];
+        core::str::from_utf8(comdat.name).map_err(|_| Error("Invalid UTF-8 in COMDAT name"))
+    }
+
+    fn sections(&self) -> Self::SectionIterator {
+        let comdat = &self.file.comdats[self.index];
+        OmfComdatSectionIterator {
+            segment_index: (comdat.segment_index as usize).checked_sub(1),
+            returned: false,
+            _phantom: core::marker::PhantomData,
+        }
+    }
+}
+
+/// An iterator over COMDAT sections.
+#[derive(Debug)]
+pub struct OmfComdatIterator<'data, 'file, R: ReadRef<'data>> {
+    pub(super) file: &'file OmfFile<'data, R>,
+    pub(super) index: usize,
+}
+
+impl<'data, 'file, R: ReadRef<'data>> Iterator for OmfComdatIterator<'data, 'file, R> {
+    type Item = OmfComdat<'data, 'file, R>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        if self.index < self.file.comdats.len() {
+            let comdat = OmfComdat {
+                file: self.file,
+                index: self.index,
+                _phantom: core::marker::PhantomData,
+            };
+            self.index += 1;
+            Some(comdat)
+        } else {
+            None
+        }
+    }
+}
+
+/// An iterator over sections in a COMDAT.
+#[derive(Debug)]
+pub struct OmfComdatSectionIterator<'data, 'file, R: ReadRef<'data>> {
+    segment_index: Option<usize>,
+    returned: bool,
+    _phantom: core::marker::PhantomData<(&'data (), &'file (), R)>,
+}
+
+impl<'data, 'file, R: ReadRef<'data>> Iterator for OmfComdatSectionIterator<'data, 'file, R> {
+    type Item = SectionIndex;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        if !self.returned {
+            self.returned = true;
+            self.segment_index.map(|idx| SectionIndex(idx + 1))
+        } else {
+            None
+        }
+    }
+}

src/read/omf/relocation.rs

@@ -1,16 +1,41 @@
-use crate::{omf, read, Relocation, SectionIndex};
+use crate::read::ReadRef;
+use crate::{
+    omf, Relocation, RelocationEncoding, RelocationFlags, RelocationKind, RelocationTarget,
+    SectionIndex, SymbolIndex,
+};
 
-use super::OmfFile;
+use super::{FrameMethod, OmfFile, TargetMethod};
+
+/// An OMF fixup (relocation entry).
+#[derive(Debug, Clone)]
+pub(super) struct OmfFixup {
+    /// Offset in segment where fixup is applied
+    pub(super) offset: u32,
+    /// Location type (what to patch)
+    pub(super) location: omf::FixupLocation,
+    /// Frame method
+    pub(super) frame_method: FrameMethod,
+    /// Target method
+    pub(super) target_method: TargetMethod,
+    /// Frame index (meaning depends on frame_method)
+    pub(super) frame_index: u16,
+    /// Target index (meaning depends on target_method)
+    pub(super) target_index: u16,
+    /// Target displacement
+    pub(super) target_displacement: u32,
+    /// M-bit: true for segment-relative, false for PC-relative
+    pub(super) is_segment_relative: bool,
+}
 
 /// An iterator over OMF relocations.
 #[derive(Debug)]
-pub struct OmfRelocationIterator<'data, 'file, R: read::ReadRef<'data>> {
+pub struct OmfRelocationIterator<'data, 'file, R: ReadRef<'data>> {
     pub(super) file: &'file OmfFile<'data, R>,
     pub(super) segment_index: usize,
     pub(super) index: usize,
 }
 
-impl<'data, 'file, R: read::ReadRef<'data>> Iterator for OmfRelocationIterator<'data, 'file, R> {
+impl<'data, 'file, R: ReadRef<'data>> Iterator for OmfRelocationIterator<'data, 'file, R> {
     type Item = (u64, Relocation);
 
     fn next(&mut self) -> Option<Self::Item> {
@@ -18,58 +43,109 @@ impl<'data, 'file, R: read::ReadRef<'data>> Iterator for OmfRelocationIterator<'
         let reloc = relocations.get(self.index)?;
         self.index += 1;
 
-        // Convert OMF relocation to generic relocation
-        let (kind, size, addend) = match reloc.location {
-            omf::FixupLocation::LowByte => (read::RelocationKind::Absolute, 8, 0),
-            omf::FixupLocation::HighByte => (read::RelocationKind::Absolute, 8, 0),
+        let (mut kind, size, base_addend) = match reloc.location {
+            omf::FixupLocation::LowByte => (RelocationKind::Absolute, 8, 0),
+            omf::FixupLocation::HighByte => (RelocationKind::Absolute, 8, 0),
             omf::FixupLocation::Offset | omf::FixupLocation::LoaderOffset => {
                 if reloc.is_segment_relative {
-                    // M=1: Segment-relative
-                    (read::RelocationKind::Absolute, 16, 0)
+                    (RelocationKind::SectionOffset, 16, 0)
                 } else {
-                    // M=0: PC-relative (self-relative)
-                    (read::RelocationKind::Relative, 16, -2)
+                    (RelocationKind::Relative, 16, -2)
                 }
             }
             omf::FixupLocation::Offset32 | omf::FixupLocation::LoaderOffset32 => {
                 if reloc.is_segment_relative {
-                    // M=1: Segment-relative
-                    (read::RelocationKind::Absolute, 32, 0)
+                    (RelocationKind::SectionOffset, 32, 0)
                 } else {
-                    // M=0: PC-relative (self-relative)
-                    (read::RelocationKind::Relative, 32, -4)
+                    (RelocationKind::Relative, 32, -4)
                 }
             }
-            omf::FixupLocation::Base => (read::RelocationKind::Absolute, 16, 0),
-            omf::FixupLocation::Pointer => (read::RelocationKind::Absolute, 32, 0),
-            omf::FixupLocation::Pointer48 => (read::RelocationKind::Absolute, 48, 0),
+            omf::FixupLocation::Base => {
+                if matches!(reloc.target_method, TargetMethod::SegmentIndex) {
+                    (RelocationKind::SectionIndex, 16, 0)
+                } else {
+                    (RelocationKind::Unknown, 16, 0)
+                }
+            }
+            omf::FixupLocation::Pointer => (RelocationKind::Absolute, 32, 0),
+            omf::FixupLocation::Pointer48 => (RelocationKind::Absolute, 48, 0),
+        };
+
+        if matches!(kind, RelocationKind::SectionOffset)
+            && !matches!(reloc.target_method, TargetMethod::SegmentIndex)
+        {
+            kind = RelocationKind::Unknown;
+        }
+
+        if matches!(
+            reloc.location,
+            omf::FixupLocation::LoaderOffset | omf::FixupLocation::LoaderOffset32
+        ) && matches!(reloc.frame_method, FrameMethod::ExternalIndex)
+        {
+            kind = RelocationKind::Unknown;
+        }
+
+        if matches!(reloc.target_method, TargetMethod::GroupIndex) {
+            kind = RelocationKind::Unknown;
+        }
+
+        let target = match reloc.target_method {
+            TargetMethod::SegmentIndex => {
+                if let Some(zero_based) = reloc.target_index.checked_sub(1) {
+                    let index = zero_based as usize;
+                    if index < self.file.segments.len() {
+                        RelocationTarget::Section(SectionIndex(index))
+                    } else {
+                        RelocationTarget::Absolute
+                    }
+                } else {
+                    RelocationTarget::Absolute
+                }
+            }
+            TargetMethod::ExternalIndex => {
+                // External indices in OMF are 1-based indices into the external-name table
+                if let Some(symbol) = self.file.external_symbol(reloc.target_index) {
+                    RelocationTarget::Symbol(SymbolIndex(symbol.symbol_index))
+                } else {
+                    RelocationTarget::Absolute
+                }
+            }
+            TargetMethod::GroupIndex | TargetMethod::FrameNumber => RelocationTarget::Absolute,
+        };
+
+        let fixup_frame = match reloc.frame_method {
+            FrameMethod::SegmentIndex => omf::FixupFrame::Segment(reloc.frame_index),
+            FrameMethod::GroupIndex => omf::FixupFrame::Group(reloc.frame_index),
+            FrameMethod::ExternalIndex => omf::FixupFrame::External(reloc.frame_index),
+            FrameMethod::FrameNumber => omf::FixupFrame::FrameNumber(reloc.frame_index),
+            FrameMethod::Location => omf::FixupFrame::Location,
+            FrameMethod::Target => omf::FixupFrame::Target,
+        };
+
+        let fixup_target = match reloc.target_method {
+            TargetMethod::SegmentIndex => omf::FixupTarget::Segment(reloc.target_index),
+            TargetMethod::GroupIndex => omf::FixupTarget::Group(reloc.target_index),
+            TargetMethod::ExternalIndex => omf::FixupTarget::External(reloc.target_index),
+            TargetMethod::FrameNumber => omf::FixupTarget::FrameNumber(reloc.target_index),
         };
 
         let relocation = Relocation {
             kind,
-            encoding: read::RelocationEncoding::Generic,
+            encoding: RelocationEncoding::Generic,
             size,
-            target: match reloc.target_method {
-                omf::TargetMethod::SegmentIndex => {
-                    read::RelocationTarget::Section(SectionIndex(reloc.target_index as usize))
-                }
-                omf::TargetMethod::ExternalIndex => {
-                    // External indices in OMF are 1-based indices into the external-name table
-                    if let Some(symbol) = self.file.external_symbol(reloc.target_index) {
-                        read::RelocationTarget::Symbol(read::SymbolIndex(symbol.symbol_index))
-                    } else {
-                        // Invalid external index
-                        read::RelocationTarget::Absolute
-                    }
-                }
-                _ => read::RelocationTarget::Absolute,
-            },
-            addend: reloc.target_displacement as i64 + addend,
+            target,
+            addend: (reloc.target_displacement as i64) + base_addend,
             implicit_addend: false,
-            flags: read::RelocationFlags::Generic {
-                kind,
-                encoding: read::RelocationEncoding::Generic,
-                size,
+            flags: RelocationFlags::Omf {
+                location: reloc.location,
+                mode: if reloc.is_segment_relative {
+                    omf::FixupMode::SegmentRelative
+                } else {
+                    omf::FixupMode::SelfRelative
+                },
+                frame: fixup_frame,
+                target: fixup_target,
+                // target_displacement: reloc.target_displacement,
             },
         };
 

src/read/omf/section.rs

@@ -1,18 +1,13 @@
-//! OMF section implementation.
-
 use alloc::borrow::Cow;
-use alloc::vec;
+use alloc::{vec, vec::Vec};
 use core::str;
 
-use crate::{
-    read::{
-        self, CompressedData, CompressedFileRange, Error, ObjectSection, ReadRef, RelocationMap,
-        Result, SectionFlags, SectionIndex, SectionKind,
-    },
-    ComdatKind, ObjectComdat, SymbolIndex,
+use crate::read::{
+    self, CompressedData, CompressedFileRange, Error, ObjectSection, ReadRef, RelocationMap,
+    Result, SectionFlags, SectionIndex, SectionKind,
 };
 
-use super::{relocation::OmfRelocationIterator, OmfDataChunk, OmfFile, OmfSegment};
+use super::{expand_lidata_block, OmfDataChunk, OmfFile, OmfRelocationIterator, OmfSegment};
 
 /// A section in an OMF file.
 #[derive(Debug)]
@@ -21,6 +16,16 @@ pub struct OmfSection<'data, 'file, R: ReadRef<'data>> {
     pub(super) index: usize,
 }
 
+/// An OMF group definition
+#[derive(Debug, Clone)]
+#[allow(unused)]
+pub(super) struct OmfGroup {
+    /// Group name index (into names table)
+    pub(super) name_index: u16,
+    /// Segment indices in this group
+    pub(super) segments: Vec<u16>,
+}
+
 impl<'data, 'file, R: ReadRef<'data>> OmfSection<'data, 'file, R> {
     fn segment(&self) -> &OmfSegment<'data> {
         &self.file.segments[self.index]
@@ -153,7 +158,7 @@ impl<'data, 'file, R: ReadRef<'data>> ObjectSection<'data> for OmfSection<'data,
                 }
                 OmfDataChunk::Iterated(lidata) => {
                     // LIDATA needs expansion
-                    if let Ok(expanded) = self.file.expand_lidata_block(lidata) {
+                    if let Ok(expanded) = expand_lidata_block(lidata) {
                         let end = start + expanded.len();
                         if end <= result.len() {
                             result[start..end].copy_from_slice(&expanded);
@@ -245,97 +250,3 @@ impl<'data, 'file, R: ReadRef<'data>> Iterator for OmfSectionIterator<'data, 'fi
         }
     }
 }
-
-/// A COMDAT section in an OMF file.
-#[derive(Debug)]
-pub struct OmfComdat<'data, 'file, R: ReadRef<'data>> {
-    file: &'file OmfFile<'data, R>,
-    index: usize,
-    _phantom: core::marker::PhantomData<&'data ()>,
-}
-
-impl<'data, 'file, R: ReadRef<'data>> read::private::Sealed for OmfComdat<'data, 'file, R> {}
-
-impl<'data, 'file, R: ReadRef<'data>> ObjectComdat<'data> for OmfComdat<'data, 'file, R> {
-    type SectionIterator = OmfComdatSectionIterator<'data, 'file, R>;
-
-    fn kind(&self) -> ComdatKind {
-        let comdat = &self.file.comdats[self.index];
-        match comdat.selection {
-            super::OmfComdatSelection::Explicit => ComdatKind::NoDuplicates,
-            super::OmfComdatSelection::UseAny => ComdatKind::Any,
-            super::OmfComdatSelection::SameSize => ComdatKind::SameSize,
-            super::OmfComdatSelection::ExactMatch => ComdatKind::ExactMatch,
-        }
-    }
-
-    fn symbol(&self) -> SymbolIndex {
-        // COMDAT symbols don't have a direct symbol index in OMF
-        SymbolIndex(usize::MAX)
-    }
-
-    fn name_bytes(&self) -> Result<&'data [u8]> {
-        let comdat = &self.file.comdats[self.index];
-        Ok(comdat.name)
-    }
-
-    fn name(&self) -> Result<&'data str> {
-        let comdat = &self.file.comdats[self.index];
-        core::str::from_utf8(comdat.name).map_err(|_| Error("Invalid UTF-8 in COMDAT name"))
-    }
-
-    fn sections(&self) -> Self::SectionIterator {
-        let comdat = &self.file.comdats[self.index];
-        OmfComdatSectionIterator {
-            segment_index: (comdat.segment_index as usize).checked_sub(1),
-            returned: false,
-            _phantom: core::marker::PhantomData,
-        }
-    }
-}
-
-/// An iterator over COMDAT sections.
-#[derive(Debug)]
-pub struct OmfComdatIterator<'data, 'file, R: ReadRef<'data>> {
-    pub(super) file: &'file OmfFile<'data, R>,
-    pub(super) index: usize,
-}
-
-impl<'data, 'file, R: ReadRef<'data>> Iterator for OmfComdatIterator<'data, 'file, R> {
-    type Item = OmfComdat<'data, 'file, R>;
-
-    fn next(&mut self) -> Option<Self::Item> {
-        if self.index < self.file.comdats.len() {
-            let comdat = OmfComdat {
-                file: self.file,
-                index: self.index,
-                _phantom: core::marker::PhantomData,
-            };
-            self.index += 1;
-            Some(comdat)
-        } else {
-            None
-        }
-    }
-}
-
-/// An iterator over sections in a COMDAT.
-#[derive(Debug)]
-pub struct OmfComdatSectionIterator<'data, 'file, R: ReadRef<'data>> {
-    segment_index: Option<usize>,
-    returned: bool,
-    _phantom: core::marker::PhantomData<(&'data (), &'file (), R)>,
-}
-
-impl<'data, 'file, R: ReadRef<'data>> Iterator for OmfComdatSectionIterator<'data, 'file, R> {
-    type Item = SectionIndex;
-
-    fn next(&mut self) -> Option<Self::Item> {
-        if !self.returned {
-            self.returned = true;
-            self.segment_index.map(|idx| SectionIndex(idx + 1))
-        } else {
-            None
-        }
-    }
-}

src/read/omf/segment.rs

@@ -1,6 +1,69 @@
-use crate::{read, ObjectSegment, ReadRef, Result, SegmentFlags};
+use alloc::vec::Vec;
 
-use super::OmfFile;
+use crate::read::{self, ObjectSegment, ReadRef, Result};
+use crate::{omf, SegmentFlags};
+
+use super::{OmfFile, OmfFixup};
+
+/// An OMF segment definition
+#[derive(Debug, Clone)]
+pub struct OmfSegment<'data> {
+    /// Segment name index (into names table)
+    pub(super) name_index: u16,
+    /// Class name index (into names table)
+    pub(super) class_index: u16,
+    /// Overlay name index (into names table)
+    #[allow(unused)] // TODO
+    pub(super) overlay_index: u16,
+    /// Segment alignment
+    pub(super) alignment: omf::SegmentAlignment,
+    /// Segment combination
+    pub(super) combination: omf::SegmentCombination,
+    /// Whether this is a 32-bit segment
+    #[allow(unused)] // TODO
+    pub(super) use32: bool,
+    /// Segment length
+    pub(super) length: u32,
+    /// Segment data chunks (offset, data)
+    /// Multiple LEDATA/LIDATA records can contribute to a single segment
+    pub(super) data_chunks: Vec<(u32, OmfDataChunk<'data>)>,
+    /// Relocations for this segment
+    pub(super) relocations: Vec<OmfFixup>,
+}
+
+/// Data chunk for a segment
+#[derive(Debug, Clone)]
+pub(super) enum OmfDataChunk<'data> {
+    /// Direct data from LEDATA record
+    Direct(&'data [u8]),
+    /// Compressed/iterated data from LIDATA record (needs expansion)
+    Iterated(&'data [u8]),
+}
+
+impl<'data> OmfSegment<'data> {
+    /// Get the raw data of the segment if it's a single contiguous chunk
+    pub fn get_single_chunk(&self) -> Option<&'data [u8]> {
+        if self.data_chunks.len() == 1 {
+            let (offset, chunk) = &self.data_chunks[0];
+            if *offset == 0 {
+                match chunk {
+                    OmfDataChunk::Direct(data) if data.len() == self.length as usize => {
+                        return Some(data);
+                    }
+                    _ => {}
+                }
+            }
+        }
+        None
+    }
+
+    /// Check if any data chunk needs expansion (LIDATA)
+    pub fn has_iterated_data(&self) -> bool {
+        self.data_chunks
+            .iter()
+            .any(|(_, chunk)| matches!(chunk, OmfDataChunk::Iterated(_)))
+    }
+}
 
 /// An OMF segment reference.
 #[derive(Debug)]

src/read/omf/symbol.rs

@@ -1,14 +1,53 @@
-//! OMF symbol implementation.
-
 use core::str;
 
 use crate::read::{
-    self, ObjectSymbol, ObjectSymbolTable, ReadRef, Result, SectionIndex, SymbolFlags, SymbolIndex,
-    SymbolKind, SymbolScope, SymbolSection,
+    self, Error, ObjectSymbol, ObjectSymbolTable, ReadRef, Result, SectionIndex, SymbolFlags,
+    SymbolIndex, SymbolKind, SymbolScope, SymbolSection,
 };
-use crate::Error;
 
-use super::{OmfFile, OmfSymbol};
+use super::OmfFile;
+
+/// An OMF symbol
+#[derive(Debug, Clone)]
+pub struct OmfSymbol<'data> {
+    /// Symbol table index
+    pub symbol_index: usize,
+    /// Symbol name
+    pub name: &'data [u8],
+    /// Symbol class (Public, External, etc.)
+    pub class: OmfSymbolClass,
+    /// Group index (0 if none)
+    pub group_index: u16,
+    /// Segment index (0 if external)
+    pub segment_index: u16,
+    /// Frame number (for absolute symbols when segment_index == 0)
+    pub frame_number: u16,
+    /// Offset within segment
+    pub offset: u32,
+    /// Type index (usually 0)
+    pub type_index: u16,
+    /// Pre-computed symbol kind
+    pub kind: SymbolKind,
+}
+
+/// Symbol class for OMF symbols
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum OmfSymbolClass {
+    /// Public symbol (PUBDEF)
+    Public,
+    /// Local public symbol (LPUBDEF)
+    LocalPublic,
+    /// External symbol (EXTDEF)
+    External,
+    /// Local external symbol (LEXTDEF)
+    LocalExternal,
+    /// Communal symbol (COMDEF)
+    Communal,
+    /// Local communal symbol (LCOMDEF)
+    LocalCommunal,
+    /// COMDAT external symbol (CEXTDEF)
+    ComdatExternal,
+}
 
 impl<'data> read::private::Sealed for OmfSymbol<'data> {}