Support for COMDAT sections for COFF output

[igg0]

Hi,

I'd like to include COMDAT support to nasm. When linking a win32/win64 module, it provides the possibility of function-level linking, and generally helps with dead code elimination (for example, when we link our x64 object files with a proper unwinding information, all object files from the library end up in the resulting module even if they are not used at all - most likely because the unwinding info introduces references to them and the linker is subsequently unable to remove them).

I'm not saying I'm any expert on COMDAT format - but I checked the specs, checked the Microsoft compiler/linker behavior on simple test files, and prepared some changes to nasm code. The output of the new nasm works fine with my code and gets rid of the dead code I've had so far, though of course I'm not suggesting I covered all possible scenarios.

The changes are restricted to outcoff.c. The new code is conditioned by the new section qualifier being used (or the subsequent coff_Section fields having been set) - so it shouldn't break any existing functionality.

The COMDAT support is enabled by comdat=... extension of the SECTION directive. The format I chose (for no special reason) is comdat=selection:name where selection is a number corresponding to one of the IMAGE_COMDAT_SELECT_* constants and name is the chosed comdat symbol/name). So it can be e.g.
SECTION .text$aa align=32 comdat=1:MyFunction1
If selection==5, the name is the comdat symbol of the section this section should be associated with. So the unwinding info for the previous example may start e.g. with
SECTION .xdata comdat=5:MyFunction1
Since the extension is ignored by older versions of nasm, a source using it successfully compiles even on older versions of nasm - just the code is emitted into the "big" sections, not split into comdat's (not saying this property is very useful, but it could be kinda convenient).

I created this pull request mainly so that you can check the code and give me your comments - regarding the chosen syntax of the extension, the code changes, and also about the interest in this feature :-) (I admit I'm not too familiar with what's happening during the multiple passes when nasm runs so maybe you'll spot some troubles.) If you find the syntax (and also the rest) OK, I can also extend the docs, prepare some samples etc.

Thanks!

[igg0]

Even though the specs don't say that a specific order of sections is required for IMAGE_COMDAT_SELECT_ASSOCIATIVE, it seems it is... so I added a little hack to support defining the associated "pair" in any order. If it's too ugly, it can be removed and enforced (as in the initial commit) that the first section has to be defined before the one with selection==5 (or possibly there's a better solution, of course).
Now I'll just wait for @hpax or @cyrillos I guess ;-)

[cyrillos]

Thanks for patches! I'll take a look once time permit. And very sorry for long delay in answers.

[igg0]

I know time is a precious commodity… I would just like to ask (if that answer itself isn’t a bigger task as a "design" decision) if you find the suggested syntax fine.
I’m OK with using a custom nasm build in our projects for the time being, however I’d rather avoid updating our asm code twice (if the COMDAT-related changes got included in nasm one day but with a different syntax).
Thanks!

[cyrillos]

@igg0 I'm very very sorry for taking you so long. You know we're not big fans of pull requests. Could you be so kind and send patches to https://lists.zytor.com/nasm-devel mailing list? At lease there would be a chance the patches get reviewed and merged. Again, I'm really sorry for keeping silence since my last reply.

[cyrillos]

Or you can send them to me directly to [email protected] and I'll resend them for review.