diff mbox

[Annotalysis] Bugfix where lock function is attached to a base class.

Message ID CAF6KqwVsCvWkmUX_OGOewh3ZVfYz3vvzNG=a8JShKAqtSXWO+Q@mail.gmail.com
State New
Headers show

Commit Message

Delesley Hutchins Oct. 11, 2011, 5:52 p.m. UTC 
This patch fixes an error where Annotalysis generates bogus warnings
when using lock and unlock functions that are attached to a base class.
The canonicalize routine did not work correctly in this case.

Bootstrapped and passed gcc regression testsuite on
x86_64-unknown-linux-gnu.  Okay for google/gcc-4_6?

  -DeLesley

Changelog.google-4_6:
2011-10-11  DeLesley Hutchins  <delesley@google.com>

   * tree-threadsafe-analyze.c (get_canonical_lock_expr)

testsuite/Changelog.google-4_6:
2011-10-11  DeLesley Hutchins  <delesley@google.com>

   * g++.dg/thread-ann/thread_annot_lock-83.C

Comments

Diego Novillo Oct. 12, 2011, 12:10 p.m. UTC | #1 
On Tue, Oct 11, 2011 at 13:52, Delesley Hutchins <delesley@google.com> wrote:
> This patch fixes an error where Annotalysis generates bogus warnings
> when using lock and unlock functions that are attached to a base class.
> The canonicalize routine did not work correctly in this case.
>
> Bootstrapped and passed gcc regression testsuite on
> x86_64-unknown-linux-gnu.  Okay for google/gcc-4_6?
>
>  -DeLesley
>
> Changelog.google-4_6:
> 2011-10-11  DeLesley Hutchins  <delesley@google.com>
>
>   * tree-threadsafe-analyze.c (get_canonical_lock_expr)
>
> testsuite/Changelog.google-4_6:
> 2011-10-11  DeLesley Hutchins  <delesley@google.com>
>
>   * g++.dg/thread-ann/thread_annot_lock-83.C

ChangeLog entries are missing.

OK otherwise.


Diego.
diff mbox

Patch

Index: gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-83.C
===================================================================
--- gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-83.C	(revision 0)
+++ gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-83.C	(revision 0)
@@ -1,5 +1,8 @@ 
-// Regression test for bugfix, where shared locks are not properly
-// removed from locksets if a "universal lock" is present.
+// Regression test for two bugfixes.
+// Bugfix 1:  Shared locks are not properly removed from locksets
+// if a "universal lock" is present.
+// Bugfix 2:  Canonicalization does not properly store the lock in
+// the hash table if the lock function is attached to a base class.
 // { dg-do compile }
 // { dg-options "-Wthread-safety" }

@@ -7,6 +10,7 @@ 

 class Foo;

+/* Bugfix 1 */
 class Bar {
 public:
   Foo*  foo;
@@ -29,3 +33,23 @@  void Bar::bar() {
   ReaderMutexLock rlock(&mu_);
 }

+
+/* Bugfix 2 */
+class LOCKABLE Base {
+public:
+  Mutex mu_;
+
+  void Lock()   EXCLUSIVE_LOCK_FUNCTION()   { mu_.Lock();   }
+  void Unlock() UNLOCK_FUNCTION()           { mu_.Unlock(); }
+};
+
+class Derived : public Base {
+public:
+  int b;
+};
+
+void doSomething(Derived *d) {
+  d->Lock();
+  d->Unlock();
+};
+
Index: gcc/tree-threadsafe-analyze.c
===================================================================
--- gcc/tree-threadsafe-analyze.c	(revision 179771)
+++ gcc/tree-threadsafe-analyze.c	(working copy)
@@ -927,7 +927,16 @@  get_canonical_lock_expr (tree lock, tree base_obj,
                                             NULL_TREE);

           if (lang_hooks.decl_is_base_field (component))
-            return canon_base;
+            {
+              if (is_temp_expr)
+                return canon_base;
+              else
+                /* return canon_base, but recalculate it so that it is stored
+                   in the hash table. */
+                return get_canonical_lock_expr (base, base_obj,
+                                                false /* is_temp_expr */,
+                                                new_leftmost_base_var);
+            }

           if (base != canon_base)
             lock = build3 (COMPONENT_REF, TREE_TYPE (component),