Enhance Annotalysis to support cloned functions/methods (especially created
by IPA-SRA). The patch basically does the following:
1. For a FUNCTION_DECL, check whether it's a clone, and if so, grab its
original DECL.
2. Deal with the situation where a reference/pointer parameter is converted
to a value parameter by IPA-SRA.
Bootstrapped and passed GCC regression testsuite on x86_64-unknown-linux-gnu.
OK for google/main?
Le-chun
2011-06-10 Le-Chun Wu <l...@google.com>
* gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-78.C: New test.
* gcc/tree-threadsafe-analyze.c (get_canonical_lock_expr): Fold
expressions that are INDIRECT_REF on top of ADDR_EXPR.
(check_lock_required): Handle cloned methods.
(process_function_attrs): Likewise.
diff --git a/gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-78.C
b/gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-78.C
new file mode 100644
index 0000000..5055a92
--- /dev/null
+++ b/gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-78.C
@@ -0,0 +1,28 @@
+// Test the support to handle cloned methods.
+// { dg-do compile }
+// { dg-options "-O2 -Wthread-safety -fipa-sra" }
+
+#include "thread_annot_common.h"
+
+struct A {
+ int *data_ PT_GUARDED_BY(mu_);
+ mutable Mutex mu_;
+ void Reset(void) EXCLUSIVE_LOCKS_REQUIRED(mu_) {
+ delete data_;
+ }
+};
+
+struct B {
+ A a_;
+ void TestFunc1();
+ void TestFunc2();
+};
+
+void B::TestFunc1() {
+ MutexLock l(&a_.mu_);
+ a_.Reset(); // This call is an IPA-SRA clone candidate.
+}
+
+void B::TestFunc2() {
+ a_.Reset(); // { dg-warning "Calling function 'Reset' requires lock" }
+}
diff --git a/gcc/tree-threadsafe-analyze.c b/gcc/tree-threadsafe-analyze.c
index 3510219..d39666d 100644
--- a/gcc/tree-threadsafe-analyze.c
+++ b/gcc/tree-threadsafe-analyze.c
@@ -956,8 +956,18 @@ get_canonical_lock_expr (tree lock, tree base_obj, bool
is_temp_expr,
true /* is_temp_expr */,
new_leftmost_base_var);
if (base != canon_base)
- lock = build1 (INDIRECT_REF, TREE_TYPE (TREE_TYPE (canon_base)),
- canon_base);
+ {
+ /* If CANON_BASE is an ADDR_EXPR (e.g. &a), doing an indirect or
+ memory reference on top of it is equivalent to accessing the
+ variable itself. That is, *(&a) == a. So if that's the case,
+ simply return the variable. Otherwise, build an indirect ref
+ expression. */
+ if (TREE_CODE (canon_base) == ADDR_EXPR)
+ lock = TREE_OPERAND (canon_base, 0);
+ else
+ lock = build1 (INDIRECT_REF,
+ TREE_TYPE (TREE_TYPE (canon_base)), canon_base);
+ }
break;
}
default:
@@ -1135,10 +1145,18 @@ check_lock_required (tree lock, tree decl, tree
base_obj, bool is_indirect_ref,
NULL_TREE);
if (TREE_CODE (canon_base) != ADDR_EXPR)
{
- gcc_assert (POINTER_TYPE_P (TREE_TYPE (canon_base)));
- base_obj = build1 (INDIRECT_REF,
- TREE_TYPE (TREE_TYPE (canon_base)),
- canon_base);
+ if (POINTER_TYPE_P (TREE_TYPE (canon_base)))
+ base_obj = build1 (INDIRECT_REF,
+ TREE_TYPE (TREE_TYPE (canon_base)),
+ canon_base);
+ /* If the base object is neither an ADDR_EXPR, nor a pointer,
+ and DECL is a cloned method, most likely we have done IPA-SRA
+ optimization, where reference parameters are changed to be
+ passed by value. So in this case, just use the CANON_BASE. */
+ else if (DECL_ABSTRACT_ORIGIN (decl))
+ base_obj = canon_base;
+ else
+ gcc_assert (false);
}
else
base_obj = TREE_OPERAND (canon_base, 0);
@@ -1154,9 +1172,9 @@ check_lock_required (tree lock, tree decl, tree base_obj,
bool is_indirect_ref,
/* We want to use fully-qualified expressions (i.e. including base_obj
if any) for DECL when emitting warning messages. */
- if (base_obj)
+ if (TREE_CODE (decl) != FUNCTION_DECL)
{
- if (TREE_CODE (decl) != FUNCTION_DECL)
+ if (base_obj)
{
tree full_decl = build3 (COMPONENT_REF, TREE_TYPE (decl),
base_obj, decl, NULL_TREE);
@@ -1164,6 +1182,10 @@ check_lock_required (tree lock, tree decl, tree
base_obj, bool is_indirect_ref,
true /* is_temp_expr */, NULL_TREE);
}
}
+ else
+ /* If DECL is a function, call DECL_ORIGIN first in case it is a clone so
+ that we can avoid using the cloned name in the warning messages. */
+ decl = DECL_ORIGIN (decl);
if (!lock)
{
@@ -2116,8 +2138,15 @@ process_function_attrs (gimple call, tree fdecl,
current_bb_info->writes_ignored = false;
/* If the function is a class member, the first argument of the function
- (i.e. "this" pointer) would be the base object. */
- if (TREE_CODE (TREE_TYPE (fdecl)) == METHOD_TYPE)
+ (i.e. "this" pointer) would be the base object. Note that here we call
+ DECL_ORIGIN on fdecl first before we check whether it's a METHOD_TYPE
+ because if fdecl is a cloned method, the TREE_CODE of its type would be
+ FUNCTION_DECL instead of METHOD_DECL, which would lead us to not grab
+ its base object. One possible situation where fdecl could be a clone is
+ when -fipa-sra is enabled. (-fipa-sra is enabled by default at -O2
+ starting from GCC-4.5.). The clones could be created as early as when
+ constructing SSA. */
+ if (TREE_CODE (TREE_TYPE (DECL_ORIGIN (fdecl))) == METHOD_TYPE)
base_obj = gimple_call_arg (call, 0);
/* Check whether this is a locking primitive of any kind. */
--
This patch is available for review at http://codereview.appspot.com/4591066
