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

[Diego Novillo <dnovillo@google.com>]

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.