This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Do not create huge garbage files on section offset overflow.
ClosedPublic

Authored by grimar on Dec 25 2016, 7:52 AM.

Details

Summary

This can be reproduced easily on 32bit configuration with using testcase from D27712.
I believe there are other ways to achieve the same effect. Like having corrupted huge
sections sizes which we do not handle too.

But at last this is what we can fix easily as at the moment of issue ErrorCount > 0 already.

Issue is next:
FileSize is 0xfffffffffffff210 because of address/offsets calculation overflow.
And we trigger "unable to move location counter backward for: .text" in that case.

It works with a remark. FileSize it then passed as size_t and under win32 configuration truncates to 4gb,
that results in a creating temp 4gb files named "locationcountererr.s.tmp1.tmpXXXX" or alike,
that is a SSD space eater on each testcase run:

ErrorOr<std::unique_ptr<FileOutputBuffer>> BufferOrErr =
    FileOutputBuffer::create(Config->OutputFile, FileSize,
                             FileOutputBuffer::F_executable);

I suggest to add a simple check not only after openFile, but also before.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar updated this revision to Diff 82475.Dec 25 2016, 7:52 AM
grimar retitled this revision from to [ELF] - Do not create huge garbage files on section offset overflow..
grimar updated this object.
grimar added reviewers: ruiu, rafael, davide.
grimar added subscribers: llvm-commits, grimar, evgeny777.
davide edited edge metadata.Dec 25 2016, 9:31 AM

This needs a testcase.

This needs a testcase.

Should it check that we do not create temporarily file ? Seems there is no clean way to check this,
we do not know its exact name from test.

locationcounter.s already checks that "unable to move location counter backward for: .text" text is shown for this case.

Problem that openFile() creates temporarily huge file and unable to create mapped_file_region then,
so it leaves temporarily file behind. Not sure what is the best solution, but since we already know we have an error,
it seems reasonable not even try to open any files, that is what patch do.

ruiu edited edge metadata.Jan 11 2017, 5:26 PM

I think I suggested this before, but I'd suggest using uint64_t instead of uintX_t to keep a file size. On 32-bit, if a file size is >4GB, you want to report an error.

This revision was automatically updated to reflect the committed changes.