Page MenuHomePhabricator

[llvm-ar] Fix support for archives with members larger than 4GB
ClosedPublic

Authored by gbreynoo on Jul 22 2019, 7:25 AM.

Details

Summary

llvm-ar outputs a strange error message when handling archives with members larger than 4GB due to not checking file size when passing the value as an unsigned 32 bit integer. This overflow issue caused malformed archives to be created.:

https://bugs.llvm.org/show_bug.cgi?id=38058

This change allows for members above 4GB and will error in a case that is over the formats size limit, a 10 digit decimal integer.

Diff Detail

Repository
rL LLVM

Event Timeline

gbreynoo created this revision.Jul 22 2019, 7:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 22 2019, 7:25 AM
MaskRay added inline comments.Jul 22 2019, 7:39 AM
include/llvm/Object/Archive.h
224 ↗(On Diff #211092)

I am curious why you want to handle a file whose size is within [4294967296,9999999999] (4G is a bit unusual as a binary. But you don't allow it to be larger..).

I have not included a test for this change due to how large the test files would need to be.

gbreynoo marked an inline comment as done.Jul 22 2019, 8:07 AM
gbreynoo added inline comments.
include/llvm/Object/Archive.h
224 ↗(On Diff #211092)

In the past we have had a user that required support for members larger 4gb, and when experimenting with gnu-ar on my Ubuntu machine, it supports members over that size.

The maximum size value 9999999999 is a limit of the file format itself. The file size in bytes is stored as a 10 digit decimal integer, meaning MaxMemberSize is the largest member size the format supports.

I tried patching this, and it looks like you have \r\n endings that conflict w/ the checked in \n. Did you use --config core.autocrlf=false on your initial clone? (http://llvm.org/docs/GettingStarted.html#getting-started-quickly-a-summary)

rupprecht accepted this revision.Jul 22 2019, 11:10 AM

The patch tests out fine when piping through dos2unix (there's probably an equivalent on Windows), so LGTM if you can strip \r before submitting

This revision is now accepted and ready to land.Jul 22 2019, 11:10 AM
MaskRay accepted this revision.Jul 22 2019, 5:53 PM

Thanks rupprecht, I'll fix my clone before committing.

This revision was automatically updated to reflect the committed changes.