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; + + /// Mark a new "-Wunsafe-buffer-usage" opt-out region and set its' start + /// location to `StartLoc` + void setSafeBufferOptOutStartLoc(const SourceLocation &StartLoc); + + /// Set the end location of the current "-Wunsafe-buffer-usage" opt-out region + /// to `EndLoc` + 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 @@ -943,4 +943,15 @@ } +def err_pp_double_begin_pragma_unsafe_buffer_usage : +Error<"already inside '#pragma unsafe_buffer_usage'">; + +def err_pp_unmatched_end_begin_pragma_unsafe_buffer_usage : +Error<"not currently inside '#pragma unsafe_buffer_usage'">; + +def err_pp_unclosed_pragma_unsafe_buffer_usage : +Error<"'#pragma unsafe_buffer_usage' was not ended">; + +def err_pp_pragma_unsafe_buffer_usage_syntax : +Error<"Expected 'begin' or 'end'">; } Index: clang/include/clang/Lex/Preprocessor.h =================================================================== --- clang/include/clang/Lex/Preprocessor.h +++ clang/include/clang/Lex/Preprocessor.h @@ -2688,6 +2688,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; // It is used to report the start location of an never-closed region. + +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 @@ -16,6 +16,11 @@ using namespace clang; using namespace ast_matchers; +// Returns true iff `Loc` is in a safe-buffer opt-out region +bool isSafeBufferOptOut(const SourceManager &SM, const SourceLocation &Loc) { + return SM.getDiagnostics().isSafeBufferOptOut(SM, Loc); +} + namespace clang::ast_matchers { // A `RecursiveASTVisitor` that traverses all descendants of a given node "n" // except for those belonging to a different callable of "n". @@ -114,6 +119,12 @@ MatchDescendantVisitor Visitor(&DTM, Finder, Builder, ASTMatchFinder::BK_All); return Visitor.findMatch(DynTypedNode::create(Node)); } + +// Matches a `Stmt` node iff the node is in a safe-buffer opt-out region +AST_MATCHER(Stmt, notInSafeBufferOptOut) { + const SourceManager &SM = Finder->getASTContext().getSourceManager(); + return !isSafeBufferOptOut(SM, Node.getBeginLoc()); +} } // namespace clang::ast_matchers namespace { @@ -529,7 +540,7 @@ stmt(anyOf( // Add Gadget::matcher() for every gadget in the registry. #define GADGET(x) \ - x ## Gadget::matcher().bind(#x), + allOf(x ## Gadget::matcher().bind(#x), notInSafeBufferOptOut()), #include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def" // In parallel, match all DeclRefExprs so that to find out // whether there are any uncovered by gadgets. @@ -537,7 +548,11 @@ // Also match DeclStmts because we'll need them when fixing // their underlying VarDecls that otherwise don't have // any backreferences to DeclStmts. - declStmt().bind("any_ds") + allOf(declStmt().bind("any_ds"), notInSafeBufferOptOut()) + // We match all DREs regardless of whether they are in safe-buffer + // opt-out region. Because an unclaimed DRE 'd', regardless of where it is, + // should prevent a Fixable associated to the same variable as 'd' + // from being emitting. )) // FIXME: Idiomatically there should be a forCallable(equalsNode(D)) // here, to make sure that the statement actually belongs to the 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 @@ -332,6 +332,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::err_pp_unclosed_pragma_unsafe_buffer_usage); + } // 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 @@ -1242,6 +1242,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::err_pp_pragma_unsafe_buffer_usage_syntax); + return; + } + + IdentifierInfo *II = Tok.getIdentifierInfo(); + SourceLocation Loc = Tok.getLocation(); + + if (II->isStr("begin")) { + if (PP.enterOrExitSafeBufferOptOutRegion(true, Loc)) + PP.Diag(Loc, diag::err_pp_double_begin_pragma_unsafe_buffer_usage); + } else if (II->isStr("end")) { + if (PP.enterOrExitSafeBufferOptOutRegion(false, Loc)) + PP.Diag(Loc, diag::err_pp_unmatched_end_begin_pragma_unsafe_buffer_usage); + } else + PP.Diag(Tok, diag::err_pp_pragma_unsafe_buffer_usage_syntax); + } +}; + /// PragmaDiagnosticHandler - e.g. '\#pragma GCC diagnostic ignored "-Wformat"' struct PragmaDiagnosticHandler : public PragmaHandler { private: @@ -2127,6 +2153,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 @@ -1461,6 +1461,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/test/SemaCXX/warn-unsafe-buffer-usage-pragma-misuse.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma-misuse.cpp @@ -0,0 +1,33 @@ +// 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-error{{already inside '#pragma unsafe_buffer_usage'}} + x++; +#pragma clang unsafe_buffer_usage end +} + +void endUnopened(int *x) { +#pragma clang unsafe_buffer_usage end // expected-error{{not currently inside '#pragma unsafe_buffer_usage'}} + +#pragma clang unsafe_buffer_usage begin + x++; +#pragma clang unsafe_buffer_usage end +} + +void wrongOption() { +#pragma clang unsafe_buffer_usage start // expected-error{{Expected 'begin' or 'end'}} +#pragma clang unsafe_buffer_usage close // expected-error{{Expected 'begin' or 'end'}} +} + +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-error{{'#pragma unsafe_buffer_usage' was not ended}} +} Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma.h =================================================================== --- /dev/null +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma.h @@ -0,0 +1,14 @@ +#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 +p2++; // expected-note{{used in pointer arithmetic here}} +#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,100 @@ +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -Wno-unused-value -verify %s + +void basic(int * x) { // expected-warning{{'x' is an unsafe pointer used for buffer access}} + int *p1 = new int[10]; // not to warn + int *p2 = new int[10]; // expected-warning{{'p2' is an unsafe pointer used for buffer access}} + +#pragma clang unsafe_buffer_usage begin + p1[5]; // not to warn + +#define _INCLUDE_NO_WARN +#include "warn-unsafe-buffer-usage-pragma.h" // increment p1 in header + + int *p3 = new int[10]; // expected-warning{{'p3' is an unsafe pointer used for buffer access}} + +#pragma clang unsafe_buffer_usage end + p2[5]; //expected-note{{used in buffer access here}} + p3[5]; //expected-note{{used in buffer access here}} + x++; //expected-note{{used in pointer arithmetic here}} +#define _INCLUDE_WARN +#include "warn-unsafe-buffer-usage-pragma.h" // increment p2 in header +} + + +void withDiagnosticWarning() { + int *p1 = new int[10]; // not to warn + int *p2 = new int[10]; // expected-warning{{'p2' is an unsafe pointer used for buffer access}} + + // 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{{used in buffer access here}} +} + + +void withDiagnosticIgnore() { + int *p1 = new int[10]; // not to warn + int *p2 = new int[10]; // expected-warning{{'p2' is an unsafe pointer used for buffer access}} + int *p3 = new int[10]; // expected-warning{{'p3' is an unsafe pointer used for buffer access}} + +#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{{used in buffer access here}} + +#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{{used in buffer access here}} +#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 +}