[clang] 6d861d4 - [-Wunsafe-buffer-usage] Move the whole analysis to the end of a translation unit
via cfe-commits
cfe-commits at lists.llvm.org
Fri May 12 11:52:44 PDT 2023
Author: ziqingluo-90
Date: 2023-05-12T11:50:51-07:00
New Revision: 6d861d498de1320d22771c329ec69f9419ef06b7
URL: https://github.com/llvm/llvm-project/commit/6d861d498de1320d22771c329ec69f9419ef06b7
DIFF: https://github.com/llvm/llvm-project/commit/6d861d498de1320d22771c329ec69f9419ef06b7.diff
LOG: [-Wunsafe-buffer-usage] Move the whole analysis to the end of a translation unit
The unsafe-buffer analysis requires a complete view of the translation
unit (TU) to be conservative. So the analysis is moved to the end of a
TU.
A summary of changes made: add a new `IssueWarnings` function in
`AnalysisBasedWarnings.cpp` for TU-based analyses. So far
[-Wunsafe-buffer-usage] is the only analysis using it but there could
be more. `Sema` will call the new `IssueWarnings` function at the end
of parsing a TU.
Reviewed by: NoQ (Artem Dergachev)
Differential revision: https://reviews.llvm.org/D146342
Added:
Modified:
clang/include/clang/Sema/AnalysisBasedWarnings.h
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/lib/Sema/Sema.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Sema/AnalysisBasedWarnings.h b/clang/include/clang/Sema/AnalysisBasedWarnings.h
index 13a88bb9f8968..c73506894db9d 100644
--- a/clang/include/clang/Sema/AnalysisBasedWarnings.h
+++ b/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,9 @@ class AnalysisBasedWarnings {
void IssueWarnings(Policy P, FunctionScopeInfo *fscope,
const Decl *D, QualType BlockType);
+ // Issue warnings that require whole-translation-unit analysis.
+ void IssueWarnings(const TranslationUnitDecl *D);
+
Policy getDefaultPolicy() { return DefaultPolicy; }
void PrintStats() const;
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index f5c48ed89e93d..4d96f3b9ab32b 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/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"
@@ -25,6 +26,8 @@
#include "clang/AST/StmtCXX.h"
#include "clang/AST/StmtObjC.h"
#include "clang/AST/StmtVisitor.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Type.h"
#include "clang/Analysis/Analyses/CFGReachabilityAnalysis.h"
#include "clang/Analysis/Analyses/CalledOnceCheck.h"
#include "clang/Analysis/Analyses/Consumed.h"
@@ -35,6 +38,7 @@
#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Analysis/CFG.h"
#include "clang/Analysis/CFGStmtMap.h"
+#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Lex/Preprocessor.h"
@@ -43,6 +47,7 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/BitVector.h"
#include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
@@ -2290,6 +2295,87 @@ static void flushDiagnostics(Sema &S, const sema::FunctionScopeInfo *fscope) {
S.Diag(D.Loc, D.PD);
}
+// An AST Visitor that calls a callback function on each callable DEFINITION
+// that is NOT in a dependent context:
+class CallableVisitor : public RecursiveASTVisitor<CallableVisitor> {
+private:
+ llvm::function_ref<void(const Decl *)> Callback;
+
+public:
+ CallableVisitor(llvm::function_ref<void(const Decl *)> Callback)
+ : Callback(Callback) {}
+
+ bool VisitFunctionDecl(FunctionDecl *Node) {
+ if (cast<DeclContext>(Node)->isDependentContext())
+ return true; // Not to analyze dependent decl
+ // `FunctionDecl->hasBody()` returns true if the function has a body
+ // somewhere defined. But we want to know if this `Node` has a body
+ // child. So we use `doesThisDeclarationHaveABody`:
+ if (Node->doesThisDeclarationHaveABody())
+ Callback(Node);
+ return true;
+ }
+
+ bool VisitBlockDecl(BlockDecl *Node) {
+ if (cast<DeclContext>(Node)->isDependentContext())
+ return true; // Not to analyze dependent decl
+ Callback(Node);
+ return true;
+ }
+
+ bool VisitObjCMethodDecl(ObjCMethodDecl *Node) {
+ if (cast<DeclContext>(Node)->isDependentContext())
+ return true; // Not to analyze dependent decl
+ if (Node->hasBody())
+ Callback(Node);
+ return true;
+ }
+
+ bool VisitLambdaExpr(LambdaExpr *Node) {
+ return VisitFunctionDecl(Node->getCallOperator());
+ }
+
+ bool shouldVisitTemplateInstantiations() const { return true; }
+ bool shouldVisitImplicitCode() const { return false; }
+};
+
+void clang::sema::AnalysisBasedWarnings::IssueWarnings(
+ const TranslationUnitDecl *TU) {
+ if (!TU)
+ return; // This is unexpected, give up quietly.
+
+ DiagnosticsEngine &Diags = S.getDiagnostics();
+
+ if (S.hasUncompilableErrorOccurred() || Diags.getIgnoreAllWarnings())
+ // exit if having uncompilable errors or ignoring all warnings:
+ return;
+
+ // Whether -Wunsafe-buffer-usage should emit fix-its:
+ const bool UnsafeBufferEmitFixits =
+ Diags.getDiagnosticOptions().ShowFixits && S.getLangOpts().CPlusPlus20;
+ UnsafeBufferUsageReporter R(S);
+
+ // The Callback function that performs analyses:
+ auto CallAnalyzers = [&](const Decl *Node) -> void {
+ // Perform unsafe buffer analysis:
+ if (!Diags.isIgnored(diag::warn_unsafe_buffer_operation,
+ Node->getBeginLoc()) ||
+ !Diags.isIgnored(diag::warn_unsafe_buffer_variable,
+ Node->getBeginLoc()))
+ clang::checkUnsafeBufferUsage(Node, R, UnsafeBufferEmitFixits);
+
+ // More analysis ...
+ };
+ // Emit per-function analysis-based warnings that require the whole-TU
+ // reasoning. Check if any of them is enabled at all before scanning the AST:
+ if (!Diags.isIgnored(diag::warn_unsafe_buffer_operation, SourceLocation()) ||
+ !Diags.isIgnored(diag::warn_unsafe_buffer_variable, SourceLocation())) {
+ CallableVisitor(CallAnalyzers)
+ .TraverseTranslationUnitDecl(
+ std::remove_const_t<TranslationUnitDecl *>(TU));
+ }
+}
+
void clang::sema::AnalysisBasedWarnings::IssueWarnings(
sema::AnalysisBasedWarnings::Policy P, sema::FunctionScopeInfo *fscope,
const Decl *D, QualType BlockType) {
@@ -2518,16 +2604,6 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
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())) {
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 831e9db6e4f07..9c6db547dbefd 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1426,6 +1426,8 @@ void Sema::ActOnEndOfTranslationUnit() {
}
}
+ AnalysisWarnings.IssueWarnings(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.
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
index 62aeb1c24b547..0581ece928a82 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -220,11 +220,11 @@ void testPointerArithmetic(int * p, const int **q, T * x) {
void testTemplate(int * p) {
int *a[10];
foo(f(p, &p, a, a)[1]); // expected-warning{{unsafe buffer access}}
- // expected-note at -1{{in instantiation of function template specialization 'f<int *, 10>' requested here}}
+ // FIXME: expected note at -1{{in instantiation of function template specialization 'f<int *, 10>' requested here}}
const int **q = const_cast<const int **>(&p);
- testPointerArithmetic(p, q, p); //expected-note{{in instantiation of}}
+ testPointerArithmetic(p, q, p); //FIXME: expected note{{in instantiation of}}
}
void testPointerToMember() {
@@ -315,7 +315,7 @@ template<typename T> void fArr(T t[]) {
foo(ar[5]); // expected-note{{used in buffer access here}}
}
-template void fArr<int>(int t[]); // expected-note {{in instantiation of}}
+template void fArr<int>(int t[]); // FIXME: expected note {{in instantiation of}}
int testReturn(int t[]) {
// expected-warning at -1{{'t' is an unsafe pointer used for buffer access}}
More information about the cfe-commits
mailing list