| Message ID | CAE4aFAkArCKp5jyuToj+NFw=-PwZ481LUe6dp7NG_MjAqKCrPA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Oct 05, 2016 at 04:40:50PM -0400, Fritz Reese wrote: > When union type symbols are compared and they contain maps containing > characters with different lengths, their type symbols should have > different backend declarations otherwise the gimple tree explodes. > Strangely enough the gimple checker only explodes with '-g' enabled > and certain other specific conditions, however the problem seems > clear. See the attached testcase for an example, and the attached > patch for a fix. > > RFC: My only concern is that this patch would technically also change > the way components are compared between derived types and class types, > not just union/map types. However from what I can tell if two derived > types are declared with character components of different lengths then > the two types should have distinct backend declarations anyway. If > anyone can think of any issues this patch might cause with derived > types/class types then I'd be okay guarding the new if statement to > only run for union/structure types. But with all my tests it doesn't > seem to result in any concrete differences. > > The patch does pass all regression tests on x86_64-redhat-linux. I > will give it a couple days for the RFC before committing. > > --- > Fritz Reese > > 2016-10-05 Fritz Reese <fritzoreese@gmail.com> > > Fix ICE due to map typespecs with different sized charlens being copied. > > * gcc/fortran/interface.c (compare_components): Check charlens. > > * gcc/testsuite/gfortran.dg/dec_union_11.f90: New testcase. > Patch looks ok to me.
diff --git a/gcc/fortran/interface.c b/gcc/fortran/interface.c index e7f1878..17f544e 100644 --- a/gcc/fortran/interface.c +++ b/gcc/fortran/interface.c @@ -495,6 +495,17 @@ compare_components (gfc_component *cmp1, gfc_component *cmp2, if (cmp1->attr.dimension && gfc_compare_array_spec (cmp1->as, cmp2->as) == 0) return 0; + if (cmp1->ts.type == BT_CHARACTER && cmp2->ts.type == BT_CHARACTER) + { + gfc_charlen *l1 = cmp1->ts.u.cl; + gfc_charlen *l2 = cmp2->ts.u.cl; + if (l1 && l2 && l1->length && l2->length + && l1->length->expr_type == EXPR_CONSTANT + && l2->length->expr_type == EXPR_CONSTANT + && gfc_dep_compare_expr (l1->length, l2->length) != 0) + return 0; + } + /* Make sure that link lists do not put this function into an endless recursive loop! */ if (!(cmp1->ts.type == BT_DERIVED && derived1 == cmp1->ts.u.derived) diff --git a/gcc/testsuite/gfortran.dg/dec_union_11.f90 b/gcc/testsuite/gfortran.dg/dec_union_11.f90 new file mode 100644 index 0000000..3ff4b49 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/dec_union_11.f90 @@ -0,0 +1,63 @@ +! { dg-do compile } +! { dg-options "-g -fdec-structure" } +! +! Test a regression where typespecs of unions containing character buffers of +! different lengths where copied, resulting in a bad gimple tree state. +! + +subroutine sub2 (otherbuf) + integer, parameter :: L_bbuf = 65536 + integer, parameter :: L_bbuf2 = 24 + + structure /buffer2/ + union + map + character(L_bbuf2) sbuf + end map + end union + end structure + structure /buffer/ + union + map + character(L_bbuf) sbuf + end map + end union + end structure + + record /buffer/ buf1 + record /buffer2/ buf2 + common /c/ buf1, buf2 + + record /buffer2/ otherbuf +end subroutine + +subroutine sub() + integer, parameter :: L_bbuf = 65536 + integer, parameter :: L_bbuf2 = 24 + + structure /buffer2/ + union + map + character(L_bbuf2) sbuf + end map + end union + end structure + structure /buffer/ + union + map + character(L_bbuf) sbuf + end map + end union + end structure + + record /buffer/ buf1 + record /buffer2/ buf2 + common /c/ buf1, buf2 + + call sub2 (buf1) ! { dg-warning "Type mismatch" } + return +end subroutine + +call sub() + +end
When union type symbols are compared and they contain maps containing characters with different lengths, their type symbols should have different backend declarations otherwise the gimple tree explodes. Strangely enough the gimple checker only explodes with '-g' enabled and certain other specific conditions, however the problem seems clear. See the attached testcase for an example, and the attached patch for a fix. RFC: My only concern is that this patch would technically also change the way components are compared between derived types and class types, not just union/map types. However from what I can tell if two derived types are declared with character components of different lengths then the two types should have distinct backend declarations anyway. If anyone can think of any issues this patch might cause with derived types/class types then I'd be okay guarding the new if statement to only run for union/structure types. But with all my tests it doesn't seem to result in any concrete differences. The patch does pass all regression tests on x86_64-redhat-linux. I will give it a couple days for the RFC before committing. --- Fritz Reese 2016-10-05 Fritz Reese <fritzoreese@gmail.com> Fix ICE due to map typespecs with different sized charlens being copied. * gcc/fortran/interface.c (compare_components): Check charlens. * gcc/testsuite/gfortran.dg/dec_union_11.f90: New testcase.