Re: [PATCH][google] Add warning flags for clang compatibility
google/integration: r192542. On Tue, Oct 16, 2012 at 1:00 PM, Delesley Hutchins deles...@google.com wrote: Committed. google/gcc-4_6: r192510. google/gcc-4_7: r192511. google/main: r192513. On Tue, Oct 16, 2012 at 12:44 PM, Delesley Hutchins deles...@google.com wrote: Yes, but that won't happen for a while yet. On Tue, Oct 16, 2012 at 12:43 PM, Diego Novillo dnovi...@google.com wrote: On Tue, Oct 16, 2012 at 3:19 PM, Delesley Hutchins deles...@google.com wrote: This patch adds the clang thread safety warning flags as recognized warning flags that do nothing, for command-line compatibility with clang. Enclosed patch is for branches/google/gcc-4_6, I have identical patches for google/gcc-4_7 and google/main. OK for all google/ branches. You still need to remove the old thread safety code, right? Diego. -- DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315 -- DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315 -- DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
[PATCH][google] Add warning flags for clang compatibility
This patch adds the clang thread safety warning flags as recognized warning flags that do nothing, for command-line compatibility with clang. Enclosed patch is for branches/google/gcc-4_6, I have identical patches for google/gcc-4_7 and google/main. Tested on linux x86. -- DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315 gcc-warning-flags.patch Description: Binary data
Re: [PATCH][google] Add warning flags for clang compatibility
Yes, but that won't happen for a while yet. On Tue, Oct 16, 2012 at 12:43 PM, Diego Novillo dnovi...@google.com wrote: On Tue, Oct 16, 2012 at 3:19 PM, Delesley Hutchins deles...@google.com wrote: This patch adds the clang thread safety warning flags as recognized warning flags that do nothing, for command-line compatibility with clang. Enclosed patch is for branches/google/gcc-4_6, I have identical patches for google/gcc-4_7 and google/main. OK for all google/ branches. You still need to remove the old thread safety code, right? Diego. -- DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
Re: [PATCH][google] Add warning flags for clang compatibility
Committed. google/gcc-4_6: r192510. google/gcc-4_7: r192511. google/main: r192513. On Tue, Oct 16, 2012 at 12:44 PM, Delesley Hutchins deles...@google.com wrote: Yes, but that won't happen for a while yet. On Tue, Oct 16, 2012 at 12:43 PM, Diego Novillo dnovi...@google.com wrote: On Tue, Oct 16, 2012 at 3:19 PM, Delesley Hutchins deles...@google.com wrote: This patch adds the clang thread safety warning flags as recognized warning flags that do nothing, for command-line compatibility with clang. Enclosed patch is for branches/google/gcc-4_6, I have identical patches for google/gcc-4_7 and google/main. OK for all google/ branches. You still need to remove the old thread safety code, right? Diego. -- DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315 -- DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
Re: [PATCH] [Annotalysis] Add support for arrays in lock expressions
Why PLUS_EXPR and MULT_EXPR? Pointer arithmetic should use POINTER_PLUS_EXPR exclusively. I don't think you should be seeing PLUS_EXPRs here. The MULT_EXPR show up in scaling expressions? MULT_EXPR shows up in array indexing, since the index is multiplied by the size of the element; gcc converts everything to bytes before lowering. I added PLUS_EXPR for completeness, since I'm sure someone will write an expression like array[i+1] at some point. :-) -DeLesley
Re: [PATCH] [Annotalysis] Add support for arrays in lock expressions
But you should not see such an expression in gimple. The array index is always a gimple_val. I'm not sure what you mean. The expression array[i+1] compiles to the following (courtesy of dump-tree-ssa): D.2095_4 = (long unsigned int) i_1; D.2096_5 = D.2095_4 + 1; D.2097_6 = D.2096_5 * 4; D.2098_8 = array_7(D) + D.2097_6; Annotalysis canonicalizes this gimple code into a tree that includes a PLUS_EXPR, a MULT_EXPR, and a POINTER_PLUS_EXPR. -DeLesley
[PATCH] [Annotalysis] Support trylock attributes on virtual methods.
This patch fixes a bug wherein the trylock attribute would not work if
it was attached to a virtual method.
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-08 DeLesley Hutchins deles...@google.com
* tree-threadsafe-analyze.c:
factors out code to get function decl.
testsuite/Changelog.google-4_6:
2011-11-08 DeLesley Hutchins deles...@google.com
* g++.dg/thread-ann/thread_annot_lock-85.C:
New regression test
--
DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
Index: testsuite/g++.dg/thread-ann/thread_annot_lock-85.C
===
--- testsuite/g++.dg/thread-ann/thread_annot_lock-85.C (revision 0)
+++ testsuite/g++.dg/thread-ann/thread_annot_lock-85.C (revision 0)
@@ -0,0 +1,22 @@
+// Regression test, handle trylock on virtual method.
+// { dg-do compile }
+// { dg-options -Wthread-safety }
+
+#include thread_annot_common.h
+
+class LOCKABLE Lock {
+ public:
+ virtual ~Lock() {}
+ virtual bool TryLock() EXCLUSIVE_TRYLOCK_FUNCTION(true) { return true; }
+ void Unlock() UNLOCK_FUNCTION() {}
+};
+
+
+void foo() {
+ Lock x;
+ Lock *p = x;
+ if (p-TryLock()) {
+p-Unlock();
+ }
+}
+
Index: tree-threadsafe-analyze.c
===
--- tree-threadsafe-analyze.c (revision 180984)
+++ tree-threadsafe-analyze.c (working copy)
@@ -2508,26 +2508,14 @@
return;
}
-/* The main routine that handles gimple call statements. */
-static void
-handle_call_gs (gimple call, struct bb_threadsafe_info *current_bb_info)
-{
+/* Get the function declaration from a gimple call stmt. This handles both
+ ordinary function calls and virtual methods. */
+static tree
+get_fdecl_from_gimple_stmt (gimple call) {
tree fdecl = gimple_call_fndecl (call);
- int num_args = gimple_call_num_args (call);
- int arg_index = 0;
- tree arg_type = NULL_TREE;
- tree arg;
- tree lhs;
- location_t locus;
-
- if (!gimple_has_location (call))
-locus = input_location;
- else
-locus = gimple_location (call);
-
/* If the callee fndecl is NULL, check if it is a virtual function,
- and if so, try to get its decl through the reference object. */
+ and if so, try to get its decl through the reference object. */
if (!fdecl)
{
tree callee = gimple_call_fn (call);
@@ -2546,7 +2534,28 @@
fdecl = lang_hooks.get_virtual_function_decl (callee, objtype);
}
}
+ return fdecl;
+}
+
+/* The main routine that handles gimple call statements. */
+
+static void
+handle_call_gs (gimple call, struct bb_threadsafe_info *current_bb_info)
+{
+ tree fdecl = get_fdecl_from_gimple_stmt (call);
+ int num_args = gimple_call_num_args (call);
+ int arg_index = 0;
+ tree arg_type = NULL_TREE;
+ tree arg;
+ tree lhs;
+ location_t locus;
+
+ if (!gimple_has_location (call))
+locus = input_location;
+ else
+locus = gimple_location (call);
+
/* The callee fndecl could be NULL, e.g., when the function is passed in
as an argument. */
if (fdecl)
@@ -2839,7 +2848,8 @@
}
else if (is_gimple_call (gs))
{
- tree fdecl = gimple_call_fndecl (gs);
+ tree fdecl = get_fdecl_from_gimple_stmt (gs);
+
/* The function decl could be null in some cases, e.g.
a function pointer passed in as a parameter. */
if (fdecl
Re: [PATCH] [Annotalysis] Fix ICE caused by ipa-sra optimization.
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 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
Re: [PATCH] [Annotalysis] Fix ICE caused by ipa-sra optimization.
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
Re: [PATCH] [Annotalysis] Fix ICE caused by ipa-sra optimization.
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 | deles...@google.com | 505-206-0315
[PATCH] [Annotalysis] Add support for arrays in lock expressions
This patch adds support for array indexing (i.e. operator []) in lock
expressions. The current version of gcc seems to emit these as
expressions involving pointer arithmetic, so we update
get_canonical_lock_expr() to handle such expressions.
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-03 DeLesley Hutchins deles...@google.com
* tree-threadsafe-analyze.c (get_canonical_lock_expr):
Add support for pointer arithmetic operations
testsuite/Changelog.google-4_6:
2011-11-02 DeLesley Hutchins deles...@google.com
* g++.dg/thread-ann/thread_annot_lock-84.C:
New regression test.
--
DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
Index: testsuite/g++.dg/thread-ann/thread_annot_lock-84.C
===
--- testsuite/g++.dg/thread-ann/thread_annot_lock-84.C (revision 0)
+++ testsuite/g++.dg/thread-ann/thread_annot_lock-84.C (revision 0)
@@ -0,0 +1,29 @@
+// Test lock expressions involving array elements.
+// { dg-do compile }
+// { dg-options -Wthread-safety }
+
+#include thread_annot_common.h
+
+struct Foo {
+ Mutex mu_;
+ int a GUARDED_BY(mu_);
+
+ static void foo1(Foo* foos, int n);
+ static void foo2(Foo* foos, int n);
+};
+
+void Foo::foo1(Foo* foos, int n) {
+ for (int i = 0; i n; ++i) {
+foos[i].mu_.Lock();
+foos[i].a = 0;
+foos[i].mu_.Unlock();
+ }
+}
+
+void Foo::foo2(Foo* foos, int n) {
+ for (int i = 0; i n-1; ++i) {
+foos[i].mu_.Lock();
+foos[i+1].a = 0;// { dg-warning Writing to variable }
+foos[i].mu_.Unlock();
+ }
+}
Index: tree-threadsafe-analyze.c
===
--- tree-threadsafe-analyze.c (revision 180716)
+++ tree-threadsafe-analyze.c (working copy)
@@ -79,6 +79,7 @@ along with GCC; see the file COPYING3.
#include coretypes.h
#include tm.h
#include tree.h
+#include gimple.h
#include c-family/c-common.h
#include toplev.h
#include input.h
@@ -804,12 +805,27 @@ get_canonical_lock_expr (tree lock, tree
!gimple_nop_p (SSA_NAME_DEF_STMT (lock)))
{
gimple def_stmt = SSA_NAME_DEF_STMT (lock);
- if (is_gimple_assign (def_stmt)
- (get_gimple_rhs_class (gimple_assign_rhs_code (def_stmt))
- == GIMPLE_SINGLE_RHS))
-return get_canonical_lock_expr (gimple_assign_rhs1 (def_stmt),
-base_obj, is_temp_expr,
-NULL_TREE);
+ if (is_gimple_assign (def_stmt))
+{
+ enum gimple_rhs_class gcls =
+get_gimple_rhs_class (gimple_assign_rhs_code (def_stmt));
+ tree rhs = 0;
+
+ if (gcls == GIMPLE_SINGLE_RHS)
+rhs = gimple_assign_rhs1 (def_stmt);
+ else if (gcls == GIMPLE_UNARY_RHS)
+rhs = build1 (gimple_assign_rhs_code (def_stmt),
+ TREE_TYPE (gimple_assign_lhs (def_stmt)),
+ gimple_assign_rhs1 (def_stmt));
+ else if (gcls == GIMPLE_BINARY_RHS)
+rhs = build2 (gimple_assign_rhs_code (def_stmt),
+ TREE_TYPE (gimple_assign_lhs (def_stmt)),
+ gimple_assign_rhs1 (def_stmt),
+ gimple_assign_rhs2 (def_stmt));
+ if (rhs)
+return get_canonical_lock_expr (rhs, base_obj,
+is_temp_expr, NULL_TREE);
+}
else if (is_gimple_call (def_stmt))
{
tree fdecl = gimple_call_fndecl (def_stmt);
@@ -981,6 +997,24 @@ get_canonical_lock_expr (tree lock, tree
TREE_TYPE (TREE_TYPE (canon_base)), canon_base);
break;
}
+ case PLUS_EXPR:
+ case POINTER_PLUS_EXPR:
+ case MULT_EXPR:
+{
+ tree left = TREE_OPERAND (lock, 0);
+ tree canon_left = get_canonical_lock_expr (left, base_obj,
+ true /* is_temp_expr */,
+ NULL_TREE);
+
+ tree right = TREE_OPERAND (lock, 1);
+ tree canon_right = get_canonical_lock_expr (right, base_obj,
+ true /* is_temp_expr */,
+ NULL_TREE);
+ if (left != canon_left || right != canon_right)
+lock = build2 (TREE_CODE(lock), TREE_TYPE(lock),
+ canon_left, canon_right);
+ break;
+}
default:
break;
}
Re: [PATCH] [Annotalysis] Fix ICE caused by ipa-sra optimization.
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 dnovi...@google.com wrote:
On Thu, Nov 3, 2011 at 12:41, Delesley Hutchins deles...@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 | 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,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
[PATCH] [Annotalysis] Fix ICE caused by ipa-sra optimization.
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
Re: [PATCH] [Annotalysis] Bugfix for spurious thread safety warnings with shared mutexes
I don't think that will fix this bug. The bug occurs if: (1) The exclusive lock set has error_mark_node. (2) The shared lock set has the actual lock. In this case, remove_lock_from_lockset thinks that it has found the lock in the exclusive lock set, and fails to remove it from the shared lock set. To fix the bug, the first call to lock_set_contains should ignore the universal lock and return null, so that remove_lock will continue on to search the shared lock set. If I understand your suggested fix correctly, lock_set_contains would still return non-null when the universal lock was present, which is not what we want. IMHO, lock_set_contains is operating correctly; it was just passed the wrong arguments. -DeLesley On Tue, Oct 11, 2011 at 2:34 PM, Ollie Wild a...@google.com wrote: On Mon, Oct 10, 2011 at 3:37 PM, Delesley Hutchins deles...@google.com wrote: --- gcc/tree-threadsafe-analyze.c (revision 179771) +++ gcc/tree-threadsafe-analyze.c (working copy) @@ -1830,14 +1830,27 @@ remove_lock_from_lockset (tree lockable, struct po This feels like a bug in lock_set_contains(), not remove_lock_from_lockset(). I'd modify lock_set_contains() as follows: 1) During the universal lock conditional, remove the return statement. Instead, set default_lock = lock (where default_lock is a new variable initialized to NULL_TREE). 2) Anywhere NULL_TREE is returned later, replace it with default_lock. Ollie -- DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
[PATCH] [Annotalysis] Bugfix where lock function is attached to a base class.
This patch fixes an error where Annotalysis generates bogus warnings
when using lock and unlock functions that are attached to a base class.
The canonicalize routine did not work correctly in this case.
Bootstrapped and passed gcc regression testsuite on
x86_64-unknown-linux-gnu. Okay for google/gcc-4_6?
-DeLesley
Changelog.google-4_6:
2011-10-11 DeLesley Hutchins deles...@google.com
* tree-threadsafe-analyze.c (get_canonical_lock_expr)
testsuite/Changelog.google-4_6:
2011-10-11 DeLesley Hutchins deles...@google.com
* g++.dg/thread-ann/thread_annot_lock-83.C
Index: gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-83.C
===
--- gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-83.C (revision 0)
+++ gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-83.C (revision 0)
@@ -1,5 +1,8 @@
-// Regression test for bugfix, where shared locks are not properly
-// removed from locksets if a universal lock is present.
+// Regression test for two bugfixes.
+// Bugfix 1: Shared locks are not properly removed from locksets
+// if a universal lock is present.
+// Bugfix 2: Canonicalization does not properly store the lock in
+// the hash table if the lock function is attached to a base class.
// { dg-do compile }
// { dg-options -Wthread-safety }
@@ -7,6 +10,7 @@
class Foo;
+/* Bugfix 1 */
class Bar {
public:
Foo* foo;
@@ -29,3 +33,23 @@ void Bar::bar() {
ReaderMutexLock rlock(mu_);
}
+
+/* Bugfix 2 */
+class LOCKABLE Base {
+public:
+ Mutex mu_;
+
+ void Lock() EXCLUSIVE_LOCK_FUNCTION() { mu_.Lock(); }
+ void Unlock() UNLOCK_FUNCTION() { mu_.Unlock(); }
+};
+
+class Derived : public Base {
+public:
+ int b;
+};
+
+void doSomething(Derived *d) {
+ d-Lock();
+ d-Unlock();
+};
+
Index: gcc/tree-threadsafe-analyze.c
===
--- gcc/tree-threadsafe-analyze.c (revision 179771)
+++ gcc/tree-threadsafe-analyze.c (working copy)
@@ -927,7 +927,16 @@ get_canonical_lock_expr (tree lock, tree base_obj,
NULL_TREE);
if (lang_hooks.decl_is_base_field (component))
-return canon_base;
+{
+ if (is_temp_expr)
+return canon_base;
+ else
+/* return canon_base, but recalculate it so that it is stored
+ in the hash table. */
+return get_canonical_lock_expr (base, base_obj,
+false /* is_temp_expr */,
+new_leftmost_base_var);
+}
if (base != canon_base)
lock = build3 (COMPONENT_REF, TREE_TYPE (component),
--
DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
[PATCH] [Annotalysis] Bugfix for spurious thread safety warnings with shared mutexes
This patch fixes an error where Annotalysis generates bogus warnings
when a shared lock is released in a function that has a universal
lock -- typically produced when gcc cannot parse a lock expression.
Bootstrapped and passed gcc regression testsuite on
x86_64-unknown-linux-gnu. Okay for google/gcc-4_6?
-DeLesley
Changelog.google-4_6:
2011-10-10 DeLesley Hutchins deles...@google.com
* tree-threadsafe-analyze.c (remove_lock_from_lockset)
testsuite/Changelog.google-4_6:
2011-10-10 DeLesley Hutchins deles...@google.com
* g++.dg/thread-ann/thread_annot_lock-83.C:
New regression test
Index: gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-83.C
===
--- gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-83.C (revision 0)
+++ gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-83.C (revision 0)
@@ -0,0 +1,31 @@
+// Regression test for bugfix, where shared locks are not properly
+// removed from locksets if a universal lock is present.
+// { dg-do compile }
+// { dg-options -Wthread-safety }
+
+#include thread_annot_common.h
+
+class Foo;
+
+class Bar {
+public:
+ Foo* foo;
+ Mutex mu_;
+
+ // foo-mu_ is not visible at this point in the code.
+ // so the attribute will become a universal lock.
+ void bar() EXCLUSIVE_LOCKS_REQUIRED(foo-mu_);
+};
+
+
+class Foo {
+public:
+ Mutex mu_;
+ int a;
+};
+
+
+void Bar::bar() {
+ ReaderMutexLock rlock(mu_);
+}
+
Index: gcc/tree-threadsafe-analyze.c
===
--- gcc/tree-threadsafe-analyze.c (revision 179771)
+++ gcc/tree-threadsafe-analyze.c (working copy)
@@ -1830,14 +1830,27 @@ remove_lock_from_lockset (tree lockable, struct po
{
tree lock_contained;
- if ((lock_contained = lock_set_contains(live_excl_locks, lockable, NULL_TREE,
- false)) != NULL_TREE)
+ /* Try to remove the actual lock. */
+ if ((lock_contained = lock_set_contains(live_excl_locks, lockable,
+ NULL_TREE, true)) != NULL_TREE)
pointer_set_delete (live_excl_locks, lock_contained);
else if ((lock_contained = lock_set_contains(live_shared_locks, lockable,
- NULL_TREE, false)) != NULL_TREE)
+ NULL_TREE, true)) != NULL_TREE)
pointer_set_delete (live_shared_locks, lock_contained);
- return lock_contained;
+ if (lock_contained)
+return lock_contained;
+
+ /* If either of lock sets contains the universal lock, then pretend that
+ we've removed it, to avoid a warning about unlocking a lock that was
+ not acquired. */
+ if (pointer_set_contains (live_excl_locks, error_mark_node))
+return lockable;
+
+ if (pointer_set_contains(live_shared_locks, error_mark_node))
+return lockable;
+
+ return NULL_TREE;
}
/* This function handles function calls that release locks (i.e. the
[PATCH] [Annotalysis] Fix internal compiler error on template methods
I don't have a test case, but look at the definitions of the two macros in cp/cp-tree.h. DECL_CLONED_FUNCTION_P calls decl_cloned_function_p, passing true as the second argument, while DECL_CLONED_FUNCTION makes the same call, but passes false. Now look at the definition of decl_cloned_function_p in cp/class.c. If the second argument is true, it will step into templates, and if it is false, it won't. Incidentally, the ICE occurs when DECL_CLONED_FUNCTION is applied to a template function, so this is not a hypothetical case. :-) IMHO, it doesn't make sense to me to have a predicate that can potentially succeed on templates, when the macro itself will fail. However, I don't understand how cloned functions work, so I may be missing something here -- perhaps a template function is never a clone, so the predicate still returns NULL in that case? But if so, then why step into templates at all? -DeLesley On Thu, Sep 29, 2011 at 8:42 AM, Diego Novillo dnovi...@google.com wrote: On Thu, Sep 29, 2011 at 11:26, Ollie Wild a...@google.com wrote: On Thu, Sep 29, 2011 at 10:13 AM, Delesley Hutchins deles...@google.com wrote: Unfortunately, DECL_CLONED_FUNCTION_P is not actually a predicate that allows you to call DECL_CLONED_FUNCTION safely. Look at the definition of the macros; despite what the comments say, DECL_CLONED_FUNCTION_P may return true in cases where DECL_CLONED_FUNCTION will still crash. The correct fix is to fix the macros, but I have no understanding of what they are actually doing. :-( -DeLesley Diego, can you comment? Really? That's surprising. I would certainly expect DECL_CLONED_FUNCTION_P to be exactly the right predicate to guard DECL_CLONED_FUNCTION with. That's how it's used elsewhere. Delesley, can you give more details on when DECL_CLONED_FUNCTION_P fails? Changing that predicate with: if (DECL_CLONED_FUNCTION_P (clone) DECL_CLONED_FUNCTION (clone) == decl) should be all you need. If that's not working, then send me the test case, cause I'll be confused :) Diego.
Re: [PATCH] [Annotalysis] Fix internal compiler error on template methods
The ICE occurs when decl is a TEMPLATE_DECL; that's the corner case that causes a problem. The patch that you and Ollie suggest does stop the ICE for our particular example; I assume because the template in question is not a clone, and so the predicate fails further on. However, I'm not convinced that it will work in all cases. I wish I could come up with a test case, but like I said, I don't understand enough about clones to understand what's happening here. If you are confident that DECL_CLONED_FUNCTION_P is correct, then we can use that; I personally had no such confidence. -DeLesley On Thu, Sep 29, 2011 at 10:41 AM, Diego Novillo dnovi...@google.com wrote: On 11-09-29 13:21 , Delesley Hutchins wrote: I don't have a test case, but look at the definitions of the two macros in cp/cp-tree.h. DECL_CLONED_FUNCTION_P calls decl_cloned_function_p, passing true as the second argument, while DECL_CLONED_FUNCTION makes the same call, but passes false. Now look at the definition of decl_cloned_function_p in cp/class.c. If the second argument is true, it will step into templates, and if it is false, it won't. Incidentally, the ICE occurs when DECL_CLONED_FUNCTION is applied to a template function, so this is not a hypothetical case. :-) But notice that STRIP_TEMPLATE is a NOP when DECL is not a TEMPLATE_DECL. So, I'm not sure where you saw it ICE. We are already using this idiom all over the parser, so it would be great if you could produce a test case for the failure you have in mind. Incidentally, I applied the variant of the patch Ollie and I suggested and the testcase works fine with it (while it fails without the patch, of course). Diego. -- DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
Re: [PATCH] [Annotalysis] Fix internal compiler error on template methods
That seems much cleaner! LGTM. Are you submitting the patch? I can
submit the test case.
-DeLesley
On Thu, Sep 29, 2011 at 1:50 PM, Diego Novillo dnovi...@google.com wrote:
On 11-09-29 15:19 , Delesley Hutchins wrote:
However, I'm not convinced that it will work in all cases. I wish I
could come up with a test case, but like I said, I don't understand
enough about clones to understand what's happening here. If you are
confident that DECL_CLONED_FUNCTION_P is correct, then we can use
that; I personally had no such confidence.
Yes, there is no case in which a TEMPLATE_DECL will match
DECL_CLONED_FUNCTION_P. I agree with you that the logic in
decl_cloned_function_p seems like it may allow that, but clones of functions
are created as FUNCTION_DECLs.
In looking at the code again, I think we could even simplify it a bit more
using FOR_EACH_CLONE. This does the same work as the if()/for() combination
in a more compact way.
Diego.
Index: cp/parser.c
===
--- cp/parser.c (revision 179065)
+++ cp/parser.c (working copy)
@@ -19279,6 +19279,7 @@ cp_parser_late_parsing_attribute_arg_lis
cp_token_cache *tokens = (cp_token_cache *) TREE_VALUE
(artificial_node);
tree ctype;
VEC(tree,gc) *vec;
+ tree clone;
gcc_assert (tokens);
gcc_assert (decl decl != error_mark_node);
@@ -19322,16 +19323,9 @@ cp_parser_late_parsing_attribute_arg_lis
/* If decl has clones (when it is a ctor or a dtor), we need to
modify the clones' attributes as well. */
- if (TREE_CODE (decl) == FUNCTION_DECL
- (DECL_CONSTRUCTOR_P (decl) || DECL_DESTRUCTOR_P (decl)))
- {
- tree clone;
- for (clone = TREE_CHAIN (decl); clone; clone = TREE_CHAIN
(clone))
- {
- if (DECL_CLONED_FUNCTION (clone) == decl)
- DECL_ATTRIBUTES (clone) = DECL_ATTRIBUTES (decl);
- }
- }
+ FOR_EACH_CLONE (clone, decl)
+ if (DECL_CLONED_FUNCTION (clone) == decl)
+ DECL_ATTRIBUTES (clone) = DECL_ATTRIBUTES (decl);
pop_nested_class ();
--
DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
Re: [PATCH] [Annotalysis] Fix internal compiler error on template methods
No problem. Will do. -DeLesley On Thu, Sep 29, 2011 at 2:44 PM, Diego Novillo dnovi...@google.com wrote: On 11-09-29 17:24 , Delesley Hutchins wrote: That seems much cleaner! LGTM. Are you submitting the patch? I can submit the test case. I tested it on an unclean tree, sorry. I'd rather leave the testing and final submit to you. Thanks. Diego. -- DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
[PATCH] [Annotalysis] Fix bogus warnings caused by ipa-sra optimization
This patch suppresses bogus warnings that are caused when annotalysis
is run with the IPA-SRA optimization (i.e. -O2 or higher). IPA-SRA
creates clones of functions in which some arguments have been eliminated,
even if those arguments are used in the thread safety annotations;
this renders the attributes on such functions invalid. Annotalysis will now
detect when such clones have been created, and suppress certain
warnings to avoid false positives.
Bootstrapped and passed gcc regression testsuite on
x86_64-unknown-linux-gnu. Okay for google/gcc-4_6?
-DeLesley
cp/Changelog.google-4_6:
2011-9-27 DeLesley Hutchins deles...@google.com
* cp/tree-threadsafe-analyze.c (process_function_attrs)
detect clones and suppress warnings
testsuite/Changelog.google-4_6:
2011-9-27 DeLesley Hutchins deles...@google.com
* testsuite/g++.dg/thread-ann/thread_annot_lock-82.C
(new regression test)
Index: gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-82.C
===
--- gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-82.C (revision 0)
+++ gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-82.C (revision 0)
@@ -0,0 +1,30 @@
+// Test template methods in the presence of cloned constructors.
+// Regression test for bugfix.
+// { dg-do compile }
+// { dg-options -Wthread-safety -O3 }
+
+#include thread_annot_common.h
+
+class Foo;
+void do_something(void* a);
+
+class Foo {
+ Mutex mu_;
+
+ // with optimization turned on, ipa-sra should eliminate the hidden
+ // this argument, thus invalidating EXCLUSIVE_LOCKS_REQUIRED.
+ inline void clone_me_ipasra(Foo* f) EXCLUSIVE_LOCKS_REQUIRED(mu_) {
+do_something(f);
+ }
+
+ void foo(Foo* f);
+};
+
+void Foo::foo(Foo* f) {
+ mu_.Lock();
+ // in the cloned version, it looks like the required lock is f-mu_
+ // we should detect this and ignore the attribute.
+ clone_me_ipasra(f);
+ mu_.Unlock();
+}
+
Index: gcc/tree-threadsafe-analyze.c
===
--- gcc/tree-threadsafe-analyze.c (revision 179371)
+++ gcc/tree-threadsafe-analyze.c (working copy)
@@ -2129,6 +2129,7 @@ process_function_attrs (gimple call, tree fdecl,
tree base_obj = NULL_TREE;
bool is_exclusive_lock;
bool is_trylock;
+ bool bad_fun_args = false;
gcc_assert (is_gimple_call (call));
@@ -2147,16 +2148,28 @@ process_function_attrs (gimple call, tree fdecl,
!= NULL_TREE)
current_bb_info-writes_ignored = false;
+ /* If the given function is a clone, and if some of the parameters of the
+ clone have been optimized away, then the function attribute is no longer
+ correct, and we should supress certain warnings. Clones are often created
+ when -fipa-sra is enabled, which happens by default at -O2 starting from
+ GCC-4.5. The clones could be created as early as when constructing SSA.
+ ipa-sra is particularly fond of optimizing away the this pointer,
+ which is a problem because that makes it impossible to determine the
+ base object, which then causes spurious errors. It's better to just
+ remain silent in this case. */
+ if ((TREE_CODE (TREE_TYPE (fdecl)) == FUNCTION_TYPE
+ || TREE_CODE (TREE_TYPE (fdecl)) == METHOD_TYPE) /* sanity check */
+ (fdecl != DECL_ORIGIN (fdecl)) /* is it a clone? */
+ (type_num_arguments (TREE_TYPE (fdecl)) != /* compare args */
+ type_num_arguments (TREE_TYPE (DECL_ORIGIN(fdecl)
+bad_fun_args = true;
+
/* If the function is a class member, the first argument of the function
(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. Also note that the parameters of a cloned method could
- be optimized away. */
+ its base object. */
if (TREE_CODE (TREE_TYPE (DECL_ORIGIN (fdecl))) == METHOD_TYPE
gimple_call_num_args(call) 0)
base_obj = gimple_call_arg (call, 0);
@@ -2285,7 +2298,8 @@ process_function_attrs (gimple call, tree fdecl,
current_bb_info, locus);
}
- if (warn_thread_unguarded_func)
+ /* suppress warnings if the function arguments are no longer accurate. */
+ if (warn_thread_unguarded_func !bad_fun_args)
{
/* Handle the attributes specifying the lock requirements of
functions. */
--
DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
[PATCH] [Annotalysis] Fix internal compiler error on template methods
This patch fixes a bug in the parser which cause an internal compiler
error when copying attributes from cloned methods. The bug occurs
when a class has both an annotated constructor and a template method.
Bootstrapped and passed gcc regression testsuite on
x86_64-unknown-linux-gnu. Okay for google/gcc-4_6?
-DeLesley
cp/Changelog.google-4_6:
2011-9-27 DeLesley Hutchins deles...@google.com
* cp/parser.c (cp_parser_late_parsing_attribute_arg_lists)
fixed case where the potential clone is a template.
testsuite/Changelog.google-4_6:
2011-9-27 DeLesley Hutchins deles...@google.com
* testsuite/g++.dg/thread-ann/thread_annot_lock-81.C
(new regression test)
--
DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
Index: testsuite/g++.dg/thread-ann/thread_annot_lock-81.C
===
--- testsuite/g++.dg/thread-ann/thread_annot_lock-81.C (revision 0)
+++ testsuite/g++.dg/thread-ann/thread_annot_lock-81.C (revision 0)
@@ -0,0 +1,17 @@
+// Test template methods in the presence of cloned constructors.
+// Regression test for bugfix.
+// { dg-do compile }
+// { dg-options -Wthread-safety -O }
+
+#include thread_annot_common.h
+
+Mutex mu_;
+Mutex mu2_;
+
+class Foo {
+ Foo() LOCKS_EXCLUDED(mu_) { }
+
+ template class T
+ void bar(T* t) LOCKS_EXCLUDED(mu2_) { }
+};
+
Index: cp/parser.c
===
--- cp/parser.c (revision 179283)
+++ cp/parser.c (working copy)
@@ -19328,7 +19328,8 @@ cp_parser_late_parsing_attribute_arg_lists (cp_par
tree clone;
for (clone = TREE_CHAIN (decl); clone; clone = TREE_CHAIN (clone))
{
- if (DECL_CLONED_FUNCTION (clone) == decl)
+ tree* clonefun = DECL_CLONED_FUNCTION (clone);
+ if (clonefun *clonefun == decl)
DECL_ATTRIBUTES (clone) = DECL_ATTRIBUTES (decl);
}
}
[PATCH] [Annotalysis] Fix internal compiler errors in thread safety analysis.
This patch fixes two bugs which cause an internal compiler error in
annotalysis. The first change fixes a failure when a variable is
typecast from an unknown (forward-defined) type. The second change
fixes a case in which an assert was triggered, because subexpressions
were not canonicalized correctly.
Bootstrapped and passed gcc regression testsuite on
x86_64-unknown-linux-gnu. Tested on google core components.
Okay for google/gcc-4_6?
-DeLesley
gcc/Changelog.annotalysis:
2011-9-13 DeLesley Hutchins deles...@google.com
* gcc/cp/class.c (cp_get_virtual_function_decl) bugfix where
type is uknown
* gcc/tree-threadsafe-analyze.c (get_canonical_lock_expr)
don't remove on recursive call
Index: gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-79.C
===
--- gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-79.C (revision
178784)
+++ gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-79.C (working copy)
@@ -75,3 +75,27 @@ void foo3()
(new FooDerived)-doSomething();
}
+class FooNodef;
+
+// test case for cast from undefined type
+void foo4(FooNodef *f) {
+ ((Foo*) f)-doSomething();
+}
+
+
+// Regression test for canonicalization of subexpressions that refer to
+// lockable objects.
+class LOCKABLE Base {
+public:
+ Mutex mu_;
+ virtual void baseMethod() SHARED_LOCKS_REQUIRED(mu_) = 0;
+};
+
+class Derived : public Base {
+public:
+ void foo() SHARED_LOCKS_REQUIRED(mu_);
+};
+
+void Derived::foo() {
+ baseMethod();
+}
Index: gcc/cp/class.c
===
--- gcc/cp/class.c (revision 178784)
+++ gcc/cp/class.c (working copy)
@@ -8384,9 +8384,15 @@ cp_get_virtual_function_decl (tree ref, tree known
{
HOST_WIDE_INT index = tree_low_cst (OBJ_TYPE_REF_TOKEN (ref), 1);
HOST_WIDE_INT i = 0;
- tree v = BINFO_VIRTUALS (TYPE_BINFO (known_type));
+ tree binfo = TYPE_BINFO(known_type);
+ tree v;
tree fndecl;
-
+
+ if (!binfo)
+return NULL_TREE;
+
+ v = BINFO_VIRTUALS (TYPE_BINFO (known_type));
+
while (v i != index)
{
i += (TARGET_VTABLE_USES_DESCRIPTORS
Index: gcc/tree-threadsafe-analyze.c
===
--- gcc/tree-threadsafe-analyze.c (revision 178784)
+++ gcc/tree-threadsafe-analyze.c (working copy)
@@ -884,10 +884,12 @@ get_canonical_lock_expr (tree lock, tree base_obj,
{
tree base = TREE_OPERAND (lock, 0);
tree canon_base;
- /* When the expr is a pointer to a lockable type (i.e. mu.Lock()
+ /* For expressions that denote locks,
+ When the expr is a pointer to a lockable type (i.e. mu.Lock()
or Lock(mu) internally), we don't need the address-taken
operator (). */
- if (lookup_attribute(lockable, TYPE_ATTRIBUTES (TREE_TYPE (base
+ if (!is_temp_expr
+ lookup_attribute(lockable, TYPE_ATTRIBUTES (TREE_TYPE (base
return get_canonical_lock_expr (base, base_obj,
false /* is_temp_expr */,
new_leftmost_base_var);
--
DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
[google] Backport r176188, r176489 from google/main to google/gcc-4_6 branch.
Committed. -- DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
[PATCH] [google] [annotalysis] Fix remove operation from pointer_set in case of hash collisions
This patch fixes a bug in pointer_set.c, where removing a pointer from
a pointer set would corrupt the hash table if the pointer was involved
in any hash collisions.
Bootstrapped and passed gcc regression testsuite on x86_64-unknown-linux-gnu.
Okay for google/gcc-4_6?
-DeLesley
gcc/Changelog.annotalysis:
2011-7-26 DeLesley Hutchins deles...@google.com
* gcc/pointer-set.c (pointer_set_delete) bugfix for case of
hash collisions
Index: gcc/pointer-set.c
===
--- gcc/pointer-set.c (revision 176809)
+++ gcc/pointer-set.c (working copy)
@@ -192,19 +192,16 @@ int
pointer_set_delete (struct pointer_set_t *pset, const void *p)
{
size_t n = hash1 (p, pset-n_slots, pset-log_slots);
+ size_t n2;
+ const void* ptr;
+ /* find location of p */
while (true)
{
if (pset-slots[n] == p)
-{
- pset-slots[n] = 0;
- --pset-n_elements;
- return 1;
-}
+break;
else if (pset-slots[n] == 0)
-{
- return 0;
-}
+return 0;
else
{
++n;
@@ -212,6 +209,29 @@ pointer_set_delete (struct pointer_set_t *pset, co
n = 0;
}
}
+
+ /* Remove p from set. */
+ pset-slots[n] = 0;
+
+ /* Now we need to scan foward and re-hash every value that we encounter,
+ until we find an empty slot.
+ */
+ while (true)
+{
+ ++n;
+ if (n = pset-n_slots)
+n = 0;
+ ptr = pset-slots[n];
+
+ if (ptr == 0) break;
+
+ pset-slots[n] = 0; /* remove ptr from set. */
+ n2 = insert_aux(ptr, pset-slots, pset-n_slots, pset-log_slots);
+ pset-slots[n2] = ptr; /* put ptr back in set. */
+}
+
+ --pset-n_elements;
+ return 1;
}
/* Pass each pointer in PSET to the function in FN, together with the fixed
--
DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
Re: [PATCH] [google] [annotalysis] Fix remove operation from pointer_set in case of hash collisions
Le-Chun added the additional routine to remove pointers from a set; that code is unique to annotalysis. I can't easily include a test case, because the bug is difficult to trigger. It occurs only when there is a hash collision between two pointers in the set, and the first pointer is removed before the second. I do have a test case, but it will only work for my particular build on my machine, since the actual pointer addresses involved will change as soon as you touch something. I could write a unit test using bogus pointer values that are engineered to trigger a collision, but it wouldn't be a normal compiler test case; where would I put it? -DeLesley On Tue, Jul 26, 2011 at 5:59 PM, Diego Novillo dnovi...@google.com wrote: On Tue, Jul 26, 2011 at 16:13, Delesley Hutchins deles...@google.com wrote: This patch fixes a bug in pointer_set.c, where removing a pointer from a pointer set would corrupt the hash table if the pointer was involved in any hash collisions. Could you include a test case? It's not clear to me what you are fixing and when this happens. Is this a bug in trunk as well? The pointer-set implementation has been around for a while, so I'm surprised that you are running into this now. Or is this something that only happens with the pointer set changes we have in for annotalysis? Thanks. Diego. -- DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
Re: [google] Disable annotalysis in google/main (issue4816050)
Looks Good To Me.
-DeLesley
On Mon, Jul 25, 2011 at 2:04 PM, Diego Novillo dnovi...@google.com wrote:
Annotalysis has been broken in google/main since the last merge from
trunk. This patch disables it until Delesley comes up with a
permanent solution.
Tested on x86_64. OK for google/main?
2011-07-25 Diego Novillo dnovi...@google.com
* tree-threadsafe-analyze.c (gate_threadsafe_analyze): Always
return false.
2011-07-25 Diego Novillo dnovi...@google.com
* gcc/testsuite/g++.dg/dg.exp: Remove tests in directory
thread-ann.
Index: gcc/testsuite/g++.dg/dg.exp
===
--- gcc/testsuite/g++.dg/dg.exp (revision 176640)
+++ gcc/testsuite/g++.dg/dg.exp (working copy)
@@ -48,6 +48,7 @@ set tests [prune $tests $srcdir/$subdir/
set tests [prune $tests $srcdir/$subdir/torture/*]
set tests [prune $tests $srcdir/$subdir/graphite/*]
set tests [prune $tests $srcdir/$subdir/guality/*]
+set tests [prune $tests $srcdir/$subdir/thread-ann/*]
# Main loop.
dg-runtest $tests $DEFAULT_CXXFLAGS
Index: gcc/tree-threadsafe-analyze.c
===
--- gcc/tree-threadsafe-analyze.c (revision 176640)
+++ gcc/tree-threadsafe-analyze.c (working copy)
@@ -3571,7 +3571,8 @@ execute_threadsafe_analyze (void)
static bool
gate_threadsafe_analyze (void)
{
- return warn_thread_safety != 0;
+ /* FIXME google/main - Annotalysis is currently broken. */
+ return false;
}
struct gimple_opt_pass pass_threadsafe_analyze =
--
This patch is available for review at http://codereview.appspot.com/4816050
--
DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
[PATCH] [Annotalysis] Fix to memory corruption issue in Annotalysis
This patch changes lock_expr_tab so that it is allocated by the
garbage collector, and properly marked as a GC root.
Bootstrapped and passed GCC regression testsuite on x86_64-unknown-linux-gnu.
Okay for branches/annotalysis and google/main?
-DeLesley
2011-07-19 DeLesley Hutchins deles...@google.com
* tree-threadsafe-analyze.c: Changes lock_expr_tab to be allocated by
the garbage collector, and marked as a GC root.
Index: gcc/Makefile.in
===
--- gcc/Makefile.in (revision 176187)
+++ gcc/Makefile.in (working copy)
@@ -3762,6 +3762,7 @@ GTFILES = $(CPP_ID_DATA_H) $(srcdir)/input.h $(src
$(srcdir)/tree-iterator.c $(srcdir)/gimplify.c \
$(srcdir)/tree-chrec.h \
$(srcdir)/tree-scalar-evolution.c \
+ $(srcdir)/tree-threadsafe-analyze.c \
$(srcdir)/tree-ssa-operands.h \
$(srcdir)/tree-profile.c $(srcdir)/tree-nested.c \
$(srcdir)/varpool.c \
Index: gcc/tree-threadsafe-analyze.c
===
--- gcc/tree-threadsafe-analyze.c (revision 176188)
+++ gcc/tree-threadsafe-analyze.c (working copy)
@@ -239,11 +239,11 @@ static struct pointer_map_t *lock_locus_map;
/* Each entry maps a lock to its canonicalized expression (see
get_canonical_lock_expr()). */
-static htab_t lock_expr_tab;
+static GTY((param_is (union tree_node))) htab_t lock_expr_tab = NULL;
/* Each entry is a gimple call statement. Calls to the same function with
symbolically identical arguments will hash to the same entry. */
-static htab_t gimple_call_tab;
+static htab_t gimple_call_tab = NULL;
/* Each entry maps a trylock call expr to its trylock_info. */
static struct pointer_map_t *trylock_info_map;
@@ -517,11 +517,8 @@ destroy_acquired_after_set (const void * ARG_UNUSE
void
clean_up_threadsafe_analysis (void)
{
- if (lock_expr_tab)
-{
- htab_delete (lock_expr_tab);
- lock_expr_tab = NULL;
-}
+ /* lock_expr_tab is garbage collected. */
+ lock_expr_tab = NULL;
/* Free the lock acquired_after map and the sets */
if (lock_acquired_after_map)
@@ -989,7 +984,7 @@ get_canonical_lock_expr (tree lock, tree base_obj,
in the lock_expr_tab. If so, grab and return it. Otherwise, insert the
new lock expr to the map. */
if (lock_expr_tab == NULL)
-lock_expr_tab = htab_create (10, lock_expr_hash, lock_expr_eq, NULL);
+lock_expr_tab = htab_create_ggc (10, lock_expr_hash, lock_expr_eq, NULL);
canon_lock = (tree) htab_find_with_hash (lock_expr_tab, lock, hash);
if (canon_lock)
@@ -3560,6 +3555,7 @@ execute_threadsafe_analyze (void)
pointer_map_traverse (trylock_info_map, delete_trylock_info, NULL);
pointer_map_destroy (trylock_info_map);
htab_delete (gimple_call_tab);
+ gimple_call_tab = NULL;
/* The flag that controls the warning of mismatched lock acquire/release
could be turned off when we see an unlock primitive with an unsupported
@@ -3596,3 +3592,5 @@ struct gimple_opt_pass pass_threadsafe_analyze =
TODO_dump_func/* todo_flags_finish */
}
};
+
+#include gt-tree-threadsafe-analyze.h
[PATCH] [Annotalysis] Fix to get_canonical_lock_expr
This patch fixes get_canonical_lock_expr so that it works on lock
expressions that involve a MEM_REF. Gimple code can use either
MEM_REF or INDIRECT_REF in many expressions, and the choice of which
to use is somewhat arbitrary. The canonical form of a lock expression
must rewrite all MEM_REFs to INDIRECT_REFs to accurately compare
expressions. The surrounding if block prevented this rewrite from
happening in certain cases.
Bootstrapped and passed GCC regression testsuite on x86_64-unknown-linux-gnu.
Okay for branches/annotalysis and google/main?
-DeLesley
2011-07-06 DeLesley Hutchins deles...@google.com
* cp_get_virtual_function_decl.c (handle_call_gs): Changes
function to return null if the method cannot be found.
* thread_annot_lock-79.C: Additional annotalysis test cases
Index: gcc/tree-threadsafe-analyze.c
===
--- gcc/tree-threadsafe-analyze.c (revision 176188)
+++ gcc/tree-threadsafe-analyze.c (working copy)
@@ -959,19 +959,17 @@ get_canonical_lock_expr (tree lock, tree base_obj,
tree canon_base = get_canonical_lock_expr (base, base_obj,
true /* is_temp_expr */,
new_leftmost_base_var);
- if (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);
-}
+
+ /* 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:
--
DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
Re: [PATCH] [Annotalysis] Fixes virtual method calls when type is unknown
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. Annotalysis is just a static analyzer, so if the types are somehow inaccurate (as they are in certain cases), then the only ill effect will be that the analysis issues a warning where it shouldn't, or fails to issue a warning where it should. Of course, no analysis that's based on types will ever be sound for C++, because C++ is not type-safe. We merely assume that the declared types represent the intentions of the programmer, and hope that the declared types are preserved in gimple with some degree of fidelity. Just out of curiosity, I does your analysis crash also on the testcase below (add the Mutex field if it is necessary)? No, the static type of b (in b-test()) remains Bar* in gimple, so the analysis works fine (i.e. the analyzer will not crash). 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...? It doesn't matter whether it's overridden or not, since any overriding versions are required to have the same type signature as the original. Annotalysis is merely an extended type-checker. And pretty please, use -p diff option when creating patches to be posted here. My apologies; will do. -DeLesley
Re: [PATCH] [Annotalysis] Fixes virtual method calls when type is unknown
This is indeed a problem. Good catch; thanks!
-DeLesley
On Fri, Jul 1, 2011 at 11:50 AM, Martin Jambor mjam...@suse.cz wrote:
Hi,
On Fri, Jul 01, 2011 at 09:31:30AM -0700, Delesley Hutchins wrote:
Just out of curiosity, I does your
analysis crash also on the testcase below (add the Mutex field if it
is necessary)?
No, the static type of b (in b-test()) remains Bar* in gimple, so the
analysis works fine (i.e. the analyzer will not crash).
Sorry, I've had a look only just now and you do the analysis before
early inlining and as long as you do that, the type is really Bar and
you are fine as far as far as placement new is concerned.
However, on the second thought, I still think you need to handle the
case when BINFO_VIRTUALS are NULL in cp_get_virtual_function_decl
(BTW, there is a non-langhook variant in gimple-fold.c called
gimple_get_virt_method_for_binfo) because of the case below.
Martin
// { dg-do compile }
// { dg-options -Wthread-safety -O }
//#include thread_annot_common.h
class Anc {
public:
int a;
};
class Foo : public Anc {
public:
// Mutex m;
Foo();
virtual ~Foo();
virtual void doSomething() = 0;// EXCLUSIVE_LOCKS_REQUIRED(m) = 0;
};
class FooDerived : public Foo {
public:
FooDerived();
virtual ~FooDerived();
virtual void doSomething();
};
// reinterpret_cast
void foo1(Anc* ptr)
{
reinterpret_castFoo*(ptr)-doSomething();
}
// C-style cast
void foo2(Anc* buf)
{
((Foo*) buf)-doSomething();
}
--
DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
[PATCH] [Annotalysis] Fixes virtual method calls when type is unknown
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_castFoo*(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);
+}
}
}
[PATCH] [annotalysis] Merge of google/integration into annotalysis
This patch merges recent changes from google/integration into
branches/annotalysis.
Bootstrapped and passed GCC regression testsuite on x86_64-unknown-linux-gnu.
Okay for branches/annotalysis?
--
DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315
Property changes on: .
___
Modified: svnmerge-integrated
- /branches/google/integration:1-171014 /trunk:1-170776,170779-170934
+ /branches/google/integration:1-175319 /trunk:1-170776,170779-170934
Modified: svn:mergeinfo
Merged /trunk:r171161
Merged /branches/google/integration:r171167-175149
Index: libstdc++-v3/scripts/extract_symvers.in
===
--- libstdc++-v3/scripts/extract_symvers.in (revision 175318)
+++ libstdc++-v3/scripts/extract_symvers.in (working copy)
@@ -52,6 +52,9 @@
${readelf} ${lib} |\
sed -e 's/ \[other: [A-Fa-f0-9]*\] //' -e '/\.dynsym/,/^$/p;d' |\
egrep -v ' (LOCAL|UND) ' |\
+ sed -e 's/ processor specific: / processor_specific:_/g' |\
+ sed -e 's/ OS specific: / OS_specific:_/g' |\
+ sed -e 's/ unknown: / unknown:_/g' |\
awk '{ if ($4 == FUNC || $4 == NOTYPE)
printf %s:%s\n, $4, $8;
else if ($4 == OBJECT || $4 == TLS)
Index: libstdc++-v3/src/Makefile.in
===
--- libstdc++-v3/src/Makefile.in (revision 175318)
+++ libstdc++-v3/src/Makefile.in (working copy)
@@ -484,7 +484,8 @@
$(XTEMPLATE_FLAGS) \
$(WARN_CXXFLAGS) \
$(OPTIMIZE_CXXFLAGS) \
- $(CONFIG_CXXFLAGS)
+ $(CONFIG_CXXFLAGS) \
+ $($(@)_no_omit_frame_pointer)
# libstdc++ libtool notes
@@ -522,6 +523,9 @@
$(CXX) $(OPT_LDFLAGS) $(SECTION_LDFLAGS) $(AM_CXXFLAGS) $(LTLDFLAGS) -o $@
debugdir = debug
+
+# Google-specific pessimization
+functexcept.lo_no_omit_frame_pointer = -fno-omit-frame-pointer
all: all-am
.SUFFIXES:
Index: libstdc++-v3/src/Makefile.am
===
--- libstdc++-v3/src/Makefile.am (revision 175318)
+++ libstdc++-v3/src/Makefile.am (working copy)
@@ -395,7 +395,8 @@
$(XTEMPLATE_FLAGS) \
$(WARN_CXXFLAGS) \
$(OPTIMIZE_CXXFLAGS) \
- $(CONFIG_CXXFLAGS)
+ $(CONFIG_CXXFLAGS) \
+ $($(@)_no_omit_frame_pointer)
# libstdc++ libtool notes
@@ -469,3 +470,6 @@
install_debug:
(cd ${debugdir} $(MAKE) \
toolexeclibdir=$(glibcxx_toolexeclibdir)/debug install)
+
+# Google-specific pessimization
+functexcept.lo_no_omit_frame_pointer = -fno-omit-frame-pointer
Index: libstdc++-v3/include/ext/vstring.h
===
--- libstdc++-v3/include/ext/vstring.h (revision 175318)
+++ libstdc++-v3/include/ext/vstring.h (working copy)
@@ -37,6 +37,21 @@
#include ext/rc_string_base.h
#include ext/sso_string_base.h
+#if __google_stl_debug_string !defined(_GLIBCXX_DEBUG)
+# undef _GLIBCXX_DEBUG_ASSERT
+# undef _GLIBCXX_DEBUG_PEDASSERT
+// Perform additional checks (but only in this file).
+# define _GLIBCXX_DEBUG_ASSERT(_Condition) \
+ if (! (_Condition)) {\
+char buf[512]; \
+__builtin_snprintf(buf, sizeof(buf), \
+ %s:%d: %s: Assertion '%s' failed.\n, \
+ __FILE__, __LINE__, __func__, # _Condition); \
+std::__throw_runtime_error(buf); \
+ }
+# define _GLIBCXX_DEBUG_PEDASSERT(_Condition) _GLIBCXX_DEBUG_ASSERT(_Condition)
+#endif
+
namespace __gnu_cxx _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -2793,4 +2808,12 @@
#include vstring.tcc
+#if __google_stl_debug_string !defined(_GLIBCXX_DEBUG)
+// Undo our defines, so they don't affect anything else.
+# undef _GLIBCXX_DEBUG_ASSERT
+# undef _GLIBCXX_DEBUG_PEDASSERT
+# define _GLIBCXX_DEBUG_ASSERT(_Condition)
+# define _GLIBCXX_DEBUG_PEDASSERT(_Condition)
+#endif
+
#endif /* _VSTRING_H */
Index: libstdc++-v3/include/ext/sso_string_base.h
===
--- libstdc++-v3/include/ext/sso_string_base.h (revision 175318)
+++ libstdc++-v3/include/ext/sso_string_base.h (working copy)
@@ -86,6 +86,13 @@
{
if (!_M_is_local())
_M_destroy(_M_allocated_capacity);
+#if __google_stl_debug_string_dangling
+ else {
+ // Wipe local storage for destructed string with 0xCD.
+ // This mimics what DebugAllocation does to free()d memory.
+ __builtin_memset(_M_local_data, 0xcd, sizeof(_M_local_data));
+}
+#endif
}
void
@@ -169,15 +176,29 @@
_M_leak() { }
void
- _M_set_length(size_type __n)
+ _M_set_length_no_wipe(size_type __n)
{
_M_length(__n);
traits_type::assign(_M_data()[__n], _CharT());
}
+ void
+ _M_set_length
