Hi all, the attached code crashes at runtime with gfortran 4.7 (svn rev. 181390) when compiled with optimization (-O1 and higher). It works when compiled with -O0, with gfortran 4.6 and earlier at any optimization level tested, and a series of other compilers. Can anybody shed some light on this problem? Depending on the active line in the main program, !#################################### ! Either expression leads to a crash: u = sum(xi*wi) + sum(xi*wi) ! u = sum(xi*wi + xi*wi) !#################################### I get a backtrace or a *** glibc detected *** ./a.out: double free or corruption (fasttop): 0x080743a0 * Activating the following print !####################################### !print *, "vector_times_vector: barrier" !####################################### in module mo_dec_matrix makes the crash disappear. Thanks, Harald
Created attachment 25856 [details] Source archive
Revision 171100 is OK revision 171653 segfault .
The miscompilation is triggered by -ffrontend-optimize, work-around: use -fno-frontend-optimize. Revision 171653 is dealing with the frontend optimization. If I am not mistaken, it is the only change dealing with frontend optimization after r171100, although I don't understand how this revision could cause a miscompilation. The crash occurs in __mo_dec_matrix_MOD_sum_vector.
(In reply to comment #3) > The miscompilation is triggered by -ffrontend-optimize, work-around: use > -fno-frontend-optimize. > Revision 171653 is dealing with the frontend optimization. If I am not > mistaken, it is the only change dealing with frontend optimization after > r171100, although I don't understand how this revision could cause a > miscompilation. > > The crash occurs in __mo_dec_matrix_MOD_sum_vector. Looks like a bug in the application, but the interesting coding style makes it hard for me to read. If one removes call delete_storage (x) in sum_vector the problem goes away. That is, it looks like over-active memory management hidden beneath a layer of obfusication.
On Sat, Nov 19, 2011 at 12:40:49AM +0000, kargl at gcc dot gnu.org wrote: > > The miscompilation is triggered by -ffrontend-optimize, work-around: use > > -fno-frontend-optimize. > > Revision 171653 is dealing with the frontend optimization. If I am not > > mistaken, it is the only change dealing with frontend optimization after > > r171100, although I don't understand how this revision could cause a > > miscompilation. > > > > The crash occurs in __mo_dec_matrix_MOD_sum_vector. > > Looks like a bug in the application, but the interesting > coding style makes it hard for me to read. If one removes > > call delete_storage (x) > > in sum_vector the problem goes away. That is, it looks like > over-active memory management hidden beneath a layer of > obfusication. > Having looked at the code some more, I believe it is noconforming, and this PR should be closed with INVALID,
(In reply to comment #3) > The miscompilation is triggered by -ffrontend-optimize, work-around: use > -fno-frontend-optimize. > Revision 171653 is dealing with the frontend optimization. If I am not > mistaken, it is the only change dealing with frontend optimization after > r171100, although I don't understand how this revision could cause a > miscompilation. > > The crash occurs in __mo_dec_matrix_MOD_sum_vector. Aha. Compiling just main.f90 with -fno-frontend-optimize solves the problem. Thanks, Harald
A few comments: a) The call to delete_storage makes the code invalid in several respects b) This function should not have been inlined (i.e. the bug not exposed) without the -faggressive-function-elimination flag. c) The reason why this function call was inlined was that the implicit_pure attribute is set on the function. This is bogus. /* Only eliminate potentially impure functions if the user specifically requested it. */ if (!gfc_option.flag_aggressive_function_elimination && !(*e)->value.function.esym->attr.pure && !(*e)->value.function.esym->attr.implicit_pure) return 0; (gdb) p (*e)->value.function.esym->attr.pure $8 = 0 (gdb) p (*e)->value.function.esym->attr.implicit_pure $9 = 1 Paul, I think you introduced the implicit_pure attribute. Do you have any idea why this could be set in this case?
(In reply to comment #6) > Aha. Compiling just main.f90 with -fno-frontend-optimize solves > the problem. Comparing -fdump-tree-original for main.f90 at -O0 without and with -fno-frontend-optimize shows: --- noopt/main.f90.003t.original 2011-11-19 10:51:21.000000000 +0100 +++ opt/main.f90.003t.original 2011-11-19 10:52:10.000000000 +0100 @@ -7,6 +7,7 @@ static struct t_vector xi = {.info=0B, .n=0, .n_s=0, .alloc_l=0, .s={.data=0B }}; { + struct t_vector __var_1; integer(kind=4) overflow.2; logical(kind=4) D.1818; character(kind=4) size.1; @@ -259,14 +260,9 @@ _gfortran_transfer_character_write (&dt_parm.11, &"Before u:"[1]{lb: 1 sz : 1}, 9); _gfortran_st_write_done (&dt_parm.11); } - { - struct t_vector D.1860; - struct t_vector D.1859; - - D.1859 = vector_times_vector (&xi, &wi); - D.1860 = vector_times_vector (&xi, &wi); - u = sum_vector (&D.1859) + sum_vector (&D.1860); - } + __var_1 = vector_times_vector (&xi, &wi); + u = sum_vector (&__var_1) + sum_vector (&__var_1); + L.4:; { struct __st_parameter_dt dt_parm.12; This won't work. The implementation of the management of temporaries does not allow that the same instance is used more than once. The other case with the commented lines exchanged: +++ opt2/main.f90.003t.original 2011-11-19 10:59:23.000000000 +0100 @@ -7,6 +7,7 @@ static struct t_vector xi = {.info=0B, .n=0, .n_s=0, .alloc_l=0, .s={.data=0B }}; { + struct t_vector __var_1; integer(kind=4) overflow.2; logical(kind=4) D.1818; character(kind=4) size.1; @@ -259,16 +260,14 @@ _gfortran_transfer_character_write (&dt_parm.11, &"Before u:"[1]{lb: 1 sz : 1}, 9); _gfortran_st_write_done (&dt_parm.11); } + __var_1 = vector_times_vector (&xi, &wi); { struct t_vector D.1862; - struct t_vector D.1860; - struct t_vector D.1859; - D.1859 = vector_times_vector (&xi, &wi); - D.1860 = vector_times_vector (&xi, &wi); - D.1862 = add_vectors (&D.1859, &D.1860); + D.1862 = add_vectors (&__var_1, &__var_1); u = sum_vector (&D.1862); } + L.4:; { struct __st_parameter_dt dt_parm.12; Almost the same here, with aliasing of the arguments leading to the slightly different crash.
(In reply to comment #7) > c) The reason why this function call was inlined was that the > implicit_pure attribute is set on the function. This is > bogus. Good point. Adding: volatile :: xi, wi in main solves the problem.
Am 19.11.2011 11:18, schrieb anlauf at gmx dot de: > This won't work. The implementation of the management > of temporaries does not allow that the same instance > is used more than once. If I understand your code, you are modifying the arguments of your function and evaluating that function more than once in a single expression. This is illegal in Fortran, so gfortran could, in principle, do anything with it. I would advise you to fix your code to be standard-conforming. Because such code is unfortunately quite common, gfortran by default does not do such optimizations unless directed to be. The fact that it does here nonetheless is, indeed, a bug.
(In reply to comment #10) The code does memory management similar to that required by TR15581 for allocatable DT components and allocatable function results, but it also has to work for compilers that do not support TR15581. It does so by overloading everything needed. > If I understand your code, you are modifying the arguments of your > function and evaluating that function more than once in a single > expression. The expression xi*wi allocates a temporary, which needs to get deallocated after it was used to avoid a memory leak. All bookkeeping is done with the temporaries. Functions check whether they access a variable or a temporary. Using the temporaries is not a pure operation, which is consistent with the way the functions are declared. > This is illegal in Fortran, so gfortran could, in > principle, do anything with it. I thought that program variables are somewhat different from temporaries created from expressions. In Fortran 95 I do not see any standard-conforming way for a program to access that very temporary more than once. If the compiler decides to use it twice, bad things happen. (Same if a compiler decided to call deallocate twice.) > I would advise you to fix your code to > be standard-conforming. Do you have a suggestion without introducing a memory leak? > Because such code is unfortunately quite common, gfortran by default > does not do such optimizations unless directed to be. The fact that > it does here nonetheless is, indeed, a bug.
On Sat, Nov 19, 2011 at 10:57:23AM +0000, tkoenig at netcologne dot de wrote: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51218 > > --- Comment #10 from tkoenig at netcologne dot de <tkoenig at netcologne dot de> 2011-11-19 10:57:23 UTC --- > Am 19.11.2011 11:18, schrieb anlauf at gmx dot de: > > This won't work. The implementation of the management > > of temporaries does not allow that the same instance > > is used more than once. > > If I understand your code, you are modifying the arguments of your > function and evaluating that function more than once in a single > expression. This is illegal in Fortran, so gfortran could, in > principle, do anything with it. I would advise you to fix your code to > be standard-conforming. > > Because such code is unfortunately quite common, gfortran by default > does not do such optimizations unless directed to be. The fact that > it does here nonetheless is, indeed, a bug. > The code is also invalid because it manipulates the pointer (not the target) of a derived-type dummy argument with the intent(in) attribute. The code has at least two bugs and the PR should be closed.
On Sat, Nov 19, 2011 at 11:46:28AM +0000, anlauf at gmx dot de wrote: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51218 > > --- Comment #11 from Harald Anlauf <anlauf at gmx dot de> 2011-11-19 11:46:28 UTC --- > (In reply to comment #10) > The code does memory management similar to that required by s/does/tries to do/ > TR15581 for allocatable DT components and allocatable function > results, but it also has to work for compilers that do not > support TR15581. It does so by overloading everything needed. > > > If I understand your code, you are modifying the arguments of your > > function and evaluating that function more than once in a single > > expression. > > The expression xi*wi allocates a temporary, which needs to > get deallocated after it was used to avoid a memory leak. > All bookkeeping is done with the temporaries. Functions > check whether they access a variable or a temporary. > Using the temporaries is not a pure operation, which is > consistent with the way the functions are declared. *If* the compiler generates a temporary for xi*wi, the compiler will/should generate the necessary code to garbage collect any memory used by that temporary. > > I would advise you to fix your code to > > be standard-conforming. > > Do you have a suggestion without introducing a memory leak? > Let the compiler do its job? Don't manipulate pointers in a non-conforming way?
Tobias, Why did you mark this PR with the "wrong-code" keyword? The code is invalid, so gfortran can generated anything it wants.
(In reply to comment #14) > Tobias, Why did you mark this PR with the "wrong-code" keyword? Because it generates "wrong-code" and I wasn't completely convinced that there is no bug lurking in implicit_pure. Thus, for me the status is an "unconfirmed" bug which would be - if confirmed - a wrong-code 4.7 regression. However, it looks as if the code is invalid and so far I didn't come up with something valid which gets wrongly treated. * * * (In reply to comment #11) > The code does memory management similar to that required by > TR15581 for allocatable DT components and allocatable function > results, but it also has to work for compilers that do not > support TR15581. It does so by overloading everything needed. I think that by now most compilers support TR 15581, thus, you could consider to solve the issue by changing the code to TR 15581. At the moment you have code which might work by chance with some compilers but fail with others since it is invalid. > > I would advise you to fix your code to > > be standard-conforming. > > Do you have a suggestion without introducing a memory leak? Unfortunately not. The mix of having to use INTENT(IN) because of OPERATOR(...) combined with the need to modify the argument looks extremely fragile and - as this issue shows - also breaks in practice. In principle, using TARGET, you could allow the variable to alias - but only if you do not use INTENT(IN) ... I think there is no simple way out, if (and only if) you need to be compatible to pre-TR15581 compilers. One other suggestion would be to compile the code with -O0. For gfortran, I do not see any possibility to "fix" is, which doesn't cause severe performance regressions. The problem is not just the implicit_pure attribute or the front-end optimization. The Fortran front end also passes the INTENT(IN) to the middle end, which allows also the middle end to certain optimizations. And, here, the user has explicitly entered the INTENT(IN) himself. Alias issues are one of the most important problems which hamper optimization. Thus, compilers try to make use of anything in the standard which rules out aliasing. If they can't, temporary arrays have to be introduced, code may not be optimized away, loop vectorization might be impossible etc. I think one of the strengths of gfortran is that it makes a lot of effort to avoid array temporaries and similar alias related issues. Thus, unless a practical suggestion how gfortran should be changed comes up - or a scenario is found where gfortran generates invalid code for a valid program, I am inclined to close the bug as INVALID, which would be in line of the comments of Thomas and Steve.
(In reply to comment #15) > Because it generates "wrong-code" and I wasn't completely convinced that there > is no bug lurking in implicit_pure. Thus, for me the status is an "unconfirmed" > bug which would be - if confirmed - a wrong-code 4.7 regression. I found a shorter code which shows that functions with side-effects are erroneously treated as pure, see below. > * * * > > (In reply to comment #11) > > The code does memory management similar to that required by > > TR15581 for allocatable DT components and allocatable function > > results, but it also has to work for compilers that do not > > support TR15581. It does so by overloading everything needed. > > I think that by now most compilers support TR 15581, thus, you could consider > to solve the issue by changing the code to TR 15581. Well, most /= all. At work I (have to) use a compiler which does not support TR 15581. The (hard-/software) vendor (non-European, non-US) has been successfully declining user demands to support F2003. > At the moment you have code which might work by chance with some compilers but > fail with others since it is invalid. Well, I understand that the code does bad thing. One thing it relies on is that the compiler recognizes that the bad function are not pure, as they have a side effect (e.g. accessing module variable call_level). If a side effect is able to disable critical optimizations, then I'm optimistic that the code will work on most platforms. Now as promised, here's the reduced example: module a implicit none integer :: neval = 0 ! integer, volatile :: neval = 0 ! Won't change anything... contains subroutine inc_eval neval = neval + 1 end subroutine inc_eval end module a module b use a implicit none contains function f(x) real :: f real, intent(in) :: x f = x end function f ! impure & ! This won't help... function g(x) real :: g real, intent(in) :: x ! g is not pure, it has a side-effect! call inc_eval ! print *, "In g!" g = x end function g end module b program gfcbug114a use a use b implicit none real :: x = 1, y = 1, t, u, v, w print *, (neval == 0) ! 0 initially t = f(x)*f(y) print *, (neval == 0) u = f(x)*f(y) + f(x)*f(y) print *, (neval == 0) v = g(x)*g(y) print *, (neval == 2) w = g(x)*g(y) + g(x)*g(y) print *, (neval == 6) print *, neval ! 6??? print *, t, u, v, w end program gfcbug114a Running without optimization, I get: T T T T T 6 1.00000000 2.00000000 1.00000000 2.00000000 With optimization, the result is: T T T T F 4 1.00000000 2.00000000 1.00000000 2.00000000 Thus the number of function evaluations of the non-pure function g deepends on the optimization. Enabling the commented out print in g(), I get the expected result. Setting g as impure, as well as making neval volatile does not help. I do think that this qualifies as a bug.
(In reply to comment #16) > One thing it relies on is that the compiler recognizes > that the bad function are not pure, as they have a > side effect (e.g. accessing module variable call_level). > If a side effect is able to disable critical optimizations, > then I'm optimistic that the code will work on most platforms. > > Now as promised, here's the reduced example: Thanks for the example! Untested patch: --- a/gcc/fortran/resolve.c +++ b/gcc/fortran/resolve.c @@ -3257,6 +3255,7 @@ pure_subroutine (gfc_code *c, gfc_symbol *sym) else if (gfc_pure (NULL)) gfc_error ("Subroutine call to '%s' at %L is not PURE", sym->name, &c->loc); + gfc_current_ns->proc_name->attr.implicit_pure = 0; }
On Mon, Nov 21, 2011 at 08:02:20PM +0000, burnus at gcc dot gnu.org wrote: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51218 > > --- Comment #17 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-11-21 20:02:20 UTC --- > (In reply to comment #16) > > One thing it relies on is that the compiler recognizes > > that the bad function are not pure, as they have a > > side effect (e.g. accessing module variable call_level). > > If a side effect is able to disable critical optimizations, > > then I'm optimistic that the code will work on most platforms. > > > > Now as promised, here's the reduced example: > > Thanks for the example! > > Untested patch: > > --- a/gcc/fortran/resolve.c > +++ b/gcc/fortran/resolve.c > @@ -3257,6 +3255,7 @@ pure_subroutine (gfc_code *c, gfc_symbol *sym) > else if (gfc_pure (NULL)) > gfc_error ("Subroutine call to '%s' at %L is not PURE", sym->name, > &c->loc); > + gfc_current_ns->proc_name->attr.implicit_pure = 0; > } Harald's example has function calls not subroutines. I would expect that you need to set this in pure_function as well. Also, does this type of change inhibit reason why implicit_pure was added in the first place?
(In reply to comment #18) > > @@ -3257,6 +3255,7 @@ pure_subroutine (gfc_code *c, gfc_symbol *sym) > > + gfc_current_ns->proc_name->attr.implicit_pure = 0; > > Harald's example has function calls not subroutines. I would > expect that you need to set this in pure_function as well. Hmm, I had the feeling that such a check is already there, but I have not really audited resolve.c > Also, does this type of change inhibit reason why > implicit_pure was added in the first place? One probably should add a check whether the called procedure has attr.implicit_pure has set - and only is not mark the current namespace as impure. Then, I don't think it will inhibit the reason for the initial change. And even if it does, having correct code is more important. We might be able to mark more code as implicit none - but first we need to make sure that code which is effectively not pure is also not marked as implicit_pure.
(In reply to comment #17) > Untested patch: > > --- a/gcc/fortran/resolve.c > +++ b/gcc/fortran/resolve.c > @@ -3257,6 +3255,7 @@ pure_subroutine (gfc_code *c, gfc_symbol *sym) > else if (gfc_pure (NULL)) > gfc_error ("Subroutine call to '%s' at %L is not PURE", sym->name, > &c->loc); > + gfc_current_ns->proc_name->attr.implicit_pure = 0; > } The patch works for the testcase in comment #16. It did not produce any regressions in my tests. (It also solves my original problem, which is a nice by-product.) Thanks, Harald
Author: burnus Date: Thu Nov 24 17:57:41 2011 New Revision: 181698 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=181698 Log: 2011-11-24 Tobias Burnus <burnus@net-b.de> PR fortran/51218 * resolve.c (pure_subroutine): If called subroutine is impure, unset implicit_pure. (resolve_function): Move impure check to simplify code. 2011-11-24 Tobias Burnus <burnus@net-b.de> PR fortran/51218 * gfortran.dg/implicit_pure_1.f90: New. Added: trunk/gcc/testsuite/gfortran.dg/implicit_pure_1.f90 Modified: trunk/gcc/fortran/ChangeLog trunk/gcc/fortran/resolve.c trunk/gcc/testsuite/ChangeLog
Author: burnus Date: Thu Nov 24 20:44:28 2011 New Revision: 181699 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=181699 Log: 2011-11-24 Tobias Burnus <burnus@net-b.de> PR fortran/51218 * resolve.c (pure_subroutine): If called subroutine is impure, unset implicit_pure. (resolve_function): Move impure check to simplify code. 2011-11-24 Tobias Burnus <burnus@net-b.de> PR fortran/51218 * gfortran.dg/implicit_pure_1.f90: New. Added: branches/gcc-4_6-branch/gcc/testsuite/gfortran.dg/implicit_pure_1.f90 Modified: branches/gcc-4_6-branch/gcc/fortran/ChangeLog branches/gcc-4_6-branch/gcc/fortran/resolve.c branches/gcc-4_6-branch/gcc/testsuite/ChangeLog
FIXED - hopefully completely. Thanks for the bugreport and the (valid) testcase.
Am 24.11.2011 21:51, schrieb burnus at gcc dot gnu.org: > Thanks for the bugreport and the (valid) > testcase. > To be pedantic, the test case was not valid; however, as a design choice, we chose to support it, because the error it had is very common.
(In reply to comment #24) > > Thanks for the bugreport and the (valid) > > testcase. > > To be pedantic, the test case was not valid Can you tell me what's wrong with the test case of comment 16? It looks perfectly valid to me. (One might argue about the result the program should print, i.e. whether a compiler may optimize "a = f() + f()" to "a= 2*f()" even if "f" is impure. I think the standard is a bit ambiguous about that one. But independently how one decides, I fail to see how it makes the program invalid. [And I stand firmly on the side that the compiler should not do this optimization (by default for any -O... level). On the other hand, I am also in favour of pure functions and prefer subroutines if the procedure is not pure.])
Am 25.11.2011 18:44, schrieb burnus at gcc dot gnu.org: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51218 > > --- Comment #25 from Tobias Burnus<burnus at gcc dot gnu.org> 2011-11-25 17:44:29 UTC --- > (In reply to comment #24) >>> Thanks for the bugreport and the (valid) >>> testcase. >> >> To be pedantic, the test case was not valid > > Can you tell me what's wrong with the test case of comment 16? It looks > perfectly valid to me. I'l defer to authority here :-) To quote Dick Hendrickson in the thread ttps://groups.google.com/group/comp.lang.fortran/browse_thread/thread/6dfe2c3088f89725/3f69c230c86db253?hl=de&ie=UTF-8&q=functions+*+side+effects+group:comp.lang.fortran+author:Thomas+author:Koenig#3f69c230c86db253 # In this case, I think the standard is clear. The processor is # allowed to evaluate f(3) once or twice. By the words # Richard quoted, a function is not allowed to affect or be # affected by anything else in the statement. So, one evaluation # of f(3) can't change the result of the other, and the processor # is free to evaluate f(3) + f(3) as 2*f(3). It # is processor dependent. Richard Maine sort of disagreed, he thinks the program is illegal. So, either way, optimizing away a function call would be OK.