You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This is very much a work-in-progress, but opening a draft PR to get early feedback.
Open questions:
OMF's segment data is strange and doesn't map cleanly to the existing API. Right now, data() will attempt to return a contiguous &'data [u8] chunk if it can, but errors if the data is non-contiguous or requires expansion (LIDATA). Instead, I use the uncompressed_data API to expose the expanded/contiguous data.
Side note: This also doesn't map well to the CompressedData struct, which expects a contiguous, compressed data chunk as the source. The impl of uncompressed_data simply does self.compressed_data()?.decompress(), so I had to add a with_inner! for uncompressed_data so that it calls my overridden version in OmfFile.
Instead, I use the uncompressed_data API to expose the expanded/contiguous data.
That's logically what I would expect that API to return, so I think this is ok.
I don't want to change the data and compressed_data APIs to allow disjoint data just for OMF. We could add methods to OmfSection if that information is required (or maybe even add a new ObjectSection method). Do you have a need in objdiff for more than what uncompressed_data gives you?
I don't have any major feedback, this looks sane to me. Thanks for working on it.
For testing, the preferred approach is to run objdump on the test files and record the expected output in crates/examples/testfiles. Handwritten tests are fine for things that doesn't cover. Also readobj support in the future might help too, but it's not needed immediately.
then manually verify comprehensive_test.obj.objdump before adding it to git.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is very much a work-in-progress, but opening a draft PR to get early feedback.
Open questions:
data()will attempt to return a contiguous&'data [u8]chunk if it can, but errors if the data is non-contiguous or requires expansion (LIDATA). Instead, I use theuncompressed_dataAPI to expose the expanded/contiguous data.CompressedDatastruct, which expects a contiguous, compressed data chunk as the source. The impl ofuncompressed_datasimply doesself.compressed_data()?.decompress(), so I had to add awith_inner!foruncompressed_dataso that it calls my overridden version inOmfFile.To-do:
object-testfiles. (Done)Absolute/Relativerelocation kinds needs rethinking.RelocationFlagsvariant?)Resources:
Resolves #736