Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

write/coff: add Writer #595

Merged
merged 1 commit into from
Nov 24, 2023
Merged

write/coff: add Writer #595

merged 1 commit into from
Nov 24, 2023

Conversation

philipc
Copy link
Contributor

@philipc philipc commented Nov 16, 2023

No description provided.

@simonbuchan
Copy link

simonbuchan commented Nov 23, 2023

So I ported over to this in:

https://github.com/simonbuchan/rustc_codegen_cranelift/tree/raw_dylib_object_write

That also now includes a test based on the enum-windows test I've been using, so you should be able to repro with ./y.sh test

I had to add some writer.write_align(4) before the section raw data, but I'm not sure if there's anything much you can do there if writer.write_section() can't be given the data nicely. Perhaps a writer.start_section() as an alias to writer.write_align(4)?

Incidentally regarding that align the docs for PointerToRawData says that:

For object files, the value should be aligned on a 4-byte boundary for best performance.

But if this will also be used for image files:

For executable images, this must be a multiple of FileAlignment from the optional header.

Not sure if that's an issue?

I should note that from memory, the MSVC linker pads the section raw data length to 2-bytes, but it seems fine without that.

@philipc
Copy link
Contributor Author

philipc commented Nov 23, 2023

Perhaps a writer.start_section() as an alias to writer.write_align(4)?

Sure, although I might call it writer.write_section_align().

For executable images, this must be a multiple of FileAlignment from the optional header.

The existing write::pe::Writer already handles that. I haven't tried to share code between the PE and COFF writers.

I should note that from memory, the MSVC linker pads the section raw data length to 2-bytes, but it seems fine without that.

Is that padding included in the size_of_raw_data field of the section header? Or is it just alignment padding before relocations or the symbol table?

@simonbuchan
Copy link

I'd need to double check what dumpbin said, but I'm pretty sure it was included in the length. The docs for SizeOfRawData do mention:

Because the SizeOfRawData field is rounded but the VirtualSize field is not, it is possible for SizeOfRawData to be greater than VirtualSize as well.

But it only mentions a rounding value for images.

The only time you could tell from the sections in these objects is the dllname, so entirely possible it's specific to them.

It's easy enough to handle on the use-side either way, so I wouldn't worry about it (my existing code ignored it, after all), I just wanted to mention it in case it causes something weird for someone later.

@philipc philipc merged commit ff50233 into gimli-rs:master Nov 24, 2023
12 checks passed
@philipc philipc deleted the coff-writer branch November 24, 2023 04:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants