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

[Rui Ueyama]

  +Bigcheese

  Michael, what do you think about refactoring the data structure for PE header?


================
Comment at: lib/Object/COFFObjectFile.cpp:467
@@ +466,3 @@
+      PE32Header = 0;
+      return;
+    }
----------------
Rafael Ávila de Espíndola wrote:
> 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.
> 
Sure. I agree with the idea to refactor the data structure as you suggested.


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