[cfe-commits] r163656 - in /cfe/trunk: lib/Analysis/ThreadSafety.cpp test/SemaCXX/warn-thread-safety-analysis.cpp

[Delesley Hutchins]

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 <matthewbg at google.com> wrote:
> On Tue, Sep 11, 2012 at 4:04 PM, DeLesley Hutchins <delesley at google.com> 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits



-- 
DeLesley Hutchins | Software Engineer | delesley at google.com | 505-206-0315