[PATCH] [Annotalysis] Fix ICE caused by ipa-sra optimization.

[PATCH] [Annotalysis] Fix ICE caused by ipa-sra optimization.

[Delesley Hutchins]

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  <delesley@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
<delesley@google.com>
   * g++.dg/thread-ann/thread_annot_lock-82.C:     Expanded regression test
-- DeLesley Hutchins | Software Engineer | delesley@google.com | 505-206-0315

Re: [PATCH] [Annotalysis] Fix ICE caused by ipa-sra optimization.

[Diego Novillo]

On Wed, Nov 2, 2011 at 16:48, Delesley Hutchins <delesley@google.com> wrote:
> 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?

The patch is missing.

> Changelog.google-4_6:2011-11-02  DeLesley Hutchins  <delesley@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
> <delesley@google.com>
>    * g++.dg/thread-ann/thread_annot_lock-82.C:     Expanded regression test

The editor at your ChangeLog too ;)

Re: [PATCH] [Annotalysis] Fix ICE caused by ipa-sra optimization.

[Delesley Hutchins]

[-- Attachment #1: Type: text/plain, Size: 1096 bytes --]

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  <delesley@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 <delesley@google.com>
   * g++.dg/thread-ann/thread_annot_lock-82.C:
     Expanded regression test

-- 
DeLesley Hutchins | Software Engineer | delesley@google.com | 505-206-0315

[-- Attachment #2: gcc-ipa-sra-ice.patch --]
[-- Type: text/x-patch, Size: 6049 bytes --]

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.  */

Re: [PATCH] [Annotalysis] Fix ICE caused by ipa-sra optimization.

[Diego Novillo]

On Thu, Nov 3, 2011 at 12:09, Delesley Hutchins <delesley@google.com> wrote:
> Let's try this again; perhaps I should learn to use e-mail.  :-)

Well, all you really need is upload-gcc-patch.py from
http://gcc.gnu.org/wiki/rietveld.  We have an internal copy, I'll send
you the location.

> 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  <delesley@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 <delesley@google.com>
>    * g++.dg/thread-ann/thread_annot_lock-82.C:
>     Expanded regression test
>
> --
> DeLesley Hutchins | Software Engineer | delesley@google.com | 505-206-0315
>

+        {
+          /* 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);

Space before '('.


+          if (!lockable) {
+            if (warn_thread_optimization)
+              warning_at (*locus, OPT_Wthread_safety,
+                          G_("Unlock attribute has been removed by "
+                             "optimization."));

Brace on the line below.


+      /* 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."));

Likewise.  Diagnostics should not end in '.'.  Comments should end in
'.  */' (two spaces after '.').

I wonder... how about disabling IPA-SRA when annotalysis is enabled?
Though I think I like this approach better.  The comments state that
these warnings are due to IPA-SRA.  I think I would leave the asserts
in, and only try the recovery if IPA-SRA is enabled.

Also, why not turn OPT_Wthread_safety on by default?



Diego.

Re: [PATCH] [Annotalysis] Fix ICE caused by ipa-sra optimization.

[Delesley Hutchins]

> I wonder... how about disabling IPA-SRA when annotalysis is enabled?

That would certainly make my life a lot easier -- IPA-SRA is not
really compatible with annotalysis.  The only problem I can see   is
that people may be unhappy if turning on annotalysis makes their code
runs slower.  We need a good way to communicate the issue to users.
Is there a way to print a note that will be reported by the build
system, e.g. "Thread safety analysis enabled, some optimizations are
disabled?"

> Also, why not turn OPT_Wthread_safety on by default?

We certainly could.  However, it generates extra warnings, which may
break the build for some users.  Which would users prefer: knowing
that annotalysis is suppressing warnings, or having to update their
build files to pass in an extra flag to turn off the warnings?

  -DeLesley

-- 
DeLesley Hutchins | Software Engineer | delesley@google.com | 505-206-0315

Re: [PATCH] [Annotalysis] Fix ICE caused by ipa-sra optimization.

[Diego Novillo]

On Thu, Nov 3, 2011 at 12:41, Delesley Hutchins <delesley@google.com> wrote:
>> I wonder... how about disabling IPA-SRA when annotalysis is enabled?
>
> That would certainly make my life a lot easier -- IPA-SRA is not
> really compatible with annotalysis.  The only problem I can see   is
> that people may be unhappy if turning on annotalysis makes their code
> runs slower.  We need a good way to communicate the issue to users.
> Is there a way to print a note that will be reported by the build
> system, e.g. "Thread safety analysis enabled, some optimizations are
> disabled?"

Certainly, you could call inform() with a diagnostic.  Though, it's
true that analysis should not affect optimization levels.  I think I
prefer making the analysis tolerate the optimizers than affecting code
generation.

>
>> Also, why not turn OPT_Wthread_safety on by default?
>
> We certainly could.  However, it generates extra warnings, which may
> break the build for some users.  Which would users prefer: knowing
> that annotalysis is suppressing warnings, or having to update their
> build files to pass in an extra flag to turn off the warnings?

What is the effect of your patch on annotalysis warnings?  Does it
produce false negatives?  If so, I think we should turn this warning
by default.  I'd rather break the build than let a false negative
through.


Diego.

Re: [PATCH] [Annotalysis] Fix ICE caused by ipa-sra optimization.

[Delesley Hutchins]

[-- Attachment #1: Type: text/plain, Size: 1752 bytes --]

New patch with suggested changes incorporated.  Warning now on by
default, asserts put back if ipa-sra not enabled, and style fixes.

  -DeLesley

On Thu, Nov 3, 2011 at 11:06 AM, Diego Novillo <dnovillo@google.com> wrote:
> On Thu, Nov 3, 2011 at 12:41, Delesley Hutchins <delesley@google.com> wrote:
>>> I wonder... how about disabling IPA-SRA when annotalysis is enabled?
>>
>> That would certainly make my life a lot easier -- IPA-SRA is not
>> really compatible with annotalysis.  The only problem I can see   is
>> that people may be unhappy if turning on annotalysis makes their code
>> runs slower.  We need a good way to communicate the issue to users.
>> Is there a way to print a note that will be reported by the build
>> system, e.g. "Thread safety analysis enabled, some optimizations are
>> disabled?"
>
> Certainly, you could call inform() with a diagnostic.  Though, it's
> true that analysis should not affect optimization levels.  I think I
> prefer making the analysis tolerate the optimizers than affecting code
> generation.
>
>>
>>> Also, why not turn OPT_Wthread_safety on by default?
>>
>> We certainly could.  However, it generates extra warnings, which may
>> break the build for some users.  Which would users prefer: knowing
>> that annotalysis is suppressing warnings, or having to update their
>> build files to pass in an extra flag to turn off the warnings?
>
> What is the effect of your patch on annotalysis warnings?  Does it
> produce false negatives?  If so, I think we should turn this warning
> by default.  I'd rather break the build than let a false negative
> through.
>
>
> Diego.
>



-- 
DeLesley Hutchins | Software Engineer | delesley@google.com | 505-206-0315

[-- Attachment #2: gcc-ipa-sra-ice.patch --]
[-- Type: text/x-patch, Size: 6183 bytes --]

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,5 +1,5 @@
-// 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" }
 
@@ -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(1) 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,15 @@ 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)
+    {
+      if (flag_ipa_sra)
+        return NULL_TREE;  /* Attempt to recover gracefully. */
+      else
+        gcc_unreachable ();
+    }
 
   /* 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 +1742,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 +1918,19 @@ 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 +1948,18 @@ 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 (!flag_ipa_sra)
+            gcc_unreachable ();
+          else 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 +2150,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.  */

Re: [PATCH] [Annotalysis] Fix ICE caused by ipa-sra optimization.

[Diego Novillo]

On Thu, Nov 3, 2011 at 15:31, Delesley Hutchins <delesley@google.com> wrote:
> New patch with suggested changes incorporated.  Warning now on by
> default, asserts put back if ipa-sra not enabled, and style fixes.

Thanks.  I only found two nits:  (1) some comments need an extra space
after the final '.', (b) diagnostic messages should start in lower
case.

OK with those changes.


Diego.

Re: [PATCH] [Annotalysis] Fix ICE caused by ipa-sra optimization.

[Martin Jambor]

Hi,

On Thu, Nov 03, 2011 at 09:09:24AM -0700, Delesley Hutchins wrote:
> 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.

I know basically nothing about the analysis (what it does, at what
point it runs) but from looking at the patch, it seems you're
basically running into problems when you don't have the parameters you
expect from annotations in the function declaration.  Have you
considered examining DECL_ABSTRACT_ORIGIN?  If non-NULL, the function
you are looking at is some sort of a clone (created by ipa-sra, ipa-cp
or ipa-split and others may follow) and DECL_ABSTRACT_ORIGIN is the
original declaration with the original PARM_DECLs and stuff that might
help you not only recover but perhaps even perform what you want to?

(Of course, ipa-sra removes scalar parameters only when they are not
used in the first place and so there should be nothing to analyze.)

Martin


> 
> 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  <delesley@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 <delesley@google.com>
>    * g++.dg/thread-ann/thread_annot_lock-82.C:
>      Expanded regression test
> 
> -- 
> DeLesley Hutchins | Software Engineer | delesley@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.  */

Re: [PATCH] [Annotalysis] Fix ICE caused by ipa-sra optimization.

[Delesley Hutchins]

Thanks for the suggestion.  Unfortunately, knowing the original
declaration doesn't help me; I also need to know the original
arguments that were passed at the call site, before those arguments
were removed by ipa-sra.

> (Of course, ipa-sra removes scalar parameters only when they are not
> used in the first place and so there should be nothing to analyze.)

The problem is that the static analysis may be using the parameters,
even if those parameters are not used in the body of the function.
For example:

void dummyLock   (Mutex* mu) EXCLUSIVE_LOCK_FUNCTION(mu) { }
void dummyUnlock(Mutex* mu) UNLOCK_FUNCTION(mu) { }

Mutex* mutex;
int a GUARDED_BY(mutex);

void foo() {
  // add mutex to set of held locks
  dummyLock(mutex);     // gets rewritten by ipa-sra to dummyLock().  Oops!
  // okay to modify a, because we've "locked" mutex
  a = 0;
  // remove mutex from set of held locks
  dummyUnlock(mutex);   // gets rewritten by ipa-sra to dummyUnlock().  Oops!
}

The annotations here tell the static analyzer to treat dummyLock and
dummyUnlock as valid lock functions, even though they don't
technically do anything.  Such a pattern is not quite as deranged as
it may at first appear -- it is used, for example, when creating a
template class that may choose to either acquire a lock, or not,
depending on its template parameter.  Ipa-sra kills the arguments, so
I no longer know which mutex was locked.

  -DeLesley


> Martin
>
>
>>
>> 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  <delesley@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 <delesley@google.com>
>>    * g++.dg/thread-ann/thread_annot_lock-82.C:
>>      Expanded regression test
>>
>> --
>> DeLesley Hutchins | Software Engineer | delesley@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.  */
>
>



-- 
DeLesley Hutchins | Software Engineer | delesley@google.com | 505-206-0315

Re: [PATCH] [Annotalysis] Fix ICE caused by ipa-sra optimization.

[Martin Jambor]

Hi,

On Fri, Nov 04, 2011 at 08:01:41AM -0700, Delesley Hutchins wrote:
> Thanks for the suggestion.  Unfortunately, knowing the original
> declaration doesn't help me; I also need to know the original
> arguments that were passed at the call site, before those arguments
> were removed by ipa-sra.

I see, that is tough.  Once you re-base the analysis on 4.7, you might
be able to use the new debugging stuff for this purpose:

http://gcc.gnu.org/ml/gcc-patches/2011-06/msg00649.html

Apart from that, I think that the information about the original
actual arguments is indeed lost in the transformation.

Martin


> 
> > (Of course, ipa-sra removes scalar parameters only when they are not
> > used in the first place and so there should be nothing to analyze.)
> 
> The problem is that the static analysis may be using the parameters,
> even if those parameters are not used in the body of the function.
> For example:
> 
> void dummyLock   (Mutex* mu) EXCLUSIVE_LOCK_FUNCTION(mu) { }
> void dummyUnlock(Mutex* mu) UNLOCK_FUNCTION(mu) { }
> 
> Mutex* mutex;
> int a GUARDED_BY(mutex);
> 
> void foo() {
>   // add mutex to set of held locks
>   dummyLock(mutex);     // gets rewritten by ipa-sra to dummyLock().  Oops!
>   // okay to modify a, because we've "locked" mutex
>   a = 0;
>   // remove mutex from set of held locks
>   dummyUnlock(mutex);   // gets rewritten by ipa-sra to dummyUnlock().  Oops!
> }
> 
> The annotations here tell the static analyzer to treat dummyLock and
> dummyUnlock as valid lock functions, even though they don't
> technically do anything.  Such a pattern is not quite as deranged as
> it may at first appear -- it is used, for example, when creating a
> template class that may choose to either acquire a lock, or not,
> depending on its template parameter.  Ipa-sra kills the arguments, so
> I no longer know which mutex was locked.
> 
>   -DeLesley
> 
> 
> > Martin
> >
> >
> >>
> >> 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  <delesley@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 <delesley@google.com>
> >>    * g++.dg/thread-ann/thread_annot_lock-82.C:
> >>      Expanded regression test
> >>
> >> --
> >> DeLesley Hutchins | Software Engineer | delesley@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.  */
> >
> >
> 
> 
> 
> -- 
> DeLesley Hutchins | Software Engineer | delesley@google.com | 505-206-0315