Do not ignore SizeOfOptionalHeader in COFF header even if PE header is not present.

[milkovic]

Summary

Attribute SizeOfOptionalHeader is ignored if no PE header is present in the file. This attribute should be ignored according to standard, however there are uses of this field even though it should not be used.

This change does not conform to PE/COFF standard, but there are several COFF files without PE header, where you had to add up SizeOfOptionalHeader in order to get proper section headers. Other tools and their own parsers do take this into account.

[ruiu]

How can I create a COFF file that has no PE header but has an optional header? Do you give me information as to what tools are creating such files?

[milkovic]

I can't really give you step by step guide on how to create the files like that, because this file was given to me to analyze, so I don't really know the source of the file. You can find the file in the attachment.

Driver.o66 KBDownload

Other tools have no problems with this file, but when processing it with LLVM, I run into issues with wrong section headers. I have not yet found the file where this change would cause problems.

[milkovic]

Pinging.

[ruiu]

Sorry for the belated response. I believe this is safe to land. LGTM.

[milkovic]

Thank you for approval. Would you please commit the change? I don't have rights to write to the repository.

[ruiu]

Hi Marek,

I submitted this as r278066 but had to revert because it broke llvm/test/tools/llvm-readobj/bigobj.test test. If you still want me to submit this, please take a look and fix. Thanks!

[milkovic]

I have updated the diff with the new version. The new version checks for presence of COFFHeader because it may be nullified when bigobj COFF is detected. I have run all the tests and everything seems to be OK.

Thanks.

[milkovic]

Committed in r278429. Thanks!

COFFObjectFile.cpp

if (Header->Magic == COFF::PE32Header::PE32) {
       DataDirSize = sizeof(data_directory) * PE32PlusHeader->NumberOfRvaAndSize;
     } else {
       // It's neither PE32 nor PE32+.
       EC = object_error::parse_failed;
       return;
     }
     if ((EC = getObject(DataDirectory, Data, DataDirAddr, DataDirSize)))
       return;
-    CurPtr += COFFHeader->SizeOfOptionalHeader;
   }
 
+  CurPtr += COFFHeader->SizeOfOptionalHeader;
+
   if ((EC = getObject(SectionTable, Data, base() + CurPtr,
                       (uint64_t)getNumberOfSections() * sizeof(coff_section))))
     return;
 
   // Initialize the pointer to the symbol table.
   if (getPointerToSymbolTable() != 0) {
     if ((EC = initSymbolTablePtr()))
       return;