Follow-up of Dave's patch. I would prefer to see such checks in
trans-mem.c as follows.
In a transaction, a function pointer can be declared and assigned but
there is no check that the function pointer is transaction_safe. So at
runtime, if the function was unsafe, libitm stops on assert because the
clone is not found.
Tested on i686.
Is the patch ok? Thanks.
BTW, Should we generate a warning or an error?
--
2012-05-15 Patrick Marlier <patrick.marl...@gmail.com>
* trans-mem.c (diagnose_tm_1_op): Warn about assignment of
transaction
unsafe function to safe function pointer.
testsuite/
2012-05-15 Patrick Marlier <patrick.marl...@gmail.com>
* c-c++-common/tm/indirect-1.c: New test.
On 05/15/2012 12:23 PM, Torvald Riegel wrote:
On Tue, 2012-05-08 at 18:02 -0500, Dave Boutcher wrote:
Without this patch it is perfectly fine to assign non-transaction_safe
functions to function pointers marked as transaction_safe. Unpleasantness
happens at run time.
e.g.
__attribute__((transaction_safe)) long (*compare)(int, int);
compare = my_funky_random_function;
gcc/c-typeck.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/gcc/c-typeck.c b/gcc/c-typeck.c
index fc01a79..69687d6 100644
--- a/gcc/c-typeck.c
+++ b/gcc/c-typeck.c
@@ -5608,6 +5608,13 @@ convert_for_assignment (location_t location, tree type,
tree rhs,
}
}
+ /* Check for assignment to transaction safe */
+ if (is_tm_safe(type)&& !is_tm_safe_or_pure (rhs)) {
I don't think that assigning a tm_pure function to tm_safe works. There
will be no instrumented version of it. I don't think we generate an
alias or sth like that yet.
When contributing patches, please submit testcases along with it. For
example, regarding this particular problem, I would believe that there
are more cases that we don't check properly yet.
Also, did you do the FSF copyright assignment paperwork yet?
Torvald
Index: trans-mem.c
===================================================================
--- trans-mem.c (revision 187371)
+++ trans-mem.c (working copy)
@@ -572,6 +572,16 @@ diagnose_tm_1_op (tree *tp, int *walk_subtrees ATT
*tp);
}
+ if (code == VAR_DECL
+ && TREE_CODE (TREE_TYPE (*tp)) == POINTER_TYPE
+ && ((d->block_flags & (DIAG_TM_SAFE | DIAG_TM_RELAXED))
+ || (d->func_flags & DIAG_TM_SAFE))
+ && is_gimple_assign (d->stmt)
+ && is_tm_safe (*tp)
+ && !is_tm_safe (gimple_assign_rhs1 (d->stmt)))
+ warning_at (gimple_location (d->stmt), 0, "Assigning unsafe function to "
+ "transaction_safe function pointer");
+
return NULL_TREE;
}
Index: testsuite/c-c++-common/tm/indirect-1.c
===================================================================
--- testsuite/c-c++-common/tm/indirect-1.c (revision 0)
+++ testsuite/c-c++-common/tm/indirect-1.c (revision 0)
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm" } */
+
+__attribute__((transaction_safe)) void (*func_ptr)(void);
+
+void bar(void);
+
+__attribute__((transaction_safe))
+void foo1(void)
+{
+ func_ptr = bar; /* { dg-warning "Assigning unsafe function to transaction_safe" } */
+}
+
+void foo2(void)
+{
+ func_ptr = bar;
+}
+
+void foo3(void)
+{
+ __transaction_atomic {
+ func_ptr = bar; /* { dg-warning "Assigning unsafe function to transaction_safe" } */
+ }
+}
+
