[clang] [Fix] Speedup -Wunsafe-buffer-usage when using clang modules. (PR #127161)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: Matt (matts1)

<details>
<summary>Changes</summary>

Each piece of code should have analysis run on it precisely once. However, if you build a module, and then build another module depending on it, the header file will have `-Wunsafe-buffer-usage` run on it twice. This normally isn't a huge issue,  but in the case of using the standard library as a module, simply adding the line `#include <cstddef>` increases compile times by 900ms (from 100ms to 1 second) on my machine. I believe this is because the standard library has massive modules, of which only a small part is used (the AST is ~700k lines), and because if what I've been told is correct, the AST is lazily generated, and `-Wunsafe-buffer-usage` forces it to be evaluated every time.

See https://issues.chromium.org/issues/351909443 for details and benchmarks.

---
Full diff: https://github.com/llvm/llvm-project/pull/127161.diff


2 Files Affected:

- (modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+16-3) 
- (modified) clang/test/Modules/safe_buffers_optout.cpp (+8-20) 


``````````diff
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 589869d018657..849c899bbbc8b 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2546,14 +2546,27 @@ static void flushDiagnostics(Sema &S, const sema::FunctionScopeInfo *fscope) {
 class CallableVisitor : public DynamicRecursiveASTVisitor {
 private:
   llvm::function_ref<void(const Decl *)> Callback;
+  const unsigned int TUModuleID;
 
 public:
-  CallableVisitor(llvm::function_ref<void(const Decl *)> Callback)
-      : Callback(Callback) {
+  CallableVisitor(llvm::function_ref<void(const Decl *)> Callback,
+                  unsigned int TUModuleID)
+      : Callback(Callback), TUModuleID(TUModuleID) {
     ShouldVisitTemplateInstantiations = true;
     ShouldVisitImplicitCode = false;
   }
 
+  bool TraverseDecl(Decl *Node) override {
+    // For performance reasons, only validate the current translation unit's
+    // module, and not modules it depends on.
+    // See https://issues.chromium.org/issues/351909443 for details.
+    if (Node && Node->getOwningModuleID() == TUModuleID) {
+      return DynamicRecursiveASTVisitor::TraverseDecl(Node);
+    } else {
+      return true;
+    }
+  }
+
   bool VisitFunctionDecl(FunctionDecl *Node) override {
     if (cast<DeclContext>(Node)->isDependentContext())
       return true; // Not to analyze dependent decl
@@ -2633,7 +2646,7 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
                        SourceLocation()) ||
       (!Diags.isIgnored(diag::warn_unsafe_buffer_libc_call, SourceLocation()) &&
        S.getLangOpts().CPlusPlus /* only warn about libc calls in C++ */)) {
-    CallableVisitor(CallAnalyzers).TraverseTranslationUnitDecl(TU);
+    CallableVisitor(CallAnalyzers, TU->getOwningModuleID()).TraverseTranslationUnitDecl(TU);
   }
 }
 
diff --git a/clang/test/Modules/safe_buffers_optout.cpp b/clang/test/Modules/safe_buffers_optout.cpp
index 2129db65da752..8c3d6a235d399 100644
--- a/clang/test/Modules/safe_buffers_optout.cpp
+++ b/clang/test/Modules/safe_buffers_optout.cpp
@@ -95,18 +95,10 @@ int textual(int *p) {
 // `safe_buffers_test_optout`, which uses another top-level module
 // `safe_buffers_test_base`. (So the module dependencies form a DAG.)
 
-// No expected warnings from base.h because base.h is a separate
-// module and in a separate TU that is not textually included.  The
-// explicit command that builds base.h has no `-Wunsafe-buffer-usage`.
-
-// expected-warning at base.h:3{{unsafe buffer access}}
-// expected-note at base.h:3{{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
-// expected-warning at test_sub1.h:5{{unsafe buffer access}}
-// expected-note at test_sub1.h:5{{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
-// expected-warning at test_sub1.h:14{{unsafe buffer access}}
-// expected-note at test_sub1.h:14{{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
-// expected-warning at test_sub2.h:5{{unsafe buffer access}}
-// expected-note at test_sub2.h:5{{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
+// No expected warnings from base.h, test_sub1, or test_sub2 because they are
+// in seperate modules, and the explicit commands that builds them have no
+// `-Wunsafe-buffer-usage`.
+
 int foo(int * p) {
   int x = p[5]; // expected-warning{{unsafe buffer access}} expected-note{{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
 #pragma clang unsafe_buffer_usage begin
@@ -129,14 +121,10 @@ int foo(int * p) {
 // `safe_buffers_test_optout`, which uses another top-level module
 // `safe_buffers_test_base`. (So the module dependencies form a DAG.)
 
-// expected-warning at base.h:3{{unsafe buffer access}}
-// expected-note at base.h:3{{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
-// expected-warning at test_sub1.h:5{{unsafe buffer access}}
-// expected-note at test_sub1.h:5{{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
-// expected-warning at test_sub1.h:14{{unsafe buffer access}}
-// expected-note at test_sub1.h:14{{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
-// expected-warning at test_sub2.h:5{{unsafe buffer access}}
-// expected-note at test_sub2.h:5{{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
+// No expected warnings from base.h, test_sub1, or test_sub2 because they are
+// in seperate modules, and the explicit commands that builds them have no
+// `-Wunsafe-buffer-usage`.
+
 int foo(int * p) {
   int x = p[5]; // expected-warning{{unsafe buffer access}} expected-note{{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
 #pragma clang unsafe_buffer_usage begin

``````````

</details>


https://github.com/llvm/llvm-project/pull/127161