This is an archive of the discontinued LLVM Phabricator instance.

[COFF] Check for sections larger than 4 GiB
ClosedPublic

Authored by smeenai on Sep 18 2017, 3:39 PM.

Details

Summary

Sections are limited to 4 GiB. Error out early if a section exceeds this
size, rather than overflowing the section size and getting confusing
assertion failures/segfaults later.

Event Timeline

smeenai created this revision.Sep 18 2017, 3:39 PM

I'm not sure how to write a test case for this without creating an object file with a section > 4 GiB (as in that object file will take upwards of 4 GiB on disk). Any ideas?

ruiu edited edge metadata.Sep 18 2017, 3:43 PM

How about creating huge .bss sections and link them?

In D38005#874414, @ruiu wrote:

How about creating huge .bss sections and link them?

Do you mean using yaml2obj? I don't know of a way to do that via assembler directives (and I'd love to learn if there is one).

davide added a subscriber: davide.Sep 18 2017, 3:48 PM

yaml2obj should allow you to create a broken object like that. IIRC there's an example in test/ELF.

ruiu added a comment.Sep 18 2017, 3:49 PM

I believe there's a way to create a huge .bss section using assembler, but yaml2obj is fine. We are using it for COFF tests (but not for ELF tests.)

ruiu added inline comments.Sep 18 2017, 5:36 PM
COFF/Writer.cpp
191

Can you use errro() instead? We use fatal() only for reporting corrupted input files. Files with too large sections are not technically corrupted.

smeenai updated this revision to Diff 115775.Sep 18 2017, 7:19 PM

Change to error and add test.

smeenai updated this revision to Diff 115777.Sep 18 2017, 7:25 PM

Exit with correct error code.

ruiu accepted this revision.Sep 19 2017, 1:43 PM

LGTM

test/COFF/section-size.s
3–5 ↗(On Diff #115777)

It seems just %t1.obj and %t3.obj suffice.

9 ↗(On Diff #115777)

Instead of creating a temporary file by cp, you can just pass the same object file twice.

This revision is now accepted and ready to land.Sep 19 2017, 1:43 PM
smeenai added inline comments.Sep 19 2017, 4:04 PM
test/COFF/section-size.s
3–5 ↗(On Diff #115777)

Sure, will rename.

9 ↗(On Diff #115777)

lld-link deduplicates input object files: https://reviews.llvm.org/diffusion/L/browse/lld/trunk/COFF/Driver.cpp;313693$295-303. Is there some other way to get around that?

ruiu added inline comments.Sep 19 2017, 4:06 PM
test/COFF/section-size.s
9 ↗(On Diff #115777)

Oh, I didn't know that. cp is of course fine if that's the case.

This revision was automatically updated to reflect the committed changes.