This is an archive of the discontinued LLVM Phabricator instance.

[-Wunsafe-buffer-usage] Improving insertion of the [[clang::unsafe_buffer_usage]] attribute
AbandonedPublic

Authored by ziqingluo-90 on May 10 2023, 6:20 PM.

Details

Summary

For fix-its that insert [[clang::unsafe_buffer_usage]] attributes, they will lookup existing macros defined for [[clang::unsafe_buffer_usage]] and use the (last defined such) macro directly. Fix-its will use raw [[clang::unsafe_buffer_usage]] if no such macro is defined.

For example,

// Suppose `foo` uses `p` in an unsafe way...

void foo(int *p); // A fix-it will insert "[[clang::unsafe_buffer_usage]] " right before the declaration as there is no macro for the attribute defined here

#define MACRO [[clang::unsafe_buffer_usage]]

void foo(int *p); // A fix-it will insert "MACRO " right before the declaration

The implementation mimics how a similar machine for the [[fallthrough]] attribute was implemented. (Mentioned here https://reviews.llvm.org/D143048#inline-1435398 )

Diff Detail

Event Timeline

ziqingluo-90 created this revision.May 10 2023, 6:20 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 10 2023, 6:20 PM
Herald added a subscriber: jdoerfert. · View Herald Transcript
ziqingluo-90 requested review of this revision.May 10 2023, 6:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 10 2023, 6:21 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
ziqingluo-90 retitled this revision from [-Wunsafe-buffer-usage][WIP] Improving insertion of the [[clang::unsafe_buffer_usage]] attribute to [-Wunsafe-buffer-usage] Improving insertion of the [[clang::unsafe_buffer_usage]] attribute .Jun 15 2023, 3:13 PM
NoQ accepted this revision.Jun 29 2023, 3:32 PM

Looks good to me! This doesn't take care of the GNU syntax (__attribute__((unsafe_buffer_usage))), but this probably isn't a big deal as people can simply use the modern C++ syntax in any code for which we actually emit these fixits.

This revision is now accepted and ready to land.Jun 29 2023, 3:32 PM
This revision was landed with ongoing or failed builds.Jul 14 2023, 2:48 PM
This revision was automatically updated to reflect the committed changes.
chapuni reopened this revision.Jul 14 2023, 10:03 PM
chapuni added a subscriber: chapuni.

Excuse me, I have reverted this due to circular deps.
clangAnalysis should not depend on clangSema.

clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
19

This #include may be avoided since you declare class clang::Sema.

This revision is now accepted and ready to land.Jul 14 2023, 10:03 PM
chapuni requested changes to this revision.Jul 14 2023, 10:04 PM
This revision now requires changes to proceed.Jul 14 2023, 10:04 PM

Excuse me, I have reverted this due to circular deps.
clangAnalysis should not depend on clangSema.

Thank you @chapuni , I will fix it soon.

Re-landed in a07a6f6c74a03405eccdcd3832acb2187d8b9c21

Moved the use of clang::Sema from UnsafeBufferAnalysis to AnalysisBasedWarnings.

NoQ added a comment.Jul 20 2023, 6:52 PM

Looks like the patch has landed, let's close the revision? 🤔

NoQ accepted this revision.Jul 20 2023, 6:53 PM
ziqingluo-90 abandoned this revision.Jul 24 2023, 11:19 AM

Re-landed in a07a6f6c74a03405eccdcd3832acb2187d8b9c21

Moved the use of clang::Sema from UnsafeBufferAnalysis to AnalysisBasedWarnings.

@NoQ Thanks for accepting it again, I'm closing this revision.