llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 Author: Marco Elver (melver) <details> <summary>Changes</summary> In C++20 range-based for loops, variables declared in the init-statement were not correctly added to the CFG builder's local scope. Consequently, implicit destructors for these variables were missing from the CFG. This caused analyses relying on the CFG to incorrectly model the lifetime of these variables. Specifically, Thread Safety Analysis reported false positives for RAII locks declared in the init-statement. Fix it by calling addLocalScopeForStmt for the init-statement in CFGBuilder::VisitCXXForRangeStmt. This ensures destructors are correctly inserted into the CFG. Fixes: https://github.com/abseil/abseil-cpp/issues/1901 --- Full diff: https://github.com/llvm/llvm-project/pull/175859.diff 3 Files Affected: - (modified) clang/lib/Analysis/CFG.cpp (+2) - (added) clang/test/Analysis/cxx20-range-for-cfg.cpp (+38) - (modified) clang/test/SemaCXX/warn-thread-safety-analysis.cpp (+10-2) ``````````diff diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index f8a2afec79700..9c980b2257b8c 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -4808,6 +4808,8 @@ CFGBlock *CFGBuilder::VisitCXXForRangeStmt(CXXForRangeStmt *S) { SaveAndRestore save_scope_pos(ScopePos); // Create local scopes and destructors for range, begin and end variables. + if (Stmt *Init = S->getInit()) + addLocalScopeForStmt(Init); if (Stmt *Range = S->getRangeStmt()) addLocalScopeForStmt(Range); if (Stmt *Begin = S->getBeginStmt()) diff --git a/clang/test/Analysis/cxx20-range-for-cfg.cpp b/clang/test/Analysis/cxx20-range-for-cfg.cpp new file mode 100644 index 0000000000000..9a03db113cb5a --- /dev/null +++ b/clang/test/Analysis/cxx20-range-for-cfg.cpp @@ -0,0 +1,38 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -std=c++20 %s > %t 2>&1 +// RUN: FileCheck --input-file=%t %s + +struct A { ~A(); }; +void range_for_init() { + for (A a; int x : (int[]){1, 2, 3}) {} +} + +// CHECK-LABEL: void range_for_init() +// CHECK: [B6 (ENTRY)] +// CHECK-NEXT: Succs (1): B5 + +// CHECK: [B1] +// CHECK-NEXT: 1: [B5.2].~A() (Implicit destructor) +// CHECK-NEXT: Preds (1): B2 +// CHECK-NEXT: Succs (1): B0 + +// CHECK: [B2] +// CHECK-NEXT: 1: __begin1 +// CHECK-NEXT: 2: [B2.1] (ImplicitCastExpr, LValueToRValue, int *) +// CHECK-NEXT: 3: __end1 +// CHECK-NEXT: 4: [B2.3] (ImplicitCastExpr, LValueToRValue, int *) +// CHECK-NEXT: 5: [B2.2] != [B2.4] +// CHECK-NEXT: T: for (A a; int x : [B5.8]) { +// CHECK-NEXT: } +// CHECK: Preds (2): B3 B5 +// CHECK-NEXT: Succs (2): B4 B1 + +// CHECK: [B5] +// CHECK-NEXT: 1: (CXXConstructExpr, [B5.2], A) +// CHECK-NEXT: 2: A a; +// CHECK-NEXT: 3: 1 +// CHECK-NEXT: 4: 2 +// CHECK-NEXT: 5: 3 +// CHECK-NEXT: 6: {[B5.3], [B5.4], [B5.5]} +// CHECK-NEXT: 7: (int[3])[B5.6] +// CHECK-NEXT: 8: [B5.7] +// CHECK-NEXT: 9: auto &&__range1 = (int[3]){1, 2, 3}; \ No newline at end of file diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp index d5ae2eedc2bd7..d9efa745b7d59 100644 --- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -1,7 +1,7 @@ // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-pointer -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=0 %s // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-pointer -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=1 %s -// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wthread-safety -Wthread-safety-pointer -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=0 %s -// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wthread-safety -Wthread-safety-pointer -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=1 %s +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 -Wthread-safety -Wthread-safety-pointer -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=0 %s +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 -Wthread-safety -Wthread-safety-pointer -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=1 %s // FIXME: should also run %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++11 -Wc++98-compat %s // FIXME: should also run %clang_cc1 -fsyntax-only -verify -Wthread-safety %s @@ -1847,6 +1847,14 @@ struct TestScopedLockable { a = b + 1; b = a + 1; } + +#if __cplusplus >= 202002L + void rangeForLoopInitializer() { + for (MutexLock lock{&mu1}; int& x : (int[]){1, 2, 3}) { + a = 42; + } + } +#endif }; namespace test_function_param_lock_unlock { `````````` </details> https://github.com/llvm/llvm-project/pull/175859 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
