This is an archive of the discontinued LLVM Phabricator instance.

[Sema] Address-space sensitive check for unbounded arrays (v2)
ClosedPublic

Authored by chrish_ericsson_atx on Sep 23 2020, 11:31 AM.

Details

Summary

Check applied to unbounded (incomplete) arrays and pointers to spot
cases where the computed address is beyond the largest possible
addressable extent of the array, based on the address space in which the
array is delcared, or which the pointer refers to.

Check helps to avoid cases of nonsense pointer math and array indexing
which could lead to linker failures or runtime exceptions. Of
particular interest when building for embedded systems with small
address spaces.

This is version 2 of this patch -- version 1 had some testing issues
due to a sign error in existing code. That error is corrected and
lit test for this chagne is extended to verify the fix.

Originally reviewed/accepted by: aaron.ballman
Original revision: https://reviews.llvm.org/D86796

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptSep 23 2020, 11:31 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
chrish_ericsson_atx requested review of this revision.Sep 23 2020, 11:31 AM
clang/lib/Sema/SemaChecking.cpp
14552

This call to index.setIsUnsigned(false) is the only unreviewed part of this change in this file.

clang/test/Sema/unbounded-array-bounds.c
73–81

This function is the only unreviewed part of this change in this file.

ebevhan accepted this revision.Sep 29 2020, 4:43 AM

LGTM, but @aaron.ballman should probably give his two cents as well for completion's sake.

This revision is now accepted and ready to land.Sep 29 2020, 4:43 AM

I'm going to land this as-is, based on Bevin's LGTM and my own confidence that the 10 lines I added are correct (enough). @aaron.ballman , please reopen this review if you have additional comments or concerns. Thanks.

Reverted due to another obscure test failure. Working to diagnose.

This revision is now accepted and ready to land.Sep 29 2020, 1:47 PM

Refreshed previously-reverted changeset and re-tested.

aaron.ballman accepted this revision.Jun 9 2021, 5:58 AM

LGTM! Thank you for the fix.

This revision was landed with ongoing or failed builds.Jun 11 2021, 8:36 AM
This revision was automatically updated to reflect the committed changes.

Reverted commit due to buildbot failures -- will update shortly.

This revision is now accepted and ready to land.Jun 11 2021, 9:18 AM

Corrected APSInt.toString() usage to comply with intervening change 61cdaf66fe22be2b5

This revision was landed with ongoing or failed builds.Jun 11 2021, 10:34 AM
This revision was automatically updated to reflect the committed changes.

I'm aware that this commit triggers one failure in ASan/Windows -- I have posted a patch for review to address that here: https://reviews.llvm.org/D104141