| Message ID | 201108240001.30398.mikael.morin@sfr.fr |
|---|---|
| State | New |
| Headers | show |
Hi Mikael: > this is an attempt to fix my recent breakage for PR50050. > I forgot that shape can't always be known, and thus, that for some > expressions, the shape field is a NULL pointer. > > This patch adds an early return in gfc_free_shape in the case shape is > NULL. Then some external NULL shape checks are redundant and can be > removed. I added some asserts in the cases there was no check before, so > that the code is strictly equivalent. That patch works for me - both for the test case and for the full library that I was attempting to compile. Thanks, Andrew.
On 08/24/2011 12:01 AM, Mikael Morin wrote: > this is an attempt to fix my recent breakage for PR50050. > I forgot that shape can't always be known, and thus, that for some > expressions, the shape field is a NULL pointer. > > Neither bootstraped, nor regression tested, but it is in progress. My machine > does its best (which is not a lot) to have this properly compiled and tested > (and then committed) as soon as possible. > Otherwise OK for 4.{4..7} ? OK for 4.6 and 4.7 (after regtesting). As 4.4 and 4.5 are not affected by this regression and only by the original bug (comment 0 of PR 50050), please wait a while before committing the combined patch to older branches. (One could also consider whether having 50050c0 fixed on 4.6/4.7 is sufficient.) Tobias PS: I think older GCC versions are mainly of interest for enterprise distributions. Others either have newer versions or won't update. Currently used GCC versions are: RHEL 6 uses GCC 4.4 (RHEL 5 has it as tech preview), Ubuntu long-term support (LTS) has also 4.4, while SLES has 4.5 as tech preview (and 4.3 as system compiler.) PPS: I am looking for someone to review my simple regression fix [4.3 to 4.7] at http://gcc.gnu.org/ml/fortran/2011-08/msg00186.html it *should* be safe - see last comment in the PR for a bit more context. Additionally, I have a -fcoarray=lib patch pending, which should be also rather simple though one could discuss about the ABI: http://gcc.gnu.org/ml/fortran/2011-08/msg00182.html
Hi Tobias, > PPS: I am looking for someone to review my simple regression fix [4.3 to > 4.7] at > http://gcc.gnu.org/ml/fortran/2011-08/msg00186.html > it *should* be safe - see last comment in the PR for a bit more context. That one is OK. Thanks! Thomas
Index: trans-expr.c =================================================================== --- trans-expr.c (révision 177956) +++ trans-expr.c (copie de travail) @@ -4411,6 +4411,7 @@ gfc_trans_subarray_assign (tree dest, gfc_componen gfc_add_block_to_block (&block, &loop.pre); gfc_add_block_to_block (&block, &loop.post); + gcc_assert (lss->shape != NULL); gfc_free_shape (&lss->shape, cm->as->rank); gfc_cleanup_loop (&loop); Index: expr.c =================================================================== --- expr.c (révision 177956) +++ expr.c (copie de travail) @@ -409,6 +409,9 @@ gfc_clear_shape (mpz_t *shape, int rank) void gfc_free_shape (mpz_t **shape, int rank) { + if (*shape == NULL) + return; + gfc_clear_shape (*shape, rank); free (*shape); *shape = NULL; @@ -490,8 +493,7 @@ free_expr0 (gfc_expr *e) } /* Free a shape array. */ - if (e->shape != NULL) - gfc_free_shape (&e->shape, e->rank); + gfc_free_shape (&e->shape, e->rank); gfc_free_ref_list (e->ref); Index: resolve.c =================================================================== --- resolve.c (révision 177956) +++ resolve.c (copie de travail) @@ -5198,8 +5198,7 @@ check_host_association (gfc_expr *e) && sym->attr.contained) { /* Clear the shape, since it might not be valid. */ - if (e->shape != NULL) - gfc_free_shape (&e->shape, e->rank); + gfc_free_shape (&e->shape, e->rank); /* Give the expression the right symtree! */ gfc_find_sym_tree (e->symtree->name, NULL, 1, &st); Index: trans-io.c =================================================================== --- trans-io.c (révision 177956) +++ trans-io.c (copie de travail) @@ -1999,6 +1999,7 @@ transfer_array_component (tree expr, gfc_component gfc_add_block_to_block (&block, &loop.pre); gfc_add_block_to_block (&block, &loop.post); + gcc_assert (ss->shape != NULL); gfc_free_shape (&ss->shape, cm->as->rank); gfc_cleanup_loop (&loop);
Hello, this is an attempt to fix my recent breakage for PR50050. I forgot that shape can't always be known, and thus, that for some expressions, the shape field is a NULL pointer. This patch adds an early return in gfc_free_shape in the case shape is NULL. Then some external NULL shape checks are redundant and can be removed. I added some asserts in the cases there was no check before, so that the code is strictly equivalent. Neither bootstraped, nor regression tested, but it is in progress. My machine does its best (which is not a lot) to have this properly compiled and tested (and then committed) as soon as possible. Otherwise OK for 4.{4..7} ? Mikael PS: Sorry for the breakage, and thanks to Andrew Benson for the early report (with a reduced testcase !). I was about to break the 4.5 branch as well before I saw it. 2011-08-22 Mikael Morin <mikael.morin@gcc.gnu.org> PR fortran/50050 * expr.c (gfc_free_shape): Do nothing if shape is NULL. (free_expr0): Remove redundant NULL shape check. * resolve.c (check_host_association): Ditto. * trans-expr.c (gfc_trans_subarray_assign): Assert that shape is non-NULL. * trans-io.c (transfer_array_component): Ditto. 2011-08-22 Mikael Morin <mikael.morin@gcc.gnu.org> * gfortran.dg/pointer_comp_init_1.f90: New test.