ziqingluo-90 updated this revision to Diff 483645.
ziqingluo-90 added a comment.
Addressing comments.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140179/new/
https://reviews.llvm.org/D140179
Files:
clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
clang/include/clang/Basic/Diagnostic.h
clang/include/clang/Basic/DiagnosticLexKinds.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Lex/Preprocessor.h
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/lib/Basic/Diagnostic.cpp
clang/lib/Lex/PPLexerChange.cpp
clang/lib/Lex/Pragma.cpp
clang/lib/Lex/Preprocessor.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma-misuse.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma.h
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
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,34 @@
+// 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/lib/Sema/AnalysisBasedWarnings.cpp
===================================================================
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2150,15 +2150,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<const Stmt *> &&UnsafeUses) override {
+ const SourceManager &SM = S.getSourceManager();
+ const DiagnosticsEngine &DE = S.getDiagnostics();
+ std::vector<const Stmt *> 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/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/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:
@@ -2118,6 +2144,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/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/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<SourceLocation, SourceLocation> &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/Analysis/UnsafeBufferUsage.cpp
===================================================================
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -728,7 +728,12 @@
for (auto &&Fixit : VarF)
Fixes->push_back(std::move(Fixit));
- Handler.handleFixableVariable(VD, std::move(*Fixes));
+ std::vector<const Stmt *> 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/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/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11739,4 +11739,7 @@
InGroup<UnsafeBufferUsage>, DefaultIgnore;
def warn_unsafe_buffer_variable : Warning<"variable %0 participates in unchecked buffer operations">,
InGroup<UnsafeBufferUsage>, DefaultIgnore;
+
+def note_unsafe_buffer_usage :
+ Note<"unchecked buffer operation using variable %0">;
} // end of sema component.
Index: clang/include/clang/Basic/DiagnosticLexKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticLexKinds.td
+++ clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -940,4 +940,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/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<std::pair<SourceLocation, SourceLocation>, 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/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<const Stmt*> &&UnsafeUsages) = 0;
};
// This function invokes the analysis and allows the caller to react to it
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
