golang
proposal: debug/pe: add BigObj COFF support via new fields on File struct
Proposal Details
The debug/pe package currently uses this file struct:
// A File represents an open PE file.
type File struct {
FileHeader
OptionalHeader any // of type *OptionalHeader32 or *OptionalHeader64
Sections []*Section
Symbols []*Symbol // COFF symbols with auxiliary symbol records removed
COFFSymbols []COFFSymbol // all COFF symbols (including auxiliary symbol records)
StringTable StringTable
closer io.Closer
}
Unfortunately, in order to support BigObj COFF, (golang/go#24341), a few things need to change,.
BigObj files use a different type of header (ANON_OBJECT_HEADER_BIGOBJ) with largely the same fields as the FileHeader (IMAGE_FILE_HEADER) present already in the File struct.
You can find the C struct declarations in winnt.h.
The most important difference in the header is that FileHeader.NumberOfSections is now 32 bits, instead of 16 bits.
Supporting more sections in the file is the entire point of BigObj. I propose to add an extra:
int32 SectionCount field in the file struct to deal with this. In the case that the section count is greater than INT16_MAX, File.FileHeader.NumberOfSections will be zero. File.SectionCount will always contain an accurate number of sections.
File.FileHeader.NumberOfSections will be deprecated and new code should not rely on it, and instead refer to File.SectionCount to get the true number of sections.
The other fields in the header have 1:1 equivalents in the old FileHeader, so the code can just copy over the values from the BigObj header back to the FileHeader, and they can be accessed from that struct instead.
A similar approach has to be taken for Symbols and COFFSymbols.
Symbols inside regular COFF files are of the format IMAGE_SYMBOL (also present in winnt.h).
BigObj files instead have the format IMAGE_SYMBOL_EX. The only difference between them is that the SectionNumber field got expanded to 32 bits.
Unfortunately, the current COFFSymbol and Symbol structs have a 16 bit SectionNumber.
I propose adding two new fields:
BigObjSymbols []*BigObjSymbol
BigObjCOFFSymbols []BigObjCOFFSymbol
The only change in these structs will be a 32 bit SectionNumber.
Similar to the header, the old structs will be deprecated, and both of those arrays will be empty in the case that the section count is greater than INT16_MAX. New code should always prefer to use these fields.
For more details on BigObj, I wrote up a blog post explaining my findings when writing my initial patch: https://peter0x44.github.io/posts/bigobj_format_explained/ Unfortunately, Microsoft doesn't document the binary format. And it seems no one else did either. So you just have to "Trust Me, Bro". https://github.com/golang/go/pull/75631
My patch is functional. The issues are that it breaks the go compatibility guarantee.
I am happy to bikeshed over the naming of the structs in this API. I couldn't come up with any I fully liked.
Related Issues
- debug/pe: add APIs to support reading COMDAT info for sections #51868 (closed)
- debug/pe: big-obj is not supported #24341
Related Code Changes
- debug/pe: add support for bigobj COFF format
- debug/pe: introduce File.COFFSymbols and (*COFFSymbol).FullName
(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)
CC @golang/windows
There are two parts to this proposal.
The first part is to add a new field to pe.File to hold the 32-bit number of sections. Currently pe.File embeds pe.FileHeader, and pe.FileHeader has the field NumberOfSections uint16. The proposal is to add a new field to pe.File (not pe.FileHeader): SectionCount uint32. But actually I'm not sure we need that field. I think that the field is redundant with the length of the Sections field. I think that we can instead tell people to use len(f.Sections) rather than f.NumberOfSections to get the number of sections.
Unfortunately, I don't know that we have a way to formally deprecate a field inherited from an embedded struct. There is nothing wrong with the FileHeader.NumberOfSections field; that field is correct in the cases where the FileHeader struct is correct. The problem is with the File struct, which should be able to represent both traditional PE files and BigObj files.
The second part of the proposal is to handle the section number of a symbol. Currently the type Symbol has a field SectionNumber int16. The proposal above suggests adding new BigObjSymbol and BigObjCoffSymbol types.
I don't think we need BigObjSymbol; the Symbol type does not correspond to any data structure in a PE file. I think we can add a new field to Symbol: BigSectionNumber uint32. Then we can deprecate SectionNumber in favor of that field.
It's less clear if we can do this for COFFSymbol. That type does currently correspond to a type in the file. Still, we could decide to break that correspondence and add a BigSectionNumber field to that type as well. And, for that matter, to COFFSymbolAuxFormat5, which is returned by the (*File).COFFSymbolReadSectionDefAux method. This would complicate the code in the debug/pe package, but would simplify the code that uses it. It would be unfortunate if users would have to switch from the Symbols slice to a BigObjSymbols slice. It would be especially unfortunate if they have to always switch. Bad enough to force people to switch from SectionNumber to BigSectionNumber.
This proposal has been added to the active column of the proposals project and will now be reviewed at the weekly proposal review meetings. — aclements for the proposal review group
In general this seems like a good thing to support, but we need a concrete proposal that people are generally agreed on.
A concrete proposal:
- Document that the
NumberOfSectionsfield ofdebug/pe.File, which is a field inherited from the embedded typedebug/pe.FileHeader, is not necessarily accurate. People who need to know the number of sections given adebug/pe.Fileshould instead uselenof theSectionsfield. - Add a new field
FullSectionNumber, of typeuint32, todebug/pe.Symbol. Always set it to the symbol's section number. Document that the existingSectionNumberfield is incorrect if there are more than 65536 sections. - Similarly, add a new field
FullSectionNumber, of typeuint32, todebug/pe.COFFSymbol. - Similarly, add a new field
FullSectNum, of typeuint32, todebug/pe.COFFSymbolAuxFormat5.
I think those are the minimal changes we need to make it possible to read bigobj files.
Most of that proposal seems okay to me. But there's a few things of note I'd like to point out
Perhaps uint32 is not the correct type for these. Microsoft documents a few negative section numbers as being meaningful, here: https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#section-number-values
Now for maybe a little TMI from my results of testing various tools. The number of sections is not limited to 65535 for regular COFF. It's a little less.
65,279 is what they document. It seems to be accurate. I did some testing with cl, binutils, and clang, and learned the following: The maximum Binutils as will allow is 32766 The maximum Microsoft's assembler "ml64" will allow is 32767 The maximum cl.exe will allow is 65279. The same applies for clang. But instead of erroring, it will write out a bigobj coff instead, if 65279 is exceeded.
For bigobj, Microsoft document a limit of 4,294,967,296 (2^32-1). I could not test this because compiling/assembling such a file takes too long.
I'm fine with using int32 or int rather than uint32. Thanks.
- Document that the
NumberOfSectionsfield ofdebug/pe.File, which is a field inherited from the embedded typedebug/pe.FileHeader, is not necessarily accurate. People who need to know the number of sections given adebug/pe.Fileshould instead uselenof theSectionsfield. - Add a new field
FullSectionNumber, of typeint32, todebug/pe.Symbol. Always set it to the symbol's section number. Document that the existingSectionNumberfield is incorrect if there are more than 65536 sections. - Similarly, add a new field
FullSectionNumber, of typeint32, todebug/pe.COFFSymbol. - Similarly, add a new field
FullSectNum, of typeint32, todebug/pe.COFFSymbolAuxFormat5.
Well, on further thought, I'm not sure if Microsoft is expecting the section count to be signed or unsigned. Or signed, but you cast to unsigned. Since 65,279 is larger than INT16_MAX. a (D)WORD is signed. But they also do document the negative section numbers. So I'm confused.
Well okay, thinking about it further, "count" and "number" are two distinct things technically. So Int32 for FullSectionNumber is correct. But NumberOfSections should be unsigned. But it doesn't matter since we are telling people to use len(Sections) anyway.
So this proposal:
- Document that the
NumberOfSectionsfield ofdebug/pe.File, which is a field inherited from the embedded typedebug/pe.FileHeader, is not necessarily accurate. People who need to know the number of sections given adebug/pe.Fileshould instead uselenof theSectionsfield.- Add a new field
FullSectionNumber, of typeint32, todebug/pe.Symbol. Always set it to the symbol's section number. Document that the existingSectionNumberfield is incorrect if there are more than 65536 sections.- Similarly, add a new field
FullSectionNumber, of typeint32, todebug/pe.COFFSymbol.- Similarly, add a new field
FullSectNum, of typeint32, todebug/pe.COFFSymbolAuxFormat5.
is OK. Approved by me.
Similarly, add a new field FullSectionNumber, of type int32, to debug/pe.COFFSymbol. Similarly, add a new field FullSectNum, of type int32, to debug/pe.COFFSymbolAuxFormat5.
I arrived at the same conclusion that @ianlancetaylor did up thread that both of these layouts are currently disk-compatible, read using binary.Read.
Changing this could potentially break some callers, for example, if any assume they can binary.Write these types. Maybe that doesn't happen in practice.
I'm more worried about File.COFFSymbols, which has type []COFFSymbol, but includes both COFFSymbol values and COFFSymbolAuxFormat5 values, and code assumes it can unsafe cast between them. This is marginally excusable in a world where they're just the on-disk structures because those are guaranteed to be the same byte size. At a minimum, if we do change them, we need to test that they're the same byte size. But in general this makes me reticent to change COFFSymbol and COFFSymbolAuxFormat5.
Fair points. Unfortunately that leaves us with no good approach. Perhaps we could define additional File methods and tell people to never use the COFFSymbols field.
For bigobj, Microsoft document a limit of 4,294,967,296 (2^32-1).
This sounds like it either needs to be a uint32 or a type with an even larger range, like int64. int64 would also mean we don't have to have this conversation again in 10 years when Microsoft defines ReallyBigCOFF :)
How are IMAGE_SYM_ABSOLUTE (-1) and IMAGE_SYM_DEBUG (-2) encoded in BigCOFF?
The count is unsigned (despite being a DWORD...) but I think the section number/ID is signed. That's how I figure it works. But I will see how I can test it out.
How are IMAGE_SYM_ABSOLUTE (-1) and IMAGE_SYM_DEBUG (-2) encoded in BigCOFF?
OK. I got around to testing it, and the result is unsurprising.
0xFFFFFFFFFF for IMAGE_SYM_ABSOLUTE and 0xFEFFFFFF for IMAGE_SYM_DEBUG.
Of course, the second is -2 in little endian.
int64 would also mean we don't have to have this conversation again in 10 years when Microsoft defines ReallyBigCOFF :)
If this ever happens, we are probably getting a whole new executable format for Windows that isn't limited to 4GiB in the first place. And also, having dlls that can export more than 65535 symbols.
So, what should I implement? I think this will probably remain stalled indefinitely if I don't prompt it again...
The int64 was a minor problem. But we never found a way to address my concern about changing COFFSymbol and COFFSymbolAuxFormat5 given the unsafe casts between them and current disk-compatibility.