I'll try to be more clear in the future. When comparing two expressions, this particular bug could either wrongly return false, or cause an ICE, if one of the expressions was existentially quantified -- i.e. contained a wildcard.
-DeLesley On Tue, Sep 11, 2012 at 4:33 PM, Matt Beaumont-Gay <[email protected]> wrote: > On Tue, Sep 11, 2012 at 4:04 PM, DeLesley Hutchins <[email protected]> > wrote: >> Author: delesley >> Date: Tue Sep 11 18:04:49 2012 >> New Revision: 163656 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=163656&view=rev >> Log: >> Thread-safety analysis: fix bug in expression matching code. > > While an astute reader may be able to divine the nature of the bug > from reading the patch or the test, it would be nice to have a brief > description of the bug in the commit message. > >> >> Modified: >> cfe/trunk/lib/Analysis/ThreadSafety.cpp >> cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp >> >> Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=163656&r1=163655&r2=163656&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original) >> +++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Tue Sep 11 18:04:49 2012 >> @@ -454,7 +454,6 @@ >> void buildSExprFromExpr(Expr *MutexExp, Expr *DeclExp, const NamedDecl >> *D) { >> CallingContext CallCtx(D); >> >> - >> if (MutexExp) { >> if (StringLiteral* SLit = dyn_cast<StringLiteral>(MutexExp)) { >> if (SLit->getString() == StringRef("*")) >> @@ -562,7 +561,9 @@ >> >> bool matches(const SExpr &Other, unsigned i = 0, unsigned j = 0) const { >> if (NodeVec[i].matches(Other.NodeVec[j])) { >> - unsigned n = NodeVec[i].arity(); >> + unsigned ni = NodeVec[i].arity(); >> + unsigned nj = Other.NodeVec[j].arity(); >> + unsigned n = (ni < nj) ? ni : nj; >> bool Result = true; >> unsigned ci = i+1; // first child of i >> unsigned cj = j+1; // first child of j >> >> Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp?rev=163656&r1=163655&r2=163656&view=diff >> ============================================================================== >> --- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original) >> +++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Tue Sep 11 >> 18:04:49 2012 >> @@ -3341,3 +3341,43 @@ >> } // end namespace TemplateLockReturned >> >> >> +namespace ExprMatchingBugFix { >> + >> +class Foo { >> +public: >> + Mutex mu_; >> +}; >> + >> + >> +class Bar { >> +public: >> + bool c; >> + Foo* foo; >> + Bar(Foo* f) : foo(f) { } >> + >> + struct Nested { >> + Foo* foo; >> + Nested(Foo* f) : foo(f) { } >> + >> + void unlockFoo() UNLOCK_FUNCTION(&Foo::mu_); >> + }; >> + >> + void test(); >> +}; >> + >> + >> +void Bar::test() { >> + foo->mu_.Lock(); >> + if (c) { >> + Nested *n = new Nested(foo); >> + n->unlockFoo(); >> + } >> + else { >> + foo->mu_.Unlock(); >> + } >> +} >> + >> +}; // end namespace ExprMatchingBugfix >> + >> + >> + >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits -- DeLesley Hutchins | Software Engineer | [email protected] | 505-206-0315 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
