Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2111)

Issue 4639088: Make reorderer validate instrumenter signature (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 10 months ago by chrisha
Modified:
12 years, 9 months ago
Reviewers:
Roger McFarlane, Siggi
CC:
sawbuck-changes_googlegroups.com
Base URL:
http://sawbuck.googlecode.com/svn/trunk/
Visibility:
Public.

Description

Make reorderer validate instrumenter signature This CL makes the reorderer validate the input module versus the metadata in the instrumented module. The reorderer will refuse to run if its version is inconsistent with the instrumented module version, or if the input module signature does not match that of the original module that was run through the instrumenter. As a side effect of the metadata, this CL also makes the --input-dll command optional, as it can be inferred from the metadata. It can still be provided if the module has been moved since original instrumentation, and all validation will still work. Also added a handful of unittests. The relinker unit tests require the ability to work on test data that has been generated from a current version of the toolchain. To meet this requirement, this CL creates a test_data project, which uses the toolchain to populate $(OutDir)/test_data with auto-generated test data. The unittests can then depend on the appropriate target to ensure that the data they require has been generated. BUG=http://code.google.com/p/sawbuck/issues/detail?id=37 Committed: http://code.google.com/p/sawbuck/source/browse/#svn/trunk373

Patch Set 1 : '' #

Total comments: 5

Patch Set 2 : '' #

Total comments: 2

Patch Set 3 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+405 lines, -46 lines) Patch
M syzygy/common/common.gyp View 2 1 chunk +2 lines, -0 lines 0 comments Download
A syzygy/common/defs.h View 1 chunk +29 lines, -0 lines 0 comments Download
A syzygy/common/defs.cc View 1 chunk +21 lines, -0 lines 0 comments Download
M syzygy/common/syzygy_version.h View 1 chunk +6 lines, -0 lines 0 comments Download
M syzygy/common/syzygy_version.cc View 1 chunk +5 lines, -0 lines 0 comments Download
M syzygy/common/syzygy_version_unittest.cc View 1 chunk +11 lines, -0 lines 0 comments Download
M syzygy/instrument/instrument.gyp View 1 chunk +1 line, -0 lines 0 comments Download
M syzygy/instrument/instrumenter.cc View 3 chunks +19 lines, -3 lines 0 comments Download
M syzygy/pe/pe_file.h View 1 chunk +6 lines, -1 line 0 comments Download
M syzygy/pe/pe_file_unittest.cc View 2 chunks +82 lines, -0 lines 0 comments Download
M syzygy/relink/order_relinker_unittest.cc View 1 4 chunks +26 lines, -10 lines 0 comments Download
M syzygy/relink/relink.gyp View 1 1 chunk +1 line, -0 lines 0 comments Download
M syzygy/reorder/reorder.gyp View 1 chunk +1 line, -0 lines 0 comments Download
M syzygy/reorder/reorder_main.cc View 2 chunks +13 lines, -12 lines 0 comments Download
M syzygy/reorder/reorderer.h View 1 3 chunks +9 lines, -7 lines 0 comments Download
M syzygy/reorder/reorderer.cc View 1 4 chunks +83 lines, -13 lines 0 comments Download
M syzygy/syzygy.gyp View 1 1 chunk +1 line, -0 lines 0 comments Download
A syzygy/test_data/test_data.gyp View 1 1 chunk +89 lines, -0 lines 0 comments Download

Messages

Total messages: 8
chrisha
PTAL
12 years, 9 months ago (2011-07-04 15:14:10 UTC) #1
Roger McFarlane
lgtm with nits. http://codereview.appspot.com/4639088/diff/13001/syzygy/reorder/reorder_main.cc File syzygy/reorder/reorder_main.cc (right): http://codereview.appspot.com/4639088/diff/13001/syzygy/reorder/reorder_main.cc#newcode43 syzygy/reorder/reorder_main.cc:43: " specified it will be inferred ...
12 years, 9 months ago (2011-07-04 15:30:41 UTC) #2
Siggi
lgtm, modulo Roger's nits.
12 years, 9 months ago (2011-07-04 15:41:59 UTC) #3
chrisha
Addresses the nits. While running unittests, I realized I broke order_relinker_unittest. The problem is that ...
12 years, 9 months ago (2011-07-04 18:14:25 UTC) #4
chrisha
Ping!
12 years, 9 months ago (2011-07-04 19:22:28 UTC) #5
Siggi
lgtm http://codereview.appspot.com/4639088/diff/16001/syzygy/common/common.gyp File syzygy/common/common.gyp (right): http://codereview.appspot.com/4639088/diff/16001/syzygy/common/common.gyp#newcode26 syzygy/common/common.gyp:26: 'target_name': 'test_data_dir', leftovers?
12 years, 9 months ago (2011-07-04 20:00:23 UTC) #6
chrisha
Thanks, committing. http://codereview.appspot.com/4639088/diff/16001/syzygy/common/common.gyp File syzygy/common/common.gyp (right): http://codereview.appspot.com/4639088/diff/16001/syzygy/common/common.gyp#newcode26 syzygy/common/common.gyp:26: 'target_name': 'test_data_dir', On 2011/07/04 20:00:23, Siggi wrote: ...
12 years, 9 months ago (2011-07-04 20:11:28 UTC) #7
Roger McFarlane
12 years, 9 months ago (2011-07-04 20:13:57 UTC) #8
lgtm
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b