This is an archive of the discontinued LLVM Phabricator instance.

[-Wunsafe-buffer-usage] Match unsafe pointers being casted to bool or participating in pointer subtractions
ClosedPublic

Authored by ziqingluo-90 on Feb 14 2023, 6:15 PM.

Details

Summary

To add two new unique cases to the Unspecified Pointer Context (UPC):

  • A pointer being casted to a boolean value is in a UPC;
  • A pointer participating in pointer subtraction is in a UPC.

Diff Detail

Event Timeline

ziqingluo-90 created this revision.Feb 14 2023, 6:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 14 2023, 6:15 PM
ziqingluo-90 requested review of this revision.Feb 14 2023, 6:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 14 2023, 6:15 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
ziqingluo-90 retitled this revision from [WIP][-Wunsafe-buffer-usage] Match unsafe pointers being casted to bool or participating in pointer subtractions to [-Wunsafe-buffer-usage] Match unsafe pointers being casted to bool or participating in pointer subtractions.Feb 22 2023, 2:38 PM
malavikasamak added inline comments.Mar 8 2023, 11:55 AM
clang/lib/Analysis/UnsafeBufferUsage.cpp
168

I don't understand why we are special handling only pointer subtraction here. Won't pointer addition be also considered UPC? If so, can we just add it to this patch.

malavikasamak added inline comments.Mar 8 2023, 1:58 PM
clang/lib/Analysis/UnsafeBufferUsage.cpp
168

Never mind. It looks like pointer addition is not even legal and subtraction is a special case.

NoQ added a comment.Mar 8 2023, 2:27 PM

Looks great overall!

clang/lib/Analysis/UnsafeBufferUsage.cpp
168

I think this is about operations between two pointers, like p - q. You can subtract a pointer from a pointer (it gives you the distance between points in memory) but you can't add a pointer to a pointer (it makes no sense).

But yeah, could be clarified.

188–191

Tabs vs. spaces.

clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-addressof-arraysubscript.cpp
96

I think the original CHECK-NOT was better because it allows you to write more tests below it. It's good to have individual test cases self-contained and not affect the rest of the file.

ziqingluo-90 marked 2 inline comments as done.

Address comments

ziqingluo-90 marked 3 inline comments as done.Mar 20 2023, 2:49 PM
malavikasamak accepted this revision.Apr 10 2023, 11:30 AM
This revision is now accepted and ready to land.Apr 10 2023, 11:30 AM
NoQ accepted this revision.Apr 10 2023, 12:58 PM

LGTM!

clang/lib/Analysis/UnsafeBufferUsage.cpp
188–191

Still a bit off!

This revision was landed with ongoing or failed builds.Apr 11 2023, 3:10 PM
This revision was automatically updated to reflect the committed changes.