diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h b/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h index 14c5b679428a3..718fc342bd9cc 100644 --- a/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h +++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h @@ -556,13 +556,20 @@ class Literal : public SExpr { template typename C::CType compare(const Literal* E, C& Cmp) const { - // TODO: defer actual comparison to LiteralT - return Cmp.trueResult(); + // If the expressions are different AST nodes, try to compare their values. + if (Cexpr && E->Cexpr && Cexpr != E->Cexpr) { + if (compareLiterals(Cexpr, E->Cexpr)) + return Cmp.trueResult(); + } + return Cmp.comparePointers(Cexpr, E->Cexpr); } private: const ValueType ValType; const Expr *Cexpr = nullptr; + + // Compare two literal expressions for value equality. + static bool compareLiterals(const Expr *E1, const Expr *E2); }; // Derived class for literal values, which stores the actual value. diff --git a/clang/lib/Analysis/ThreadSafetyTIL.cpp b/clang/lib/Analysis/ThreadSafetyTIL.cpp index dcee891222a33..b032e389ddf1a 100644 --- a/clang/lib/Analysis/ThreadSafetyTIL.cpp +++ b/clang/lib/Analysis/ThreadSafetyTIL.cpp @@ -7,6 +7,8 @@ //===----------------------------------------------------------------------===// #include "clang/Analysis/Analyses/ThreadSafetyTIL.h" +#include "clang/AST/Expr.h" +#include "clang/AST/ExprCXX.h" #include "clang/Basic/LLVM.h" #include #include @@ -54,6 +56,33 @@ SExpr* Future::force() { return Result; } +bool Literal::compareLiterals(const Expr *E1, const Expr *E2) { + if (E1->getStmtClass() != E2->getStmtClass()) + return false; + switch (E1->getStmtClass()) { + case Stmt::IntegerLiteralClass: + return cast(E1)->getValue() == + cast(E2)->getValue(); + case Stmt::CharacterLiteralClass: + return cast(E1)->getValue() == + cast(E2)->getValue(); + case Stmt::CXXBoolLiteralExprClass: + return cast(E1)->getValue() == + cast(E2)->getValue(); + case Stmt::StringLiteralClass: + return cast(E1)->getString() == + cast(E2)->getString(); + case Stmt::FloatingLiteralClass: + return cast(E1)->getValue().bitwiseIsEqual( + cast(E2)->getValue()); + case Stmt::CXXNullPtrLiteralExprClass: + case Stmt::GNUNullExprClass: + return true; + default: + return false; + } +} + unsigned BasicBlock::addPredecessor(BasicBlock *Pred) { unsigned Idx = Predecessors.size(); Predecessors.reserveCheck(1, Arena); diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp index 466135a1d9cef..0caee7ccbe5c4 100644 --- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -7536,6 +7536,32 @@ void testNestedAcquire(Container *c) EXCLUSIVE_LOCK_FUNCTION(&c->foo.mu) { buf->mu.Lock(); } +// Test that array subscripts are not ignored. +void testArrayOfContainers1() { + Container array[10]; + + Foo *ptr1 = &array[0].foo; + Foo *ptr2 = &array[1].foo; + ptr1->mu.Lock(); + ptr2->mu.Lock(); + array[0].foo.data = 0; + array[1].foo.data = 1; + ptr2->mu.Unlock(); + ptr1->mu.Unlock(); +} + +// Test that we don't confuse different indices or constants. +void testArrayOfContainers2() { + Container array[2]; + + Foo *ptr = &array[0].foo; + ptr->mu.Lock(); + array[0].foo.data = 0; + array[1].foo.data = 1; // expected-warning{{writing variable 'data' requires holding mutex 'array[1].foo.mu'}} \ + // expected-note{{found near match 'array[0].foo.mu'}} + ptr->mu.Unlock(); +} + struct ContainerOfPtr { Foo *foo_ptr; ContainerOfPtr *next;