[PATCH] readobj: Dump PE/COFF optional records.

[Rafael Ávila de Espíndola]

================
Comment at: lib/Object/COFFObjectFile.cpp:467
@@ +466,3 @@
+      PE32Header = 0;
+      return;
+    }
----------------
Rui Ueyama wrote:
> Rui Ueyama wrote:
> > Rafael Ávila de Espíndola wrote:
> > > You are not setting an error and returning early, this will leave the file in a inconsistent state (SectionTable is not set for example).
> > > 
> > > When is the pe header not present? It would be much nicer if we could have just one field named Header of type pe32_header.
> > > 
> > PE header is present in executable and DLL, and is not in object file. There are two types of PE header, PE32 and PE32+ (64 bit version of PE). The size of PE32+ is different from PE32 and is not handled in this patch.
> > 
> > My intention was to reset PE32Header if it's PE32+ and fall through to the following code, so that the COFF part of the PE32+ file will be output. Writing return at this line was mistake.
> So, because PE header is not present for COFF, we can't have one field named pe32_header. We could remove coff_file_header from pe32_header if we want to avoid the duplication, though.
I guess it would be better to refactor the datastructures a bit. We could have pe32_header and pe32plus_header start at the Magic field, that way the structure of a dll and exe would be

Signature
coff_file_header
pe32_header or pe32plus_header.

that way we avoid the overlap. Can you please add Michael Spencer as a reviewer too? I just want to make sure he is ok with splitting the structures that way.



http://llvm-reviews.chandlerc.com/D939