Skip to content

Commit 918e07f

Browse files
committed
[Fix] Speedup -Wunsafe-buffer-usage when using clang modules.
See https://issues.chromium.org/issues/351909443 for details and benchmarks. This improves the performance of a file containing a single line, `#include <iostream>`, from ~1 second to ~100ms on my machine.
1 parent dc79c66 commit 918e07f

File tree

2 files changed

+24
-23
lines changed

2 files changed

+24
-23
lines changed

clang/lib/Sema/AnalysisBasedWarnings.cpp

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2546,14 +2546,27 @@ static void flushDiagnostics(Sema &S, const sema::FunctionScopeInfo *fscope) {
25462546
class CallableVisitor : public DynamicRecursiveASTVisitor {
25472547
private:
25482548
llvm::function_ref<void(const Decl *)> Callback;
2549+
const unsigned int TUModuleID;
25492550

25502551
public:
2551-
CallableVisitor(llvm::function_ref<void(const Decl *)> Callback)
2552-
: Callback(Callback) {
2552+
CallableVisitor(llvm::function_ref<void(const Decl *)> Callback,
2553+
unsigned int TUModuleID)
2554+
: Callback(Callback), TUModuleID(TUModuleID) {
25532555
ShouldVisitTemplateInstantiations = true;
25542556
ShouldVisitImplicitCode = false;
25552557
}
25562558

2559+
bool TraverseDecl(Decl *Node) override {
2560+
// For performance reasons, only validate the current translation unit's
2561+
// module, and not modules it depends on.
2562+
// See https://issues.chromium.org/issues/351909443 for details.
2563+
if (Node && Node->getOwningModuleID() == TUModuleID) {
2564+
return DynamicRecursiveASTVisitor::TraverseDecl(Node);
2565+
} else {
2566+
return true;
2567+
}
2568+
}
2569+
25572570
bool VisitFunctionDecl(FunctionDecl *Node) override {
25582571
if (cast<DeclContext>(Node)->isDependentContext())
25592572
return true; // Not to analyze dependent decl
@@ -2633,7 +2646,7 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
26332646
SourceLocation()) ||
26342647
(!Diags.isIgnored(diag::warn_unsafe_buffer_libc_call, SourceLocation()) &&
26352648
S.getLangOpts().CPlusPlus /* only warn about libc calls in C++ */)) {
2636-
CallableVisitor(CallAnalyzers).TraverseTranslationUnitDecl(TU);
2649+
CallableVisitor(CallAnalyzers, TU->getOwningModuleID()).TraverseTranslationUnitDecl(TU);
26372650
}
26382651
}
26392652

clang/test/Modules/safe_buffers_optout.cpp

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -95,18 +95,10 @@ int textual(int *p) {
9595
// `safe_buffers_test_optout`, which uses another top-level module
9696
// `safe_buffers_test_base`. (So the module dependencies form a DAG.)
9797

98-
// No expected warnings from base.h because base.h is a separate
99-
// module and in a separate TU that is not textually included. The
100-
// explicit command that builds base.h has no `-Wunsafe-buffer-usage`.
101-
102-
// [email protected]:3{{unsafe buffer access}}
103-
// [email protected]:3{{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
104-
// expected-warning@test_sub1.h:5{{unsafe buffer access}}
105-
// expected-note@test_sub1.h:5{{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
106-
// expected-warning@test_sub1.h:14{{unsafe buffer access}}
107-
// expected-note@test_sub1.h:14{{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
108-
// expected-warning@test_sub2.h:5{{unsafe buffer access}}
109-
// expected-note@test_sub2.h:5{{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
98+
// No expected warnings from base.h, test_sub1, or test_sub2 because they are
99+
// in seperate modules, and the explicit commands that builds them have no
100+
// `-Wunsafe-buffer-usage`.
101+
110102
int foo(int * p) {
111103
int x = p[5]; // expected-warning{{unsafe buffer access}} expected-note{{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
112104
#pragma clang unsafe_buffer_usage begin
@@ -129,14 +121,10 @@ int foo(int * p) {
129121
// `safe_buffers_test_optout`, which uses another top-level module
130122
// `safe_buffers_test_base`. (So the module dependencies form a DAG.)
131123

132-
// [email protected]:3{{unsafe buffer access}}
133-
// [email protected]:3{{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
134-
// expected-warning@test_sub1.h:5{{unsafe buffer access}}
135-
// expected-note@test_sub1.h:5{{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
136-
// expected-warning@test_sub1.h:14{{unsafe buffer access}}
137-
// expected-note@test_sub1.h:14{{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
138-
// expected-warning@test_sub2.h:5{{unsafe buffer access}}
139-
// expected-note@test_sub2.h:5{{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
124+
// No expected warnings from base.h, test_sub1, or test_sub2 because they are
125+
// in seperate modules, and the explicit commands that builds them have no
126+
// `-Wunsafe-buffer-usage`.
127+
140128
int foo(int * p) {
141129
int x = p[5]; // expected-warning{{unsafe buffer access}} expected-note{{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
142130
#pragma clang unsafe_buffer_usage begin

0 commit comments

Comments
 (0)