Index: clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h =================================================================== --- clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h +++ clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h @@ -34,7 +34,8 @@ /// Invoked when a fix is suggested against a variable. virtual void handleFixableVariable(const VarDecl *Variable, - FixItList &&List) = 0; + FixItList &&List, + std::vector &&UnsafeUsages) = 0; }; // This function invokes the analysis and allows the caller to react to it Index: clang/include/clang/Basic/Diagnostic.h =================================================================== --- clang/include/clang/Basic/Diagnostic.h +++ clang/include/clang/Basic/Diagnostic.h @@ -1037,6 +1037,24 @@ return Diags->ProcessDiag(*this); } + // An ordered sequence of "-Wunsafe-buffer-usage" opt-out regions in one + // translation unit. Each region is represented by a pair of start and end + // locations. + SmallVector, 8> SafeBufferOptOutMap; + +public: + /// \return true iff the given `Loc` is in a "-Wunsafe-buffer-usage" opt-out + /// region + bool isSafeBufferOptOut(const SourceManager&SourceMgr, const SourceLocation &Loc) const; + + /// Assuming regions are used correctly, saving the start location of a + /// "-Wunsafe-buffer-usage" opt-out region. + void setSafeBufferOptOutStartLoc(const SourceLocation &StartLoc); + + /// Assuming regions are used correctly, saving the end location of a + /// "-Wunsafe-buffer-usage" opt-out region. + void setSafeBufferOptOutEndLoc(const SourceLocation &EndLoc); + /// @name Diagnostic Emission /// @{ protected: Index: clang/include/clang/Basic/DiagnosticLexKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticLexKinds.td +++ clang/include/clang/Basic/DiagnosticLexKinds.td @@ -940,4 +940,15 @@ } +def warn_pragma_unsafe_buffer_usage_misorder : +ExtWarn<"pragma unsafe_buffer_usage used in misorder">, + InGroup; + +def warn_pragma_unsafe_buffer_usage_unclosed : +ExtWarn<"pragma unsafe_buffer_usage is never closed">, + InGroup; + +def warn_pragma_unsafe_buffer_usage_unknown_option : +ExtWarn<"pragma unsafe_buffer_usage only accepts \"begin\" or \"end\" option">, + InGroup; } Index: clang/include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -11739,4 +11739,7 @@ InGroup, DefaultIgnore; def warn_unsafe_buffer_variable : Warning<"variable %0 participates in unchecked buffer operations">, InGroup, DefaultIgnore; + +def note_unsafe_buffer_usage : + Note<"unchecked buffer operation using variable %0">; } // end of sema component. Index: clang/include/clang/Lex/Preprocessor.h =================================================================== --- clang/include/clang/Lex/Preprocessor.h +++ clang/include/clang/Lex/Preprocessor.h @@ -2690,6 +2690,41 @@ void emitMacroDeprecationWarning(const Token &Identifier) const; void emitRestrictExpansionWarning(const Token &Identifier) const; void emitFinalMacroWarning(const Token &Identifier, bool IsUndef) const; + + /// This boolean state keeps track if the current scanned token (by this PP) + /// is in an "-Wunsafe-buffer-usage" opt-out region. Assuming PP scans a + /// translation unit in a linear order. + bool InSafeBufferOptOutRegion = 0; + + /// Hold the start location of the current "-Wunsafe-buffer-usage" opt-out + /// region if PP is currently in such a region. Hold undefined value + /// otherwise. + SourceLocation CurrentSafeBufferOptOutStart; + +public: + /// Alter the state of whether this PP currently is in a + /// "-Wunsafe-buffer-usage" opt-out region. + /// + /// \param isEnter: true if this PP is entering a region; otherwise, this PP + /// is exiting a region + /// \param Loc: the location of the entry or exit of a + /// region + /// \return true iff it is INVALID to enter or exit a region, i.e., + /// attempt to enter a region before exiting a previous region, or exiting a + /// region that PP is not currently in. + bool enterOrExitSafeBufferOptOutRegion(bool isEnter, + const SourceLocation &Loc); + + /// \return true iff this PP is currently in a "-Wunsafe-buffer-usage" + /// opt-out region + bool isInSafeBufferOptOutRegion(); + + /// \param StartLoc: output argument. It will be set to the start location of + /// the current "-Wunsafe-buffer-usage" opt-out region iff this function + /// returns true. + /// \return true iff this PP is currently in a "-Wunsafe-buffer-usage" + /// opt-out region + bool isInSafeBufferOptOutRegion(SourceLocation &StartLoc); }; /// Abstract base class that describes a handler that will receive Index: clang/lib/Analysis/UnsafeBufferUsage.cpp =================================================================== --- clang/lib/Analysis/UnsafeBufferUsage.cpp +++ clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -8,7 +8,9 @@ #include "clang/Analysis/Analyses/UnsafeBufferUsage.h" #include "clang/AST/RecursiveASTVisitor.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/Support/raw_ostream.h" using namespace llvm; using namespace clang; @@ -728,7 +730,12 @@ for (auto &&Fixit : VarF) Fixes->push_back(std::move(Fixit)); - Handler.handleFixableVariable(VD, std::move(*Fixes)); + std::vector UnsafeUses; + + for (const Gadget *GD : Map.lookup(VD)) + UnsafeUses.push_back(GD->getBaseStmt()); + Handler.handleFixableVariable(VD, std::move(*Fixes), + std::move(UnsafeUses)); } else { // The strategy has failed. Emit the warning without the fixit. S.set(VD, Strategy::Kind::Wontfix); Index: clang/lib/Basic/Diagnostic.cpp =================================================================== --- clang/lib/Basic/Diagnostic.cpp +++ clang/lib/Basic/Diagnostic.cpp @@ -559,6 +559,64 @@ return Emitted; } +bool DiagnosticsEngine::isSafeBufferOptOut(const SourceManager &SourceMgr, + const SourceLocation &Loc) const { + // Try to find a region in `SafeBufferOptOutMap` where `Loc` is in: + auto FirstRegionEndingAfterLoc = llvm::partition_point( + SafeBufferOptOutMap, + [&SourceMgr, + &Loc](const std::pair &Region) { + return SourceMgr.isBeforeInTranslationUnit(Region.second, Loc); + }); + + if (FirstRegionEndingAfterLoc != SafeBufferOptOutMap.end()) { + // To test if the start location of the found region precedes `Loc`: + return SourceMgr.isBeforeInTranslationUnit(FirstRegionEndingAfterLoc->first, + Loc); + } + // If we do not find a region whose end location passes `Loc`, we want to + // check if the current region is still open: + if (!SafeBufferOptOutMap.empty() && + SafeBufferOptOutMap.back().first == SafeBufferOptOutMap.back().second) + return SourceMgr.isBeforeInTranslationUnit(SafeBufferOptOutMap.back().first, + Loc); + return false; +} + +void DiagnosticsEngine::setSafeBufferOptOutStartLoc( + const SourceLocation &StartLoc) { + if (!SafeBufferOptOutMap.empty()) { + auto *PrevRegion = &SafeBufferOptOutMap.back(); + + // To check if the pre-condition is satisfied: + assert(PrevRegion->first != PrevRegion->second && + "Shall not begin a safe buffer opt-out region before closing the " + "previous one."); + assert(!hasSourceManager() || getSourceManager().isBeforeInTranslationUnit( + PrevRegion->second, StartLoc) && + "Misordered safe buffer opt-out regions"); + } + // If the start location equals to the end location, we call the region a open + // region or a unclosed region (i.e., end location has not been set yet). + SafeBufferOptOutMap.emplace_back(StartLoc, StartLoc); +} + +void DiagnosticsEngine::setSafeBufferOptOutEndLoc( + const SourceLocation &EndLoc) { + assert(!SafeBufferOptOutMap.empty() && + "Misordered safe buffer opt-out regions"); + + auto *CurrRegion = &SafeBufferOptOutMap.back(); + + // To check if the pre-condition is satisfied: + assert(CurrRegion->first == CurrRegion->second && + "Set end location to a closed safe buffer opt-out region"); + assert(!hasSourceManager() || getSourceManager().isBeforeInTranslationUnit( + CurrRegion->first, EndLoc) && + "Misordered safe buffer opt-out regions"); + CurrRegion->second = EndLoc; +} + DiagnosticConsumer::~DiagnosticConsumer() = default; void DiagnosticConsumer::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, Index: clang/lib/Lex/PPLexerChange.cpp =================================================================== --- clang/lib/Lex/PPLexerChange.cpp +++ clang/lib/Lex/PPLexerChange.cpp @@ -11,6 +11,7 @@ // //===----------------------------------------------------------------------===// +#include "clang/Basic/DiagnosticSema.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/SourceManager.h" #include "clang/Lex/HeaderSearch.h" @@ -332,6 +333,15 @@ assert(!CurTokenLexer && "Ending a file when currently in a macro!"); + SourceLocation UnclosedSafeBufferOptOutLoc; + + if (IncludeMacroStack.empty() && + isInSafeBufferOptOutRegion(UnclosedSafeBufferOptOutLoc)) { + // To warn if a "-Wunsafe-buffer-usage" opt-out region is still open by the + // end of a file. + Diag(UnclosedSafeBufferOptOutLoc, + diag::warn_pragma_unsafe_buffer_usage_unclosed); + } // If we have an unclosed module region from a pragma at the end of a // module, complain and close it now. const bool LeavingSubmodule = CurLexer && CurLexerSubmodule; Index: clang/lib/Lex/Pragma.cpp =================================================================== --- clang/lib/Lex/Pragma.cpp +++ clang/lib/Lex/Pragma.cpp @@ -43,6 +43,7 @@ #include "llvm/Support/Compiler.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/Timer.h" +#include "llvm/Support/raw_ostream.h" #include #include #include @@ -1242,6 +1243,32 @@ #endif }; +struct PragmaUnsafeBufferUsageHandler : public PragmaHandler { + PragmaUnsafeBufferUsageHandler() : PragmaHandler("unsafe_buffer_usage") {} + void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer, + Token &FirstToken) override { + Token Tok; + + PP.LexUnexpandedToken(Tok); + if (Tok.isNot(tok::identifier)) { + PP.Diag(Tok, diag::warn_pragma_unsafe_buffer_usage_unknown_option); + return; + } + + IdentifierInfo *II = Tok.getIdentifierInfo(); + SourceLocation Loc = Tok.getLocation(); + + if (II->isStr("begin")) { + if (PP.enterOrExitSafeBufferOptOutRegion(true, Loc)) + PP.Diag(Loc, diag::warn_pragma_unsafe_buffer_usage_misorder); + } else if (II->isStr("end")) { + if (PP.enterOrExitSafeBufferOptOutRegion(false, Loc)) + PP.Diag(Loc, diag::warn_pragma_unsafe_buffer_usage_misorder); + } else + PP.Diag(Tok, diag::warn_pragma_unsafe_buffer_usage_unknown_option); + } +}; + /// PragmaDiagnosticHandler - e.g. '\#pragma GCC diagnostic ignored "-Wformat"' struct PragmaDiagnosticHandler : public PragmaHandler { private: @@ -2118,6 +2145,9 @@ ModuleHandler->AddPragma(new PragmaModuleBuildHandler()); ModuleHandler->AddPragma(new PragmaModuleLoadHandler()); + // Safe Buffers pragmas + AddPragmaHandler("clang", new PragmaUnsafeBufferUsageHandler); + // Add region pragmas. AddPragmaHandler(new PragmaRegionHandler("region")); AddPragmaHandler(new PragmaRegionHandler("endregion")); Index: clang/lib/Lex/Preprocessor.cpp =================================================================== --- clang/lib/Lex/Preprocessor.cpp +++ clang/lib/Lex/Preprocessor.cpp @@ -26,6 +26,7 @@ #include "clang/Lex/Preprocessor.h" #include "clang/Basic/Builtins.h" +#include "clang/Basic/Diagnostic.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/FileSystemStatCache.h" #include "clang/Basic/IdentifierTable.h" @@ -1461,6 +1462,31 @@ Diag(*A.FinalAnnotationLoc, diag::note_pp_macro_annotation) << 2; } +bool Preprocessor::enterOrExitSafeBufferOptOutRegion( + bool isEnter, const SourceLocation &Loc) { + if (isEnter) { + if (isInSafeBufferOptOutRegion()) + return true; // invalid enter action + InSafeBufferOptOutRegion = true; + CurrentSafeBufferOptOutStart = Loc; + getDiagnostics().setSafeBufferOptOutStartLoc(Loc); + } else { + if (!isInSafeBufferOptOutRegion()) + return true; // invalid enter action + InSafeBufferOptOutRegion = false; + getDiagnostics().setSafeBufferOptOutEndLoc(Loc); + } + return false; +} + +bool Preprocessor::isInSafeBufferOptOutRegion() { + return InSafeBufferOptOutRegion; +} +bool Preprocessor::isInSafeBufferOptOutRegion(SourceLocation &StartLoc) { + StartLoc = CurrentSafeBufferOptOutStart; + return InSafeBufferOptOutRegion; +} + ModuleLoader::~ModuleLoader() = default; CommentHandler::~CommentHandler() = default; Index: clang/lib/Sema/AnalysisBasedWarnings.cpp =================================================================== --- clang/lib/Sema/AnalysisBasedWarnings.cpp +++ clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -33,6 +33,8 @@ #include "clang/Analysis/AnalysisDeclContext.h" #include "clang/Analysis/CFG.h" #include "clang/Analysis/CFGStmtMap.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/DiagnosticSema.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Lex/Preprocessor.h" @@ -41,9 +43,11 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/BitVector.h" #include "llvm/ADT/MapVector.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" +#include "llvm/ADT/iterator_range.h" #include "llvm/Support/Casting.h" #include #include @@ -2150,15 +2154,31 @@ UnsafeBufferUsageReporter(Sema &S) : S(S) {} void handleUnsafeOperation(const Stmt *Operation) override { - S.Diag(Operation->getBeginLoc(), diag::warn_unsafe_buffer_expression); + if (!S.getDiagnostics().isSafeBufferOptOut(S.getSourceManager(), + Operation->getBeginLoc())) + S.Diag(Operation->getBeginLoc(), diag::warn_unsafe_buffer_expression); } - void handleFixableVariable(const VarDecl *Variable, - FixItList &&Fixes) override { + void handleFixableVariable(const VarDecl *Variable, FixItList &&Fixes, + std::vector &&UnsafeUses) override { + const SourceManager &SM = S.getSourceManager(); + const DiagnosticsEngine &DE = S.getDiagnostics(); + std::vector UnsafeUsesToReport; + + for (auto UnsafeUse : UnsafeUses) + if (!DE.isSafeBufferOptOut(SM, UnsafeUse->getBeginLoc())) + UnsafeUsesToReport.push_back(UnsafeUse); + + if (UnsafeUsesToReport.empty()) + return; // no unsafe use should be reported, bye + const auto &D = - S.Diag(Variable->getBeginLoc(), diag::warn_unsafe_buffer_variable); - D << Variable; - for (const auto &F: Fixes) { + S.Diag(Variable->getBeginLoc(), diag::warn_unsafe_buffer_variable) + << Variable; + for (auto UnsafeUse : UnsafeUsesToReport) + S.Diag(UnsafeUse->getBeginLoc(), diag::note_unsafe_buffer_usage) + << Variable; + for (const auto &F : Fixes) { D << F; } } Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma-misuse.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma-misuse.cpp @@ -0,0 +1,36 @@ +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -verify %s + +void beginUnclosed(int * x) { +#pragma clang unsafe_buffer_usage begin + +#pragma clang unsafe_buffer_usage begin // expected-warning{{pragma unsafe_buffer_usage used in misorder}} + x++; +#pragma clang unsafe_buffer_usage end + x++; // expected-warning{{unchecked operation on raw buffer in expression}} +} + +void endUnopened(int *x) { +#pragma clang unsafe_buffer_usage end // expected-warning{{pragma unsafe_buffer_usage used in misorder}} + +#pragma clang unsafe_buffer_usage begin + x++; +#pragma clang unsafe_buffer_usage end + x++; // expected-warning{{unchecked operation on raw buffer in expression}} +} + +void wrongOption() { +#pragma clang unsafe_buffer_usage start // expected-warning{{pragma unsafe_buffer_usage only accepts "begin" or "end" option}} +#pragma clang unsafe_buffer_usage close // expected-warning{{pragma unsafe_buffer_usage only accepts "begin" or "end" option}} +} + +void unclosed(int * p1) { +#pragma clang unsafe_buffer_usage begin +// End of the included file will not raise the unclosed region warning: +#define _INCLUDE_NO_WARN +#include "warn-unsafe-buffer-usage-pragma.h" +#pragma clang unsafe_buffer_usage end + +// End of this file raises the warning: +#pragma clang unsafe_buffer_usage begin // expected-warning{{pragma unsafe_buffer_usage is never closed}} +} + Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma.h =================================================================== --- /dev/null +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma.h @@ -0,0 +1,16 @@ +#ifdef _INCLUDE_NO_WARN +// the snippet will be included in an opt-out region + +p1++; + +#undef _INCLUDE_NO_WARN + +#elif defined(_INCLUDE_WARN) +// the snippet will be included in a location where warnings are expected + +p1++; // expected-warning{{unchecked operation on raw buffer in expression}} +#undef _INCLUDE_WARN + +#else + +#endif Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma.cpp @@ -0,0 +1,106 @@ +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -Wno-unused-value -verify %s + +void basic(int * x) { // TODO: to warn formal parameters + int *p1 = new int[10]; // not to warn + int *p2 = new int[10]; // expected-warning{{variable 'p2' participates in unchecked buffer operations}} + +#pragma clang unsafe_buffer_usage begin + p1[5]; // not to warn + +#define _INCLUDE_NO_WARN +#include "warn-unsafe-buffer-usage-pragma.h" + + int *p3 = new int[10]; // expected-warning{{variable 'p3' participates in unchecked buffer operations}} + +#pragma clang unsafe_buffer_usage end + p2[5]; // expected-note{{unchecked buffer operation using variable 'p2'}} + p3[5]; // expected-note{{unchecked buffer operation using variable 'p3'}} + x++; // expected-warning{{unchecked operation on raw buffer in expression}} +#define _INCLUDE_WARN +#include "warn-unsafe-buffer-usage-pragma.h" +} + + +void withDiagnosticWarning() { + int *p1 = new int[10]; // not to warn + int *p2 = new int[10]; // expected-warning{{variable 'p2' participates in unchecked buffer operations}} + + // diagnostics in opt-out region +#pragma clang unsafe_buffer_usage begin + p1[5]; // not to warn + p2[5]; // not to warn +#pragma clang diagnostic push +#pragma clang diagnostic warning "-Wunsafe-buffer-usage" + p1[5]; // not to warn + p2[5]; // not to warn +#pragma clang diagnostic warning "-Weverything" + p1[5]; // not to warn expected-warning{{expression result unused}} + p2[5]; // not to warn expected-warning{{expression result unused}} +#pragma clang diagnostic pop +#pragma clang unsafe_buffer_usage end + + // opt-out region under diagnostic warning +#pragma clang diagnostic push +#pragma clang diagnostic warning "-Wunsafe-buffer-usage" +#pragma clang unsafe_buffer_usage begin + p1[5]; // not to warn + p2[5]; // not to warn +#pragma clang unsafe_buffer_usage end +#pragma clang diagnostic pop + + p2[5]; // expected-note{{unchecked buffer operation using variable 'p2'}} +} + + +void withDiagnosticIgnore() { + int *p1 = new int[10]; // not to warn + int *p2 = new int[10]; // expected-warning{{variable 'p2' participates in unchecked buffer operations}} + int *p3 = new int[10]; // expected-warning{{variable 'p3' participates in unchecked buffer operations}} + +#pragma clang unsafe_buffer_usage begin + p1[5]; // not to warn + p2[5]; // not to warn +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunsafe-buffer-usage" + p1[5]; // not to warn + p2[5]; // not to warn +#pragma clang diagnostic ignored "-Weverything" + p1[5]; // not to warn + p2[5]; // not to warn +#pragma clang diagnostic pop +#pragma clang unsafe_buffer_usage end + +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunsafe-buffer-usage" +#pragma clang unsafe_buffer_usage begin + p1[5]; // not to warn + p2[5]; // not to warn +#pragma clang unsafe_buffer_usage end +#pragma clang diagnostic pop + + p2[5]; // expected-note{{unchecked buffer operation using variable 'p2'}} + +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunsafe-buffer-usage" +#pragma clang unsafe_buffer_usage begin + p1[5]; // not to warn + p2[5]; // not to warn +#pragma clang unsafe_buffer_usage end + p3[5]; // expected-note{{unchecked buffer operation using variable 'p3'}} +#pragma clang diagnostic pop +} + +void noteGoesWithVarDeclWarning() { +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunsafe-buffer-usage" + int *p = new int[10]; // not to warn +#pragma clang diagnostic pop + + p[5]; // not to note since the associated warning is suppressed +} + +#define HASH # +#define INC(x) \ + HASH ## pragma clang unsafe_buffer_usage begin \ + (x)++; \ + HASH ## pragma clang unsafe_buffer_usage end