This is an archive of the discontinued LLVM Phabricator instance.

Fix incorrect check for running out of source locations.
ClosedPublic

Authored by ppluzhnikov on Oct 4 2022, 1:13 PM.

Details

Summary

When CurrentLoadedOffset is less than TotalSize, current code will
trigger unsigned overflow and will not return an "allocation failed"
indicator.

Google ref: b/248613299

Diff Detail

Event Timeline

ppluzhnikov created this revision.Oct 4 2022, 1:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 4 2022, 1:13 PM
ppluzhnikov published this revision for review.Oct 4 2022, 2:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 4 2022, 2:11 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Richard, could you take a look?

You've added the check in commit 78d81ecfc38c2

rupprecht edited reviewers, added: rsmith; removed: HAPPY.Oct 6 2022, 10:00 AM

Richard, ping?

dblaikie accepted this revision.Oct 18 2022, 1:03 PM
dblaikie added a subscriber: dblaikie.

Sounds good (guess this is pretty impractical to have a regression test/would require very large files to test?)

This revision is now accepted and ready to land.Oct 18 2022, 1:03 PM

Sounds good (guess this is pretty impractical to have a regression test/would require very large files to test?)

Yea, overflowing this requires a very convoluted setup; I don't think it's practical to have a unit test for this.

Could you commit this change? (I don't have commit rights in LLVM).

Thanks,

This revision was automatically updated to reflect the committed changes.