This is an archive of the discontinued LLVM Phabricator instance.

[Object] Fix the return type of getOffset/getSize
ClosedPublic

Authored by alexander-shaposhnikov on Oct 17 2019, 11:28 AM.

Details

Summary

Header64.offset/Header64.size are uint64_t, thus we should not truncate them to unit32_t.
Moreover, there are a number of places where we sum the offset and the size (e.g. in various checks in MachOUniversal.cpp),
the truncation causes issues since the offset/size can perfectly fit into uint32_t, while the sum overflows.

Diff Detail

Repository
rL LLVM

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptOct 17 2019, 11:28 AM
alexander-shaposhnikov edited the summary of this revision. (Show Details)Oct 17 2019, 11:31 AM
compnerd accepted this revision.Oct 17 2019, 11:31 AM

I wish that we had a better type. For the time being, the 64-bit with is sufficient for current needs.

This revision is now accepted and ready to land.Oct 17 2019, 11:31 AM
smeenai accepted this revision.Oct 17 2019, 11:32 AM

This makes sense, but I'm wondering if we need to adjust any call sites to assert that a result they compute that needs to be 32 bits actually fits in 32 bits. Those locations would have just been silently overflowing before though, so this at least gives them the ability to detect that overflow.

This revision was automatically updated to reflect the committed changes.