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

Issue 4645083: Unify metadata support, and add metadata output to relink.exe (Closed)

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

Description

Unify metadata support, and add metadata output to relink.exe This CL unifies how metadata is handled, centralizing it. It retrofits existing metadata users to use the new common::Metadata class, and adds metadata output to relink.exe produced modules. Additionally, command-line and timestamp information has been added to the metadata. This CL also modifies both instrument.exe and relink.exe to ensure that .rsrc is kept as the second to last section, regardless. This ensure that modules produced by our toolchain are compatible with resource editors. The metadata section has been moved to be just prior to .rsrc if it exists, otherwise just prior to .relocs. BUG=http://code.google.com/p/sawbuck/issues/detail?id=37 Committed: http://code.google.com/p/sawbuck/source/browse/#svn/trunk382

Patch Set 1 #

Total comments: 2

Patch Set 2 : '' #

Total comments: 14

Patch Set 3 : '' #

Total comments: 1

Patch Set 4 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+939 lines, -330 lines) Patch
M syzygy/common/defs.h View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M syzygy/common/defs.cc View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M syzygy/common/syzygy_version.h View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M syzygy/common/syzygy_version.cc View 1 2 3 2 chunks +6 lines, -0 lines 0 comments Download
M syzygy/common/syzygy_version_unittest.cc View 1 2 3 1 chunk +4 lines, -0 lines 0 comments Download
M syzygy/core/serialization_impl.h View 1 2 3 2 chunks +22 lines, -0 lines 0 comments Download
M syzygy/instrument/instrumenter.h View 1 2 3 3 chunks +13 lines, -6 lines 0 comments Download
M syzygy/instrument/instrumenter.cc View 1 2 3 8 chunks +72 lines, -74 lines 0 comments Download
A syzygy/pe/metadata.h View 1 2 1 chunk +121 lines, -0 lines 0 comments Download
A syzygy/pe/metadata.cc View 1 2 3 1 chunk +436 lines, -0 lines 0 comments Download
A syzygy/pe/metadata_unittest.cc View 1 2 1 chunk +130 lines, -0 lines 0 comments Download
M syzygy/pe/pe.gyp View 1 2 3 4 chunks +5 lines, -0 lines 0 comments Download
M syzygy/pe/pe_file.h View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M syzygy/relink/order_relinker_unittest.cc View 1 2 3 2 chunks +4 lines, -2 lines 0 comments Download
M syzygy/relink/random_relinker_unittest.cc View 1 2 3 2 chunks +4 lines, -2 lines 0 comments Download
M syzygy/relink/relink.gyp View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M syzygy/relink/relink_main.cc View 1 2 3 3 chunks +8 lines, -2 lines 0 comments Download
M syzygy/relink/relinker.h View 1 2 3 5 chunks +14 lines, -3 lines 0 comments Download
M syzygy/relink/relinker.cc View 1 2 3 6 chunks +62 lines, -2 lines 0 comments Download
M syzygy/relink/relinker_unittest.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M syzygy/reorder/reorderer.cc View 1 2 3 6 chunks +25 lines, -238 lines 0 comments Download

Messages

Total messages: 12
chrisha
PTAL
12 years, 8 months ago (2011-07-06 21:29:22 UTC) #1
Siggi
lgtm, except you want the segment to live before resources. Let's make it the first ...
12 years, 8 months ago (2011-07-06 21:38:42 UTC) #2
chrisha
It's currently the second to last section so as not to affect RVAs in the ...
12 years, 8 months ago (2011-07-06 21:42:54 UTC) #3
Siggi
lgtm
12 years, 8 months ago (2011-07-06 21:43:21 UTC) #4
Roger McFarlane
lgtm
12 years, 8 months ago (2011-07-07 14:39:38 UTC) #5
chrisha
This CL got a lot more involved, so it definitely needs another review. PTAL
12 years, 8 months ago (2011-07-07 21:25:54 UTC) #6
Siggi
nice! http://codereview.appspot.com/4645083/diff/14001/syzygy/core/serialization_impl.h File syzygy/core/serialization_impl.h (right): http://codereview.appspot.com/4645083/diff/14001/syzygy/core/serialization_impl.h#newcode24 syzygy/core/serialization_impl.h:24: // Forward declare base::Time. Just include base/time.h, we ...
12 years, 8 months ago (2011-07-08 13:56:13 UTC) #7
Roger McFarlane
Doh! I came late to the (after-)party and Siggi nabbed all the nits! LGTM modulo ...
12 years, 8 months ago (2011-07-08 14:24:26 UTC) #8
chrisha
Fixed all of siggi's nits, and added new unit tests to metadata_unittest.cc. One last look? ...
12 years, 8 months ago (2011-07-08 17:45:08 UTC) #9
Siggi
lgtm http://codereview.appspot.com/4645083/diff/21001/syzygy/pe/metadata.cc File syzygy/pe/metadata.cc (right): http://codereview.appspot.com/4645083/diff/21001/syzygy/pe/metadata.cc#newcode60 syzygy/pe/metadata.cc:60: struct tm timeinfo; nit: initialize at declaration timeinfo ...
12 years, 8 months ago (2011-07-08 17:48:12 UTC) #10
Roger McFarlane
lgtm On 8 July 2011 13:48, <siggi@chromium.org> wrote: > lgtm > > > http://codereview.appspot.com/**4645083/diff/21001/syzygy/pe/**metadata.cc<http://codereview.appspot.com/4645083/diff/21001/syzygy/pe/metadata.cc> > ...
12 years, 8 months ago (2011-07-08 17:58:33 UTC) #11
chrisha
12 years, 8 months ago (2011-07-08 18:41:06 UTC) #12
Thanks, committing.
Sign in to reply to this message.

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