| Message ID | 20160919220244.GS7282@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 20 Sep 2016, Jakub Jelinek wrote: > Hi! > > fold_builtin_atomic_always_lock_free has an assertion that if the 2nd > argument isn't INTEGER_CST, it has POINTER_TYPE_P, but additionally for > casts to void * it looks through the cast. That means though for invalid > code if an integral expression etc. is converted to pointer, we violate the > assertion. This patch instead doesn't look through such casts, so > type_align is effectively alignment of void and thus it considers objects > insufficiently aligned, but doesn't ICE. > > Another approach (incomplete) is what I've attached to the PR - instead of > prototyping __atomic_{always,is}_lock_free it uses ... and only checks that > it has 2 arguments and that the last one is a pointer - that way we don't > have to try to undo implicit cast to void * and know the conversion in there > is the user conversion. Unfortunately the C++ FE isn't prepared for the > check_builtin_function_arguments to change the arguments (unlike C). > > I've bootstrapped/regtested on x86_64-linux and i686-linux the following > fix, ok for trunk, or should I invest more time into the other approach? Ok for trunk. I think the other solution would be to somehow mark the builtin to not get the (void *) cast applied in the first place ... (make it varargs?) Thanks, Richard. > 2016-09-19 Jakub Jelinek <jakub@redhat.com> > > PR middle-end/77624 > * builtins.c (fold_builtin_atomic_always_lock_free): Only look through > cast to void * if the cast is from some other pointer type. > > * c-c++-common/pr77624-1.c: New test. > * c-c++-common/pr77624-2.c: New test. > > --- gcc/builtins.c.jj 2016-09-16 22:19:38.000000000 +0200 > +++ gcc/builtins.c 2016-09-19 15:28:41.100412521 +0200 > @@ -5575,8 +5575,10 @@ fold_builtin_atomic_always_lock_free (tr > end before anything else has a chance to look at it. The pointer > parameter at this point is usually cast to a void *, so check for that > and look past the cast. */ > - if (CONVERT_EXPR_P (arg1) && POINTER_TYPE_P (ttype) > - && VOID_TYPE_P (TREE_TYPE (ttype))) > + if (CONVERT_EXPR_P (arg1) > + && POINTER_TYPE_P (ttype) > + && VOID_TYPE_P (TREE_TYPE (ttype)) > + && POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (arg1, 0)))) > arg1 = TREE_OPERAND (arg1, 0); > > ttype = TREE_TYPE (arg1); > --- gcc/testsuite/c-c++-common/pr77624-1.c.jj 2016-09-19 15:18:09.049394093 +0200 > +++ gcc/testsuite/c-c++-common/pr77624-1.c 2016-09-19 15:41:09.972949880 +0200 > @@ -0,0 +1,14 @@ > +/* PR middle-end/77624 */ > +/* { dg-do compile } */ > + > +int > +foo (int a) > +{ > + return __atomic_is_lock_free (2, a); /* { dg-warning "pointer from integer" "" { target c } } */ > +} /* { dg-error "invalid conversion" "" { target c++ } 7 } */ > + > +int > +bar (int a) > +{ > + return __atomic_always_lock_free (2, a); /* { dg-warning "pointer from integer" "" { target c } } */ > +} /* { dg-error "invalid conversion" "" { target c++ } 13 } */ > --- gcc/testsuite/c-c++-common/pr77624-2.c.jj 2016-09-19 15:18:12.280353292 +0200 > +++ gcc/testsuite/c-c++-common/pr77624-2.c 2016-09-19 15:42:37.441844581 +0200 > @@ -0,0 +1,26 @@ > +/* PR middle-end/77624 */ > +/* { dg-do compile } */ > + > +void > +foo (int *a) > +{ > + double b = 0; > + __atomic_is_lock_free (2, a, 2); /* { dg-error "too many arguments" } */ > + __atomic_is_lock_free (2); /* { dg-error "too few arguments" } */ > + __atomic_is_lock_free (2, b); /* { dg-error "incompatible type" "" { target c } } */ > + /* { dg-message "expected" "" { target c } 10 } */ > + /* { dg-error "convert" "" { target c++ } 10 } */ > + __atomic_is_lock_free (2, 0); > +} > + > +void > +bar (int *a) > +{ > + double b = 0; > + __atomic_always_lock_free (2, a, 2); /* { dg-error "too many arguments" } */ > + __atomic_always_lock_free (2); /* { dg-error "too few arguments" } */ > + __atomic_always_lock_free (2, b); /* { dg-error "incompatible type" "" { target c } } */ > + /* { dg-message "expected" "" { target c } 22 } */ > + /* { dg-error "convert" "" { target c++ } 22 } */ > + __atomic_always_lock_free (2, 0); > +} > > Jakub > >
--- gcc/builtins.c.jj 2016-09-16 22:19:38.000000000 +0200 +++ gcc/builtins.c 2016-09-19 15:28:41.100412521 +0200 @@ -5575,8 +5575,10 @@ fold_builtin_atomic_always_lock_free (tr end before anything else has a chance to look at it. The pointer parameter at this point is usually cast to a void *, so check for that and look past the cast. */ - if (CONVERT_EXPR_P (arg1) && POINTER_TYPE_P (ttype) - && VOID_TYPE_P (TREE_TYPE (ttype))) + if (CONVERT_EXPR_P (arg1) + && POINTER_TYPE_P (ttype) + && VOID_TYPE_P (TREE_TYPE (ttype)) + && POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (arg1, 0)))) arg1 = TREE_OPERAND (arg1, 0); ttype = TREE_TYPE (arg1); --- gcc/testsuite/c-c++-common/pr77624-1.c.jj 2016-09-19 15:18:09.049394093 +0200 +++ gcc/testsuite/c-c++-common/pr77624-1.c 2016-09-19 15:41:09.972949880 +0200 @@ -0,0 +1,14 @@ +/* PR middle-end/77624 */ +/* { dg-do compile } */ + +int +foo (int a) +{ + return __atomic_is_lock_free (2, a); /* { dg-warning "pointer from integer" "" { target c } } */ +} /* { dg-error "invalid conversion" "" { target c++ } 7 } */ + +int +bar (int a) +{ + return __atomic_always_lock_free (2, a); /* { dg-warning "pointer from integer" "" { target c } } */ +} /* { dg-error "invalid conversion" "" { target c++ } 13 } */ --- gcc/testsuite/c-c++-common/pr77624-2.c.jj 2016-09-19 15:18:12.280353292 +0200 +++ gcc/testsuite/c-c++-common/pr77624-2.c 2016-09-19 15:42:37.441844581 +0200 @@ -0,0 +1,26 @@ +/* PR middle-end/77624 */ +/* { dg-do compile } */ + +void +foo (int *a) +{ + double b = 0; + __atomic_is_lock_free (2, a, 2); /* { dg-error "too many arguments" } */ + __atomic_is_lock_free (2); /* { dg-error "too few arguments" } */ + __atomic_is_lock_free (2, b); /* { dg-error "incompatible type" "" { target c } } */ + /* { dg-message "expected" "" { target c } 10 } */ + /* { dg-error "convert" "" { target c++ } 10 } */ + __atomic_is_lock_free (2, 0); +} + +void +bar (int *a) +{ + double b = 0; + __atomic_always_lock_free (2, a, 2); /* { dg-error "too many arguments" } */ + __atomic_always_lock_free (2); /* { dg-error "too few arguments" } */ + __atomic_always_lock_free (2, b); /* { dg-error "incompatible type" "" { target c } } */ + /* { dg-message "expected" "" { target c } 22 } */ + /* { dg-error "convert" "" { target c++ } 22 } */ + __atomic_always_lock_free (2, 0); +}
Hi! fold_builtin_atomic_always_lock_free has an assertion that if the 2nd argument isn't INTEGER_CST, it has POINTER_TYPE_P, but additionally for casts to void * it looks through the cast. That means though for invalid code if an integral expression etc. is converted to pointer, we violate the assertion. This patch instead doesn't look through such casts, so type_align is effectively alignment of void and thus it considers objects insufficiently aligned, but doesn't ICE. Another approach (incomplete) is what I've attached to the PR - instead of prototyping __atomic_{always,is}_lock_free it uses ... and only checks that it has 2 arguments and that the last one is a pointer - that way we don't have to try to undo implicit cast to void * and know the conversion in there is the user conversion. Unfortunately the C++ FE isn't prepared for the check_builtin_function_arguments to change the arguments (unlike C). I've bootstrapped/regtested on x86_64-linux and i686-linux the following fix, ok for trunk, or should I invest more time into the other approach? 2016-09-19 Jakub Jelinek <jakub@redhat.com> PR middle-end/77624 * builtins.c (fold_builtin_atomic_always_lock_free): Only look through cast to void * if the cast is from some other pointer type. * c-c++-common/pr77624-1.c: New test. * c-c++-common/pr77624-2.c: New test. Jakub