Let's try this again; perhaps I should learn to use e-mail. :-)
This patch fixes an ICE caused when the ipa-sra optimization deletes
function arguments that are referenced from within a thread safety
attribute. It will attempt to detect this situation and recover
gracefully. Since a graceful recovery is not always possible, an
optional warning (-Wwarn-thread-optimization) can be turned on to
inform the user that certain attributes have been removed by
optimization.
Bootstrapped and passed gcc regression testsuite on
x86_64-unknown-linux-gnu. Okay for google/gcc-4_6?
-DeLesley
Changelog.google-4_6:
2011-11-02 DeLesley Hutchins <deles...@google.com>
* tree-threadsafe-analyze.c:
Ignores invalid attributes, issues a warning, recovers gracefully.
* common.opt:
Adds new thread safety warning.
testsuite/Changelog.google-4_6:
2011-11-02 DeLesley Hutchins <deles...@google.com>
* g++.dg/thread-ann/thread_annot_lock-82.C:
Expanded regression test
--
DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
Index: testsuite/g++.dg/thread-ann/thread_annot_lock-82.C
===================================================================
--- testsuite/g++.dg/thread-ann/thread_annot_lock-82.C (revision 180716)
+++ testsuite/g++.dg/thread-ann/thread_annot_lock-82.C (working copy)
@@ -1,7 +1,7 @@
-// Test template methods in the presence of cloned constructors.
-// Regression test for bugfix.
+// Regression tests: fix ICE issues when IPA-SRA deletes formal
+// function parameters.
// { dg-do compile }
-// { dg-options "-Wthread-safety -O3" }
+// { dg-options "-Wthread-safety -Wthread-warn-optimization -O3" }
#include "thread_annot_common.h"
@@ -10,6 +10,7 @@ void do_something(void* a);
class Foo {
Mutex mu_;
+ int a GUARDED_BY(mu_);
// with optimization turned on, ipa-sra should eliminate the hidden
// "this" argument, thus invalidating EXCLUSIVE_LOCKS_REQUIRED.
@@ -18,6 +19,7 @@ class Foo {
}
void foo(Foo* f);
+ void bar();
};
void Foo::foo(Foo* f) {
@@ -28,3 +30,17 @@ void Foo::foo(Foo* f) {
mu_.Unlock();
}
+
+class SCOPED_LOCKABLE DummyMutexLock {
+public:
+ // IPA-SRA should kill the parameters to these functions
+ explicit DummyMutexLock(Mutex* mutex) EXCLUSIVE_LOCK_FUNCTION(mutex) {}
+ ~DummyMutexLock() UNLOCK_FUNCTION() {}
+};
+
+
+void Foo::bar() {
+ // Matches two warnings:
+ DummyMutexLock dlock(&mu_); // { dg-warning "attribute has been removed by optimization." }
+ a = 1; // warning here should be suppressed, due to errors handling dlock
+}
Index: common.opt
===================================================================
--- common.opt (revision 180716)
+++ common.opt (working copy)
@@ -680,6 +680,10 @@ Wthread-attr-bind-param
Common Var(warn_thread_attr_bind_param) Init(1) Warning
Make the thread safety analysis try to bind the function parameters used in the attributes
+Wthread-warn-optimization
+Common Var(warn_thread_optimization) Init(0) Warning
+Warn when optimizations invalidate the thread safety analysis.
+
Wtype-limits
Common Var(warn_type_limits) Init(-1) Warning
Warn if a comparison is always true or always false due to the limited range of the data type
Index: tree-threadsafe-analyze.c
===================================================================
--- tree-threadsafe-analyze.c (revision 180716)
+++ tree-threadsafe-analyze.c (working copy)
@@ -1594,7 +1594,10 @@ get_actual_argument_from_position (gimple call, tr
lock_pos = TREE_INT_CST_LOW (pos_arg);
- gcc_assert (lock_pos >= 1 && lock_pos <= num_args);
+ /* The ipa-sra optimization can occasionally delete arguments, thus
+ invalidating the index. */
+ if (lock_pos < 1 || lock_pos > num_args)
+ return NULL_TREE;
/* The lock position specified in the attributes is 1-based, so we need to
subtract 1 from it when accessing the call arguments. */
@@ -1734,7 +1737,27 @@ handle_lock_primitive_attrs (gimple call, tree fde
a formal parameter, we need to grab the corresponding actual argument
of the call. */
else if (TREE_CODE (arg) == INTEGER_CST)
- arg = get_actual_argument_from_position (call, arg);
+ {
+ arg = get_actual_argument_from_position (call, arg);
+ /* If ipa-sra has rewritten the call, then recover as gracefully as
+ possible. */
+ if (!arg)
+ {
+ /* Add the universal lock to the lockset to suppress subsequent
+ errors. */
+ if (is_exclusive_lock)
+ pointer_set_insert(live_excl_locks, error_mark_node);
+ else
+ pointer_set_insert(live_shared_locks, error_mark_node);
+
+ if (warn_thread_optimization)
+ warning_at (*locus, OPT_Wthread_safety,
+ G_("Lock attribute has been removed by "
+ "optimization."));
+
+ return;
+ }
+ }
else if (TREE_CODE (get_leftmost_base_var (arg)) == PARM_DECL)
{
tree new_base
@@ -1890,7 +1913,18 @@ handle_unlock_primitive_attr (gimple call, tree fd
a formal parameter, we need to grab the corresponding actual argument
of the call. */
if (TREE_CODE (arg) == INTEGER_CST)
- lockable = get_actual_argument_from_position (call, arg);
+ {
+ lockable = get_actual_argument_from_position (call, arg);
+ /* If ipa-sra has rewritten the call, then fail as gracefully as
+ possible -- just leave the lock in the lockset. */
+ if (!lockable) {
+ if (warn_thread_optimization)
+ warning_at (*locus, OPT_Wthread_safety,
+ G_("Unlock attribute has been removed by "
+ "optimization."));
+ return;
+ }
+ }
else if (TREE_CODE (get_leftmost_base_var (arg)) == PARM_DECL)
{
tree fdecl = gimple_call_fndecl (call);
@@ -1908,7 +1942,15 @@ handle_unlock_primitive_attr (gimple call, tree fd
}
else
{
- gcc_assert (base_obj);
+ /* If ipa-sra has killed arguments, then base_obj may be NULL.
+ Attempt to recover gracefully by leaving the lock in the lockset. */
+ if (!base_obj) {
+ if (warn_thread_optimization)
+ warning_at (*locus, OPT_Wthread_safety,
+ G_("Unlock attribute has been removed by "
+ "optimization."));
+ return;
+ }
/* Check if the primitive is an unlock routine (e.g. the destructor or
a release function) of a scoped_lock. If so, get the lock that is
@@ -2099,6 +2141,9 @@ handle_function_lock_requirement (gimple call, tre
else if (TREE_CODE (lock) == INTEGER_CST)
{
lock = get_actual_argument_from_position (call, lock);
+ /* Ignore attribute if ipa-sra has killed the argument. */
+ if (!lock)
+ return;
/* If the lock is a function argument, we don't want to
prepend the base object to the lock name. Set the
TMP_BASE_OBJ to NULL. */
