| Message ID | CAKwh3qipOj3hjqLpQdw7_HZx_QOZF9pyDU91ZyEpp_FhjTxJQg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
ping! 2015-01-19 15:41 GMT+01:00 Janus Weil <janus@gcc.gnu.org>: > Hi, > > this is a second patch dealing with finalization-related regressions, > after the one I submitted yesterday > (https://gcc.gnu.org/ml/fortran/2015-01/msg00109.html), which btw is > also still waiting for review ... > > This patch fixes an invalid memory reference inside the finalizer > routine (at runtime), which apparently was caused by dereferencing a > pointer without checking if it's NULL. I simply insert a call to > ASSOCIATED. > > I also rename two different runtime variables, which were both called > 'ptr', to 'ptr1' and 'ptr2', just to make it easier to distinguish > them in the dump. > > I also have the feeling the a lot of what is being done in > generate_finalization_wrapper and finalize_component (including my > changes) is a bit laborious. Some helper functions might be useful to > make all that code generation a bit more readable and less verbose. I > may attack this in a follow-up patch. > > This one regtests cleanly on x86_64-unknown-linux-gnu. Ok for trunk and 4.9? > > Cheers, > Janus > > > > 2015-01-19 Janus Weil <janus@gcc.gnu.org> > > PR fortran/64230 > * class.c (finalize_component): New argument 'sub_ns'. Insert code to > check if 'expr' is associated. > (generate_finalization_wrapper): Rename 'ptr' symbols to 'ptr1' and > 'ptr2'. Pass 'sub_ns' to finalize_component. > > 2015-01-19 Janus Weil <janus@gcc.gnu.org> > > PR fortran/64230 > * gfortran.dg/class_allocate_18.f90: Extended.
Janus Weil wrote: > this is a second patch dealing with finalization-related regressions, > [...] > This patch fixes an invalid memory reference inside the finalizer > routine (at runtime), which apparently was caused by dereferencing a > pointer without checking if it's NULL. I simply insert a call to > ASSOCIATED. > [...] > > This one regtests cleanly on x86_64-unknown-linux-gnu. Ok for trunk and 4.9? OK. Thanks for the patch! Tobias > 2015-01-19 Janus Weil <janus@gcc.gnu.org> > > PR fortran/64230 > * class.c (finalize_component): New argument 'sub_ns'. Insert code to > check if 'expr' is associated. > (generate_finalization_wrapper): Rename 'ptr' symbols to 'ptr1' and > 'ptr2'. Pass 'sub_ns' to finalize_component. > > 2015-01-19 Janus Weil <janus@gcc.gnu.org> > > PR fortran/64230 > * gfortran.dg/class_allocate_18.f90: Extended.
2015-01-24 18:18 GMT+01:00 Tobias Burnus <burnus@net-b.de>: >> this is a second patch dealing with finalization-related regressions, >> [...] >> This patch fixes an invalid memory reference inside the finalizer >> routine (at runtime), which apparently was caused by dereferencing a >> pointer without checking if it's NULL. I simply insert a call to >> ASSOCIATED. >> [...] >> >> This one regtests cleanly on x86_64-unknown-linux-gnu. Ok for trunk and >> 4.9? > > > OK. Thanks for the patch! Thanks for the review, Tobias. Committed to trunk as r220125. Will backport to 4.9 soon. Cheers, Janus
Hi Janus, The testcase has a dependence on libubsan.so, which my LD_LIBRARY_PATH does not seem to be able to resolve. It therefore fails in the regression test. When I point to ~/lib64, I get the message: /svn/trunk/gcc/testsuite/gfortran.dg/class_allocate_18.f90:8: runtime error: signed integer overflow: 214358001592 * 214358001560 cannot be represented in type 'integer(kind=8)' Cheers Paul On 26 January 2015 at 15:57, Janus Weil <janus@gcc.gnu.org> wrote: > 2015-01-24 18:18 GMT+01:00 Tobias Burnus <burnus@net-b.de>: >>> this is a second patch dealing with finalization-related regressions, >>> [...] >>> This patch fixes an invalid memory reference inside the finalizer >>> routine (at runtime), which apparently was caused by dereferencing a >>> pointer without checking if it's NULL. I simply insert a call to >>> ASSOCIATED. >>> [...] >>> >>> This one regtests cleanly on x86_64-unknown-linux-gnu. Ok for trunk and >>> 4.9? >> >> >> OK. Thanks for the patch! > > Thanks for the review, Tobias. Committed to trunk as r220125. Will > backport to 4.9 soon. > > Cheers, > Janus
Hi Paul, > The testcase has a dependence on libubsan.so, which my LD_LIBRARY_PATH > does not seem to be able to resolve. It therefore fails in the > regression test. When I point to ~/lib64, I get the message: > > /svn/trunk/gcc/testsuite/gfortran.dg/class_allocate_18.f90:8: runtime > error: signed integer overflow: 214358001592 * 214358001560 cannot be > represented in type 'integer(kind=8)' of course I tested it but did not see any problem on my machine (Ubuntu 14.10, x86_64-unknown-linux-gnu). In fact class_allocate_18.f90 seems to be the only testcase in gfortran.dg/ which uses any -fsanitize options, but there are plenty in other sections (mostly c-c++-common/ubsan/ and g++.dg/ubsan/). Could it be a problem with your local setup or a dejagnu issue that only applears on certain platforms? I have no idea how to fix this. Do you have any suggestion? Cheers, Janus > On 26 January 2015 at 15:57, Janus Weil <janus@gcc.gnu.org> wrote: >> 2015-01-24 18:18 GMT+01:00 Tobias Burnus <burnus@net-b.de>: >>>> this is a second patch dealing with finalization-related regressions, >>>> [...] >>>> This patch fixes an invalid memory reference inside the finalizer >>>> routine (at runtime), which apparently was caused by dereferencing a >>>> pointer without checking if it's NULL. I simply insert a call to >>>> ASSOCIATED. >>>> [...] >>>> >>>> This one regtests cleanly on x86_64-unknown-linux-gnu. Ok for trunk and >>>> 4.9? >>> >>> >>> OK. Thanks for the patch! >> >> Thanks for the review, Tobias. Committed to trunk as r220125. Will >> backport to 4.9 soon. >> >> Cheers, >> Janus > > > > -- > Outside of a dog, a book is a man's best friend. Inside of a dog it's > too dark to read. > > Groucho Marx
Dear Janus, I know somewhat less than nothing about such matters. I suggest that you contact the authors of the testcases that use libubsan that you mention. Sorry Paul On 26 January 2015 at 22:20, Janus Weil <janus@gcc.gnu.org> wrote: > Hi Paul, > >> The testcase has a dependence on libubsan.so, which my LD_LIBRARY_PATH >> does not seem to be able to resolve. It therefore fails in the >> regression test. When I point to ~/lib64, I get the message: >> >> /svn/trunk/gcc/testsuite/gfortran.dg/class_allocate_18.f90:8: runtime >> error: signed integer overflow: 214358001592 * 214358001560 cannot be >> represented in type 'integer(kind=8)' > > of course I tested it but did not see any problem on my machine > (Ubuntu 14.10, x86_64-unknown-linux-gnu). > > In fact class_allocate_18.f90 seems to be the only testcase in > gfortran.dg/ which uses any -fsanitize options, but there are plenty > in other sections (mostly c-c++-common/ubsan/ and g++.dg/ubsan/). > > Could it be a problem with your local setup or a dejagnu issue that > only applears on certain platforms? I have no idea how to fix this. Do > you have any suggestion? > > Cheers, > Janus > > > >> On 26 January 2015 at 15:57, Janus Weil <janus@gcc.gnu.org> wrote: >>> 2015-01-24 18:18 GMT+01:00 Tobias Burnus <burnus@net-b.de>: >>>>> this is a second patch dealing with finalization-related regressions, >>>>> [...] >>>>> This patch fixes an invalid memory reference inside the finalizer >>>>> routine (at runtime), which apparently was caused by dereferencing a >>>>> pointer without checking if it's NULL. I simply insert a call to >>>>> ASSOCIATED. >>>>> [...] >>>>> >>>>> This one regtests cleanly on x86_64-unknown-linux-gnu. Ok for trunk and >>>>> 4.9? >>>> >>>> >>>> OK. Thanks for the patch! >>> >>> Thanks for the review, Tobias. Committed to trunk as r220125. Will >>> backport to 4.9 soon. >>> >>> Cheers, >>> Janus >> >> >> >> -- >> Outside of a dog, a book is a man's best friend. Inside of a dog it's >> too dark to read. >> >> Groucho Marx
Janus Weil <janus@gcc.gnu.org> writes: > 2015-01-19 Janus Weil <janus@gcc.gnu.org> > > PR fortran/64230 > * gfortran.dg/class_allocate_18.f90: Extended. FAIL: gfortran.dg/class_allocate_18.f90 -O0 (test for excess errors) Excess errors: /usr/ia64-suse-linux/bin/ld: cannot find -lubsan Andreas.
On Tue, Jan 27, 2015 at 10:24:47AM +0100, Andreas Schwab wrote: > Janus Weil <janus@gcc.gnu.org> writes: > > > 2015-01-19 Janus Weil <janus@gcc.gnu.org> > > > > PR fortran/64230 > > * gfortran.dg/class_allocate_18.f90: Extended. > > FAIL: gfortran.dg/class_allocate_18.f90 -O0 (test for excess errors) > Excess errors: > /usr/ia64-suse-linux/bin/ld: cannot find -lubsan Yeah, if you want to add ubsan tests, you need to add gfortran.dg/ubsan/ directory and hack up ubsan.exp in there, from say gcc.dg/ubsan/ubsan.exp and gfortran.dg/dg.exp. Jakub
Index: gcc/fortran/class.c =================================================================== --- gcc/fortran/class.c (Revision 219840) +++ gcc/fortran/class.c (Arbeitskopie) @@ -881,7 +881,8 @@ comp_is_finalizable (gfc_component *comp) static void finalize_component (gfc_expr *expr, gfc_symbol *derived, gfc_component *comp, - gfc_symbol *stat, gfc_symbol *fini_coarray, gfc_code **code) + gfc_symbol *stat, gfc_symbol *fini_coarray, gfc_code **code, + gfc_namespace *sub_ns) { gfc_expr *e; gfc_ref *ref; @@ -950,15 +951,32 @@ finalize_component (gfc_expr *expr, gfc_symbol *de dealloc->ext.alloc.list->expr = e; dealloc->expr1 = gfc_lval_expr_from_sym (stat); + gfc_code *cond = gfc_get_code (EXEC_IF); + cond->block = gfc_get_code (EXEC_IF); + cond->block->expr1 = gfc_get_expr (); + cond->block->expr1->expr_type = EXPR_FUNCTION; + gfc_get_sym_tree ("associated", sub_ns, &cond->block->expr1->symtree, false); + cond->block->expr1->symtree->n.sym->attr.flavor = FL_PROCEDURE; + cond->block->expr1->symtree->n.sym->attr.intrinsic = 1; + cond->block->expr1->symtree->n.sym->result = cond->block->expr1->symtree->n.sym; + gfc_commit_symbol (cond->block->expr1->symtree->n.sym); + cond->block->expr1->ts.type = BT_LOGICAL; + cond->block->expr1->ts.kind = gfc_default_logical_kind; + cond->block->expr1->value.function.isym = gfc_intrinsic_function_by_id (GFC_ISYM_ASSOCIATED); + cond->block->expr1->value.function.actual = gfc_get_actual_arglist (); + cond->block->expr1->value.function.actual->expr = gfc_copy_expr (expr); + cond->block->expr1->value.function.actual->next = gfc_get_actual_arglist (); + cond->block->next = dealloc; + if (block) - block->next = dealloc; + block->next = cond; else if (*code) { - (*code)->next = dealloc; + (*code)->next = cond; (*code) = (*code)->next; } else - (*code) = dealloc; + (*code) = cond; } else if (comp->ts.type == BT_DERIVED && comp->ts.u.derived->f2k_derived @@ -994,7 +1012,8 @@ finalize_component (gfc_expr *expr, gfc_symbol *de gfc_component *c; for (c = comp->ts.u.derived->components; c; c = c->next) - finalize_component (e, comp->ts.u.derived, c, stat, fini_coarray, code); + finalize_component (e, comp->ts.u.derived, c, stat, fini_coarray, code, + sub_ns); gfc_free_expr (e); } } @@ -1927,7 +1946,7 @@ generate_finalization_wrapper (gfc_symbol *derived { gfc_finalizer *fini, *fini_elem = NULL; - gfc_get_symbol ("ptr", sub_ns, &ptr); + gfc_get_symbol ("ptr1", sub_ns, &ptr); ptr->ts.type = BT_DERIVED; ptr->ts.u.derived = derived; ptr->attr.flavor = FL_VARIABLE; @@ -2051,7 +2070,7 @@ generate_finalization_wrapper (gfc_symbol *derived if (!ptr) { - gfc_get_symbol ("ptr", sub_ns, &ptr); + gfc_get_symbol ("ptr2", sub_ns, &ptr); ptr->ts.type = BT_DERIVED; ptr->ts.u.derived = derived; ptr->attr.flavor = FL_VARIABLE; @@ -2100,7 +2119,7 @@ generate_finalization_wrapper (gfc_symbol *derived continue; finalize_component (gfc_lval_expr_from_sym (ptr), derived, comp, - stat, fini_coarray, &block); + stat, fini_coarray, &block, sub_ns); if (!last_code->block->next) last_code->block->next = block; }
Hi, this is a second patch dealing with finalization-related regressions, after the one I submitted yesterday (https://gcc.gnu.org/ml/fortran/2015-01/msg00109.html), which btw is also still waiting for review ... This patch fixes an invalid memory reference inside the finalizer routine (at runtime), which apparently was caused by dereferencing a pointer without checking if it's NULL. I simply insert a call to ASSOCIATED. I also rename two different runtime variables, which were both called 'ptr', to 'ptr1' and 'ptr2', just to make it easier to distinguish them in the dump. I also have the feeling the a lot of what is being done in generate_finalization_wrapper and finalize_component (including my changes) is a bit laborious. Some helper functions might be useful to make all that code generation a bit more readable and less verbose. I may attack this in a follow-up patch. This one regtests cleanly on x86_64-unknown-linux-gnu. Ok for trunk and 4.9? Cheers, Janus 2015-01-19 Janus Weil <janus@gcc.gnu.org> PR fortran/64230 * class.c (finalize_component): New argument 'sub_ns'. Insert code to check if 'expr' is associated. (generate_finalization_wrapper): Rename 'ptr' symbols to 'ptr1' and 'ptr2'. Pass 'sub_ns' to finalize_component. 2015-01-19 Janus Weil <janus@gcc.gnu.org> PR fortran/64230 * gfortran.dg/class_allocate_18.f90: Extended.