Hi,
On Thu, Jun 30, 2011 at 10:01:58AM -0700, Delesley Hutchins wrote:
> This bug fixes a failure in annotalysis that is caused when gcc does
> not return the correct static type for the callee of a virtual method.
>
> Bootstrapped and passed GCC regression testsuite on x86_64-unknown-linux-gnu.
>
> Okay for branches/annotalysis and google/main?
>
> -DeLesley
>
>
> 2011-06-30 DeLesley Hutchins <deles...@google.com>
> * tree-threadsafe-analyze.c (handle_call_gs): Fixes case where
> the virtual
> method callee has unknown type.
>
>
> Index: gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-79.C
> ===================================================================
> --- gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-79.C (revision 0)
> +++ gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-79.C (revision 0)
> @@ -0,0 +1,54 @@
> +// Test virtual method calls in cases where the static type is unknown
> +// This is a "good" test case that should not incur any thread safety
> warning.
> +// { dg-do compile }
> +// { dg-options "-Wthread-safety -O" }
> +
> +#include "thread_annot_common.h"
> +
> +class Foo {
> +public:
> + Mutex m;
> +
> + Foo();
> + virtual ~Foo();
> + virtual void doSomething() EXCLUSIVE_LOCKS_REQUIRED(m) = 0;
> +};
> +
> +
> +class FooDerived : public Foo {
> +public:
> + FooDerived();
> + virtual ~FooDerived();
> + virtual void doSomething();
> +};
> +
> +
> +/**
> + * This is a test case for a bug wherein annotalysis would crash when
> analyzing
> + * a virtual method call.
> + *
> + * The following three functions represent cases where gcc loses the static
> + * type of foo in the expression foo->doSomething() when it drops down to
> + * gimple. Annotalysis should technically issue a thread-safe
> warning in these
> + * cases, but because the type is missing, it should silently accept them
> + * instead. See tree-threadsafe-analyze.c::handle_call_gs.
> + */
> +
> +// reinterpret_cast
> +void foo1(void* ptr)
> +{
> + reinterpret_cast<Foo*>(ptr)->doSomething();
> +}
> +
> +// C-style cast
> +void foo2(int* buf)
> +{
> + ((Foo*) buf)->doSomething();
> +}
> +
> +// new expression, with no local variable
> +void foo3()
> +{
> + (new FooDerived)->doSomething();
> +}
> +
> Index: gcc/tree-threadsafe-analyze.c
> ===================================================================
> --- gcc/tree-threadsafe-analyze.c (revision 175385)
> +++ gcc/tree-threadsafe-analyze.c (working copy)
> @@ -2446,7 +2446,18 @@
> if (TREE_CODE (callee) == OBJ_TYPE_REF)
> {
> tree objtype = TREE_TYPE (TREE_TYPE (OBJ_TYPE_REF_OBJECT
> (callee)));
> - fdecl = lang_hooks.get_virtual_function_decl (callee, objtype);
> + /* Check to make sure objtype is a valid type.
> + * OBJ_TYPE_REF_OBJECT does not always return the correct
> static type of the callee.
> + * For example: Given foo(void* ptr) { ((Foo*)
> ptr)->doSomething(); }
> + * objtype will be void, not Foo. Whether or not this
> happens depends on the details
> + * of how a particular call is lowered to GIMPLE, and there
> is no easy fix that works
> + * in all cases. For now, we simply rely on gcc's type
> information; if that information
> + * is not accurate, then the analysis will be less precise.
> + */
> + if (TREE_CODE(objtype) == RECORD_TYPE) /* if objtype is an
> object type... */
> + {
> + fdecl = lang_hooks.get_virtual_function_decl (callee, objtype);
> + }
> }
> }
If you in some way rely on static types of those pointers, you may be
in for a number of unpleasant surprises, because these types do not
really have any meaning at all. Just out of curiosity, I does your
analysis crash also on the testcase below (add the Mutex field if it
is necessary)?
Also, I have just very briefly glanced at the code in handle_call_gs
but still it seems to me that the code dealing with virtual calls
assumes virtual functions are never overridden...?
And pretty please, use -p diff option when creating patches to be
posted here.
Thanks,
Martin
namespace std {
typedef __SIZE_TYPE__ size_t;
}
inline void* operator new(std::size_t, void* __p) throw() {
return __p;
}
extern "C" void abort (void);
struct Foo {
public:
char stuff[1024];
};
class Bar {
public:
virtual int test (void) { return 1; }
};
Foo ft;
Foo *f = &ft;
int main()
{
Bar *b;
b = new (&f) Bar();
return b->test();
}
