Start refactoring to reduce executable type knowledge.

Start refactoring to reduce executable type knowledge.

This creates executable detection functions, a globally shared enum for
describing an executable type, and reduces the number of classes and
locations with executable specific knowledge.

These changes, along with moving architecture specific classes into their
own files should make it easier to produce special purpose clients that
only contain the code required to apply their own form of patch.

DisassemblerWin32EXE, ImagePE, CourgetteWin32X86PatchGenerator, and
CourgetteWin32X86Patcher, and ensemble handling are all heavily affected here.

This should have no effect on the behavior of the system yet, and is instead
all prep-work.

This is the same as an earlier CL, except that ParseHeader will now return
an error for 64 bit PE executables, and resource only DLLs. This is because
the detection factories depend on ParseHeader to decide if a given file
is supported.

BUG=None
TEST=Unittests

Review URL: http://codereview.chromium.org/7920004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@103879 0039d316-1c4b-4281-b951-d872f2087c98


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=106793

[dgarrett, 2011-10-19 02:01:38]

http://codereview.chromium.org/8344037/diff/1/courgette/image_info.cc
File courgette/image_info.cc (right):

http://codereview.chromium.org/8344037/diff/1/courgette/image_info.cc#newcode311
courgette/image_info.cc:311: }
These are the changes that are new from the previous CL.

They cause ParseHeader to fail for 64 bit PE's, and for 
resource only DLLs.

[Chris Masone, 2011-10-21 01:18:08]

http://codereview.chromium.org/8344037/diff/2001/courgette/ensemble_create.cc
File courgette/ensemble_create.cc (right):

http://codereview.chromium.org/8344037/diff/2001/courgette/ensemble_create.cc...
courgette/ensemble_create.cc:73: new CourgetteWin32X86PatchGenerator(
Hm.  Why not just return the allocated object?  Less intrusive change, I
supposed.

[dgarrett, 2011-10-21 01:22:38]

http://codereview.chromium.org/8344037/diff/2001/courgette/ensemble_create.cc
File courgette/ensemble_create.cc (right):

http://codereview.chromium.org/8344037/diff/2001/courgette/ensemble_create.cc...
courgette/ensemble_create.cc:73: new CourgetteWin32X86PatchGenerator(
Probably because the block that does real work I just moved around into a
switch. Could just return it.

[Chris Masone, 2011-10-21 01:23:24]

This looks OK to me, except that the diff doesn't seem to be right.  I guess
LGTM pending trybots, but I'm hopeful that someone with context will chime in
here

On 2011/10/21 01:18:08, Chris Masone wrote:
> http://codereview.chromium.org/8344037/diff/2001/courgette/ensemble_create.cc
> File courgette/ensemble_create.cc (right):
> 
>
http://codereview.chromium.org/8344037/diff/2001/courgette/ensemble_create.cc...
> courgette/ensemble_create.cc:73: new CourgetteWin32X86PatchGenerator(
> Hm.  Why not just return the allocated object?  Less intrusive change, I
> supposed.

[Michael Krebs, 2011-10-21 02:44:36]

Build failure notwithstanding, LGTM with a nit.

http://codereview.chromium.org/8344037/diff/8001/courgette/courgette.h
File courgette/courgette.h (right):

http://codereview.chromium.org/8344037/diff/8001/courgette/courgette.h#newcode93
courgette/courgette.h:93: // |*output| to NULL.
I'm guessing the above comments should be deleted, and perhaps a comment written
for DetectExecutableType().

http://codereview.chromium.org/8344037/diff/8001/courgette/ensemble.h
File courgette/ensemble.h (right):

http://codereview.chromium.org/8344037/diff/8001/courgette/ensemble.h#newcode57
courgette/ensemble.h:57: virtual PEInfo* GetImageInfo() const { return info_; }
You made the method name generic, while the return value is still PE-specific. 
Given the data is also PE-specific, I would think GetPEInfo() would be fine. 
...although this nit may be moot depending on what changes are made for ELF.

[dgarrett, 2011-10-21 18:24:59]

http://codereview.chromium.org/8344037/diff/8001/courgette/courgette.h
File courgette/courgette.h (right):

http://codereview.chromium.org/8344037/diff/8001/courgette/courgette.h#newcode93
courgette/courgette.h:93: // |*output| to NULL.
On 2011/10/21 02:44:36, Michael Krebs wrote:
> I'm guessing the above comments should be deleted, and perhaps a comment
written
> for DetectExecutableType().

Done.

http://codereview.chromium.org/8344037/diff/8001/courgette/ensemble.h
File courgette/ensemble.h (right):

http://codereview.chromium.org/8344037/diff/8001/courgette/ensemble.h#newcode57
courgette/ensemble.h:57: virtual PEInfo* GetImageInfo() const { return info_; }
On 2011/10/21 02:44:36, Michael Krebs wrote:
> You made the method name generic, while the return value is still PE-specific.

> Given the data is also PE-specific, I would think GetPEInfo() would be fine. 
> ...although this nit may be moot depending on what changes are made for ELF.

You are right, but this gets fixed in the next step of the refactor.

[commit-bot: I haz the power, 2011-10-21 20:24:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dgarrett@chromium.org/8344037/13002

[commit-bot: I haz the power, 2011-10-21 22:24:21]

Change committed as 106793