This is an archive of the discontinued LLVM Phabricator instance.

[COFF] Widen PE32Header fields to fit 64 bit versions
ClosedPublic

Authored by mstorsjo on Nov 19 2019, 2:19 PM.

Details

Summary

The PE32Header struct is only used by COFFYAML, for intermediate storage. The struct doesn't match the on-disk struct layout as it uses native integers instead of e.g. support::ulittle32_t, so just widen the fields to fit values for object::pe32plus_header, in addition to object::pe32_header.

This avoids truncating the 64 bit ImageBase for 64 bit executables.

Diff Detail

Event Timeline

mstorsjo created this revision.Nov 19 2019, 2:19 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 19 2019, 2:19 PM
rnk accepted this revision.Nov 19 2019, 2:41 PM

lgtm

Last time we did this, we just widened the relevant fields for bigobj:
rG4d57159c09b226a5608a26c1bc57f7b66cb80a24

This revision is now accepted and ready to land.Nov 19 2019, 2:41 PM
In D70464#1752563, @rnk wrote:

lgtm

Last time we did this, we just widened the relevant fields for bigobj:
rG4d57159c09b226a5608a26c1bc57f7b66cb80a24

Ah, good point - as this struct doesn't need to match any on-disk layout we could just do that instead. Will update with a new iteration of the patch (and commit within a couple days if there's no opposition to that form).

mstorsjo updated this revision to Diff 230199.Nov 20 2019, 12:08 AM
mstorsjo retitled this revision from [COFFYAML] Use PE32PlusHeader for storing the optional header to [COFF] Widen PE32Header fields to fit 64 bit versions.
mstorsjo edited the summary of this revision. (Show Details)

Widen the necessary fields in the existing struct instead of adding a completely separate struct.

rnk accepted this revision.Nov 20 2019, 3:59 PM

lgtm

This revision was automatically updated to reflect the committed changes.