| Submitter | Dave Boutcher |
|---|---|
| Date | May 8, 2012, 11:02 p.m. |
| Message ID | <20120508230202.F3FD2DC03AD@sandy> |
| Download | mbox | patch |
| Permalink | /patch/157831/ |
| State | New |
| Headers | show |
Comments
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
On Tue, May 15, 2012 at 11:23 AM, Torvald Riegel <triegel@redhat.com> 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; > > > > > 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. Torvald, we do create an alias... In trans-mem.c around 4789 /* ... marked tm_pure, record that fact for the runtime by indicating that the pure function is its own tm_callable. No need to do this if the function's address can't be taken. */ if (is_tm_pure (node->decl)) { if (!node->local.local) { record_tm_clone_pair (node->decl, node->decl); creates an entry in the clone table that points to itself so its safe to later call _ITM_getTMCloneSafe() > 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. Yeah, very likely. This one was biting me so I added the check. > Also, did you do the FSF copyright assignment paperwork yet? I sent off the form... -- Dave B
Patch
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)) { + warning_at (location, 0, + "Assigning unsafe function to transaction_safe " + "function pointer"); + } + /* Any non-function converts to a [const][volatile] void * and vice versa; otherwise, targets must be the same. Meanwhile, the lhs target must have all the qualifiers of the rhs. */