ziqingluo-90 created this revision.
ziqingluo-90 added reviewers: NoQ, jkorous, t-rasmud, malavikasamak.
Herald added a subscriber: ChuanqiXu.
Herald added a project: All.
ziqingluo-90 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Our analysis requires a complete view of the translation unit to be
conservative. As mentioned in this patch
<https://reviews.llvm.org/D143048#4173832>, we need to know if there is any
function overload of a function `f` declared after `f`. In addition, we later
may want to make global variables safe too. In such a case, we need to know if
a global variable is used somewhere in the translation unit. Moreover, the
analysis now can ignore ill-formed code detected at the end of a TU.
A summary of the change:
1. Adds a TU traversal function in `UnsafeBufferAnalysis` to traverse and
analyze each function definition;
2. Removes the old analysis entry in `AnalysisBasedWarnings.cpp`, which was
called by `Sema` at the end of parsing a function;
3. Creates a new analysis entry in `AnalysisBasedWarnings.cpp` for `Sema` to
call at the end of parsing a TU.
This patch is still work in progress as the existence of the following concerns:
1. Can we move everything in `AnalysisBasedWarnings.cpp` to `Sema`? So far
`AnalysisBasedWarnings` is used to bridge `Sema` and `UnsafeBufferAnalysis` so
that the changes are minimal.
2. We probably need a more efficient TU traversal implementation.
3. Current tests are mostly fine except that some notes with message "in
instantiation of ... " are missing. Although these notes are not emitted by
our analysis, we better understand why things change.
4. To test this patch on a branch with all ongoing [-Wunsafe-buffer-usage]
patches.
5. Maybe there are better solutions? (Looking for comments!)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D146342
Files:
clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
clang/include/clang/Sema/AnalysisBasedWarnings.h
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/lib/Sema/Sema.cpp
Index: clang/lib/Sema/Sema.cpp
===================================================================
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1426,6 +1426,8 @@
}
}
+ AnalysisWarnings.CheckUnsafeBufferUsage(Context.getTranslationUnitDecl());
+
// Check we've noticed that we're no longer parsing the initializer for every
// variable. If we miss cases, then at best we have a performance issue and
// at worst a rejects-valid bug.
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===================================================================
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -13,6 +13,7 @@
//===----------------------------------------------------------------------===//
#include "clang/Sema/AnalysisBasedWarnings.h"
+#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/EvaluatedExprVisitor.h"
@@ -2290,6 +2291,24 @@
S.Diag(D.Loc, D.PD);
}
+void clang::sema::AnalysisBasedWarnings::CheckUnsafeBufferUsage(
+ const TranslationUnitDecl *TU) {
+ if (!TU)
+ return; // This is unexpected, give up quietly.
+
+ DiagnosticsEngine &Diags = S.getDiagnostics();
+
+ // Emit unsafe buffer usage warnings and fixits.
+ if (!Diags.isIgnored(diag::warn_unsafe_buffer_operation, SourceLocation()) ||
+ !Diags.isIgnored(diag::warn_unsafe_buffer_variable, SourceLocation())) {
+ UnsafeBufferUsageReporter R(S);
+ checkUnsafeBufferUsageForTU(
+ TU, R, S,
+ /*EmitFixits=*/S.getDiagnostics().getDiagnosticOptions().ShowFixits &&
+ S.getLangOpts().CPlusPlus20);
+ }
+}
+
void clang::sema::AnalysisBasedWarnings::IssueWarnings(
sema::AnalysisBasedWarnings::Policy P, sema::FunctionScopeInfo *fscope,
const Decl *D, QualType BlockType) {
@@ -2518,16 +2537,6 @@
if (S.getLangOpts().CPlusPlus && !fscope->isCoroutine() && isNoexcept(FD))
checkThrowInNonThrowingFunc(S, FD, AC);
- // Emit unsafe buffer usage warnings and fixits.
- if (!Diags.isIgnored(diag::warn_unsafe_buffer_operation, D->getBeginLoc()) ||
- !Diags.isIgnored(diag::warn_unsafe_buffer_variable, D->getBeginLoc())) {
- UnsafeBufferUsageReporter R(S);
- checkUnsafeBufferUsage(
- D, R,
- /*EmitFixits=*/S.getDiagnostics().getDiagnosticOptions().ShowFixits &&
- S.getLangOpts().CPlusPlus20);
- }
-
// If none of the previous checks caused a CFG build, trigger one here
// for the logical error handler.
if (LogicalErrorHandler::hasActiveDiagnostics(Diags, D->getBeginLoc())) {
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===================================================================
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -7,10 +7,14 @@
//===----------------------------------------------------------------------===//
#include "clang/Analysis/Analyses/UnsafeBufferUsage.h"
+#include "clang/AST/Decl.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/DiagnosticSema.h"
#include "clang/Lex/Lexer.h"
#include "clang/Lex/Preprocessor.h"
+#include "clang/Sema/Sema.h"
#include "llvm/ADT/SmallVector.h"
#include <memory>
#include <optional>
@@ -1027,7 +1031,7 @@
return S;
}
-void clang::checkUnsafeBufferUsage(const Decl *D,
+void checkUnsafeBufferUsage(const Decl *D,
UnsafeBufferUsageHandler &Handler,
bool EmitFixits) {
assert(D && D->getBody());
@@ -1083,3 +1087,56 @@
}
}
}
+
+void clang::checkUnsafeBufferUsageForTU(const TranslationUnitDecl *TU,
+ UnsafeBufferUsageHandler &Handler,
+ clang::Sema &S, bool EmitFixits) {
+ DiagnosticsEngine &Diags = S.getDiagnostics();
+ static constexpr StringRef Tag = "Callable";
+
+ struct CallableFinderCallback : MatchFinder::MatchCallback {
+ UnsafeBufferUsageHandler &Handler;
+ DiagnosticsEngine &Diags;
+ clang::Sema &S;
+ const bool EmitFixits;
+
+ CallableFinderCallback(UnsafeBufferUsageHandler &Handler,
+ DiagnosticsEngine &Diags, clang::Sema &S,
+ bool EmitFixits)
+ : Handler(Handler), Diags(Diags), S(S), EmitFixits(EmitFixits) {}
+
+ void run(const MatchFinder::MatchResult &Result) override {
+ if (const auto *Callable = Result.Nodes.getNodeAs<Decl>(Tag)) {
+ // Do not do any analysis if we are going to just ignore them.
+ if (Diags.getIgnoreAllWarnings() ||
+ (Diags.getSuppressSystemWarnings() &&
+ S.SourceMgr.isInSystemHeader(Callable->getLocation())))
+ return;
+
+ // For code in dependent contexts, we'll do this at instantiation time.
+ if (cast<DeclContext>(Callable)->isDependentContext())
+ return;
+
+ if (S.hasUncompilableErrorOccurred())
+ return;
+
+ assert (Callable->getBody());
+
+ // Above checks and early returns are copied from
+ // `clang::sema::AnalysisBasedWarnings::IssueWarnings`.
+ checkUnsafeBufferUsage(Callable, Handler, EmitFixits);
+ }
+ }
+ };
+
+ MatchFinder Finder;
+ CallableFinderCallback CB(Handler, Diags, S, EmitFixits);
+
+ Finder.addMatcher(
+ decl(forEachDescendant(decl(anyOf(
+ functionDecl(hasBody(anything())).bind(Tag),
+ objcMethodDecl(isDefinition()).bind(Tag), blockDecl().bind(Tag))))),
+ &CB);
+ Finder.addMatcher(stmt(forEachDescendant(stmt(lambdaExpr().bind(Tag)))), &CB);
+ Finder.match(*TU, TU->getASTContext());
+}
Index: clang/include/clang/Sema/AnalysisBasedWarnings.h
===================================================================
--- clang/include/clang/Sema/AnalysisBasedWarnings.h
+++ clang/include/clang/Sema/AnalysisBasedWarnings.h
@@ -13,6 +13,7 @@
#ifndef LLVM_CLANG_SEMA_ANALYSISBASEDWARNINGS_H
#define LLVM_CLANG_SEMA_ANALYSISBASEDWARNINGS_H
+#include "clang/AST/Decl.h"
#include "llvm/ADT/DenseMap.h"
#include <memory>
@@ -95,6 +96,8 @@
void IssueWarnings(Policy P, FunctionScopeInfo *fscope,
const Decl *D, QualType BlockType);
+ void CheckUnsafeBufferUsage(const TranslationUnitDecl *D);
+
Policy getDefaultPolicy() { return DefaultPolicy; }
void PrintStats() const;
Index: clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
===================================================================
--- clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
+++ clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
@@ -16,6 +16,7 @@
#include "clang/AST/Decl.h"
#include "clang/AST/Stmt.h"
+#include "clang/Sema/Sema.h"
namespace clang {
@@ -51,10 +52,10 @@
}
};
-// This function invokes the analysis and allows the caller to react to it
-// through the handler class.
-void checkUnsafeBufferUsage(const Decl *D, UnsafeBufferUsageHandler &Handler,
- bool EmitFixits);
+
+void checkUnsafeBufferUsageForTU(const TranslationUnitDecl *TU,
+ UnsafeBufferUsageHandler &Handler,
+ clang::Sema &S, bool EmitFixits);
namespace internal {
// Tests if any two `FixItHint`s in `FixIts` conflict. Two `FixItHint`s
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
