| Message ID | CAF6KqwUnpmxJuYsaN9xT-9JBvL0SZ9BA1C5eGNF2QBb6808-Pw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Sep 13, 2011 at 18:27, Delesley Hutchins <delesley@google.com> wrote: > This patch fixes two bugs which cause an internal compiler error in > annotalysis. The first change fixes a failure when a variable is > typecast from an unknown (forward-defined) type. The second change > fixes a case in which an assert was triggered, because subexpressions > were not canonicalized correctly. > > Bootstrapped and passed gcc regression testsuite on > x86_64-unknown-linux-gnu. Tested on google core components. > > Okay for google/gcc-4_6? OK with a few nits below. > 2011-9-13 DeLesley Hutchins <delesley@google.com> > > * gcc/cp/class.c (cp_get_virtual_function_decl) bugfix where > type is uknown > * gcc/tree-threadsafe-analyze.c (get_canonical_lock_expr) > don't remove & on recursive call Missing ':' after ')'. This entry should be split, the change to tree-threadsafe-analyze.c goes in gcc/ChangeLog.google-4_6. The change to gcc/cp/class.c goes in gcc/cp/ChangeLog.google-4_6. Also, you need a ChangeLog entry for gcc/testsuite/ChangeLog.google-4_6 to describe the changes to the testsuite files. > @@ -8384,9 +8384,15 @@ cp_get_virtual_function_decl (tree ref, tree known > { > HOST_WIDE_INT index = tree_low_cst (OBJ_TYPE_REF_TOKEN (ref), 1); > HOST_WIDE_INT i = 0; > - tree v = BINFO_VIRTUALS (TYPE_BINFO (known_type)); > + tree binfo = TYPE_BINFO(known_type); space before '('. > @@ -884,10 +884,12 @@ get_canonical_lock_expr (tree lock, tree base_obj, > { > tree base = TREE_OPERAND (lock, 0); > tree canon_base; > - /* When the expr is a pointer to a lockable type (i.e. mu.Lock() > + /* For expressions that denote locks, > + When the expr is a pointer to a lockable type (i.e. mu.Lock() > or Lock(&mu) internally), we don't need the address-taken > operator (&). */ > - if (lookup_attribute("lockable", TYPE_ATTRIBUTES (TREE_TYPE (base)))) > + if (!is_temp_expr && > + lookup_attribute("lockable", TYPE_ATTRIBUTES (TREE_TYPE (base)))) '&&' goes in the line below. Diego.
Index: gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-79.C =================================================================== --- gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-79.C (revision 178784) +++ gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-79.C (working copy) @@ -75,3 +75,27 @@ void foo3() (new FooDerived)->doSomething(); } +class FooNodef; + +// test case for cast from undefined type +void foo4(FooNodef *f) { + ((Foo*) f)->doSomething(); +} + + +// Regression test for canonicalization of subexpressions that refer to +// lockable objects. +class LOCKABLE Base { +public: + Mutex mu_; + virtual void baseMethod() SHARED_LOCKS_REQUIRED(mu_) = 0; +}; + +class Derived : public Base { +public: + void foo() SHARED_LOCKS_REQUIRED(mu_); +}; + +void Derived::foo() { + baseMethod(); +} Index: gcc/cp/class.c =================================================================== --- gcc/cp/class.c (revision 178784) +++ gcc/cp/class.c (working copy) @@ -8384,9 +8384,15 @@ cp_get_virtual_function_decl (tree ref, tree known { HOST_WIDE_INT index = tree_low_cst (OBJ_TYPE_REF_TOKEN (ref), 1); HOST_WIDE_INT i = 0; - tree v = BINFO_VIRTUALS (TYPE_BINFO (known_type)); + tree binfo = TYPE_BINFO(known_type); + tree v; tree fndecl; - + + if (!binfo) + return NULL_TREE; + + v = BINFO_VIRTUALS (TYPE_BINFO (known_type)); + while (v && i != index) { i += (TARGET_VTABLE_USES_DESCRIPTORS Index: gcc/tree-threadsafe-analyze.c =================================================================== --- gcc/tree-threadsafe-analyze.c (revision 178784) +++ gcc/tree-threadsafe-analyze.c (working copy) @@ -884,10 +884,12 @@ get_canonical_lock_expr (tree lock, tree base_obj, { tree base = TREE_OPERAND (lock, 0); tree canon_base; - /* When the expr is a pointer to a lockable type (i.e. mu.Lock() + /* For expressions that denote locks, + When the expr is a pointer to a lockable type (i.e. mu.Lock() or Lock(&mu) internally), we don't need the address-taken operator (&). */ - if (lookup_attribute("lockable", TYPE_ATTRIBUTES (TREE_TYPE (base)))) + if (!is_temp_expr && + lookup_attribute("lockable", TYPE_ATTRIBUTES (TREE_TYPE (base)))) return get_canonical_lock_expr (base, base_obj, false /* is_temp_expr */, new_leftmost_base_var);
This patch fixes two bugs which cause an internal compiler error in annotalysis. The first change fixes a failure when a variable is typecast from an unknown (forward-defined) type. The second change fixes a case in which an assert was triggered, because subexpressions were not canonicalized correctly. Bootstrapped and passed gcc regression testsuite on x86_64-unknown-linux-gnu. Tested on google core components. Okay for google/gcc-4_6? -DeLesley gcc/Changelog.annotalysis: 2011-9-13 DeLesley Hutchins <delesley@google.com> * gcc/cp/class.c (cp_get_virtual_function_decl) bugfix where type is uknown * gcc/tree-threadsafe-analyze.c (get_canonical_lock_expr) don't remove & on recursive call