From 2b0b223f3e0c70ceccc97f627754c0f2cd8ad2a5 Mon Sep 17 00:00:00 2001 From: Balazs Benics Date: Wed, 12 Feb 2025 14:30:17 +0100 Subject: [PATCH] [clang][analysis] Fix flaky clang/test/Analysis/live-stmts.cpp test Multiple people reported flaky bot failures tied to clang/test/Analysis/live-stmts.cpp I tried reproducing the flaky behavior on my Linux x86_64 system, but the tests appears to be stable in my context. Only by looking at the failures reported, I could formulate a potential diagnosis. The output always looked almost the same, except that the Exprs dumped per Basic block were shuffled compared to my expectation. This suggests to me some ordering issue. If you look at the backing storage of `blocksEndToLiveness[B].liveExprs`, it uses `llvm::ImmutableSet`. That container likely uses the pointer values as keys, thus the runtime values of the addresses influence the iteration order. To fix this, before dumping, I sort the expressions by their "beginLocs". It should be efficient enough for a debug checker, where there is no performance constraint. This should hopefully fix the flaky behavior on systems where ASLR works differently than (my) Linux system. Hopefully fixes #126619 Hopefully fixes #126804 --- clang/lib/Analysis/LiveVariables.cpp | 11 ++++++- clang/test/Analysis/live-stmts.cpp | 45 ++++++++++++++-------------- 2 files changed, 32 insertions(+), 24 deletions(-) diff --git a/clang/lib/Analysis/LiveVariables.cpp b/clang/lib/Analysis/LiveVariables.cpp index 481932ee59c8e..af563702b77bf 100644 --- a/clang/lib/Analysis/LiveVariables.cpp +++ b/clang/lib/Analysis/LiveVariables.cpp @@ -16,7 +16,9 @@ #include "clang/Analysis/AnalysisDeclContext.h" #include "clang/Analysis/CFG.h" #include "clang/Analysis/FlowSensitive/DataflowWorklist.h" +#include "clang/Basic/SourceManager.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/Support/raw_ostream.h" #include #include @@ -662,12 +664,19 @@ void LiveVariables::dumpExprLiveness(const SourceManager &M) { } void LiveVariablesImpl::dumpExprLiveness(const SourceManager &M) { + auto ByBeginLoc = [&M](const Expr *L, const Expr *R) { + return M.isBeforeInTranslationUnit(L->getBeginLoc(), R->getBeginLoc()); + }; + // Don't iterate over blockEndsToLiveness directly because it's not sorted. for (const CFGBlock *B : *analysisContext.getCFG()) { llvm::errs() << "\n[ B" << B->getBlockID() << " (live expressions at block exit) ]\n"; - for (const Expr *E : blocksEndToLiveness[B].liveExprs) { + std::vector LiveExprs; + llvm::append_range(LiveExprs, blocksEndToLiveness[B].liveExprs); + llvm::sort(LiveExprs, ByBeginLoc); + for (const Expr *E : LiveExprs) { llvm::errs() << "\n"; E->dump(); } diff --git a/clang/test/Analysis/live-stmts.cpp b/clang/test/Analysis/live-stmts.cpp index 33b8d59305d3d..8034d3a30436e 100644 --- a/clang/test/Analysis/live-stmts.cpp +++ b/clang/test/Analysis/live-stmts.cpp @@ -1,6 +1,3 @@ -// Flaky on aarch64: http://llvm.org/PR126619 -// UNSUPPORTED: target=aarch64{{.*}} - // RUN: %clang_analyze_cc1 -w -analyzer-checker=debug.DumpLiveExprs %s 2>&1\ // RUN: | FileCheck %s @@ -29,34 +26,36 @@ int testThatDumperWorks(int x, int y, int z) { // CHECK-EMPTY: // CHECK: [ B2 (live expressions at block exit) ] // CHECK-EMPTY: -// CHECK-NEXT: DeclRefExpr {{.*}} 'y' 'int' -// CHECK-EMPTY: -// CHECK-NEXT: DeclRefExpr {{.*}} 'z' 'int' -// CHECK-EMPTY: // CHECK-NEXT: ImplicitCastExpr {{.*}} // CHECK-NEXT: `-ImplicitCastExpr {{.*}} // CHECK-NEXT: `-DeclRefExpr {{.*}} 'x' 'int' // CHECK-EMPTY: -// CHECK-EMPTY: -// CHECK: [ B3 (live expressions at block exit) ] -// CHECK-EMPTY: // CHECK-NEXT: DeclRefExpr {{.*}} 'y' 'int' // CHECK-EMPTY: // CHECK-NEXT: DeclRefExpr {{.*}} 'z' 'int' // CHECK-EMPTY: +// CHECK-EMPTY: +// CHECK: [ B3 (live expressions at block exit) ] +// CHECK-EMPTY: // CHECK-NEXT: ImplicitCastExpr {{.*}} // CHECK-NEXT: `-ImplicitCastExpr {{.*}} // CHECK-NEXT: `-DeclRefExpr {{.*}} 'x' 'int' -// CHECK: [ B4 (live expressions at block exit) ] // CHECK-EMPTY: // CHECK-NEXT: DeclRefExpr {{.*}} 'y' 'int' // CHECK-EMPTY: // CHECK-NEXT: DeclRefExpr {{.*}} 'z' 'int' // CHECK-EMPTY: +// CHECK-EMPTY: +// CHECK: [ B4 (live expressions at block exit) ] +// CHECK-EMPTY: // CHECK-NEXT: ImplicitCastExpr {{.*}} // CHECK-NEXT: `-ImplicitCastExpr {{.*}} // CHECK-NEXT: `-DeclRefExpr {{.*}} 'x' 'int' // CHECK-EMPTY: +// CHECK-NEXT: DeclRefExpr {{.*}} 'y' 'int' +// CHECK-EMPTY: +// CHECK-NEXT: DeclRefExpr {{.*}} 'z' 'int' +// CHECK-EMPTY: // CHECK-EMPTY: // CHECK: [ B5 (live expressions at block exit) ] // CHECK-EMPTY: @@ -226,15 +225,15 @@ int logicalOpInTernary(bool b) { // CHECK: ImplicitCastExpr {{.*}} '_Bool' // CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK-EMPTY: -// CHECK: ImplicitCastExpr {{.*}} '_Bool' -// CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' -// CHECK-EMPTY: // CHECK: BinaryOperator {{.*}} '_Bool' '||' // CHECK: |-ImplicitCastExpr {{.*}} '_Bool' // CHECK: | `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK: `-ImplicitCastExpr {{.*}} '_Bool' // CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK-EMPTY: +// CHECK: ImplicitCastExpr {{.*}} '_Bool' +// CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' +// CHECK-EMPTY: // CHECK: IntegerLiteral {{.*}} 'int' 0 // CHECK-EMPTY: // CHECK: IntegerLiteral {{.*}} 'int' 1 @@ -245,15 +244,15 @@ int logicalOpInTernary(bool b) { // CHECK: ImplicitCastExpr {{.*}} '_Bool' // CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK-EMPTY: -// CHECK: ImplicitCastExpr {{.*}} '_Bool' -// CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' -// CHECK-EMPTY: // CHECK: BinaryOperator {{.*}} '_Bool' '||' // CHECK: |-ImplicitCastExpr {{.*}} '_Bool' // CHECK: | `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK: `-ImplicitCastExpr {{.*}} '_Bool' // CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK-EMPTY: +// CHECK: ImplicitCastExpr {{.*}} '_Bool' +// CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' +// CHECK-EMPTY: // CHECK: IntegerLiteral {{.*}} 'int' 0 // CHECK-EMPTY: // CHECK: IntegerLiteral {{.*}} 'int' 1 @@ -264,15 +263,15 @@ int logicalOpInTernary(bool b) { // CHECK: ImplicitCastExpr {{.*}} '_Bool' // CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK-EMPTY: -// CHECK: ImplicitCastExpr {{.*}} '_Bool' -// CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' -// CHECK-EMPTY: // CHECK: BinaryOperator {{.*}} '_Bool' '||' // CHECK: |-ImplicitCastExpr {{.*}} '_Bool' // CHECK: | `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK: `-ImplicitCastExpr {{.*}} '_Bool' // CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK-EMPTY: +// CHECK: ImplicitCastExpr {{.*}} '_Bool' +// CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' +// CHECK-EMPTY: // CHECK: IntegerLiteral {{.*}} 'int' 0 // CHECK-EMPTY: // CHECK: IntegerLiteral {{.*}} 'int' 1 @@ -283,15 +282,15 @@ int logicalOpInTernary(bool b) { // CHECK: ImplicitCastExpr {{.*}} '_Bool' // CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK-EMPTY: -// CHECK: ImplicitCastExpr {{.*}} '_Bool' -// CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' -// CHECK-EMPTY: // CHECK: BinaryOperator {{.*}} '_Bool' '||' // CHECK: |-ImplicitCastExpr {{.*}} '_Bool' // CHECK: | `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK: `-ImplicitCastExpr {{.*}} '_Bool' // CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK-EMPTY: +// CHECK: ImplicitCastExpr {{.*}} '_Bool' +// CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' +// CHECK-EMPTY: // CHECK: IntegerLiteral {{.*}} 'int' 0 // CHECK-EMPTY: // CHECK: IntegerLiteral {{.*}} 'int' 1