David, Looks good to me. (The N*N effect you mentioned does not show itself until cwh_expr_temp() calls cwh_types_size_WN(), I think. And as you said, that is wrong.)
However, I am not convinced the first clause of the if branch has a "potentially similar problem". It seems to me the array expression code is correct. In summary, the patch as proposed by you looks good. Thanks, Michael -----Original Message----- From: David Coakley [mailto:dcoak...@gmail.com] Sent: Monday, June 27, 2011 5:34 PM To: open64-devel Subject: [Open64-devel] Code review request for bug 584: overallocation of stack space (Fortran) Could a gatekeeper please review the following bug fix? Note that there is potentially a similar problem in the first clause of the if-statement that the patch modifies (the "ARRAYEXP" case). I was unable to find any Fortran code that exercised it, so I left it alone. Fix bug 584: overallocation of temp stack space (Fortran). Consider the following test case: 1 program test 2 character (len=3233) :: lin 3 character (len=1024) :: path, nam 4 integer :: pos 5 pos=3 6 nam=path(1:(pos-1)) // lin 7 end program In order to store the result of the concatenation expression on line 7, the Fortran front-end generates code to allocate a temporary buffer on the stack. In this case the size of the buffer should be calculated as an addition of the two buffer sizes. The bug was that instead of allocating N bytes, N * N bytes were reserved. This error would either be undetected, if there was sufficent stack space, or fatal. The test case shows an example that exceeds the 8192 kbytes user stack limit common on modern Linux systems. The fix is to pass the size of the element as the second parameter to cwh_expr_temp in the Fortran front end, as documented in the function definition. We know it is the constant 1 because we have just created a character array type. Index: osprey/crayf90/sgi/cwh_stmt.cxx =================================================================== --- osprey/crayf90/sgi/cwh_stmt.cxx (revision 3663) +++ osprey/crayf90/sgi/cwh_stmt.cxx (working copy) @@ -1,5 +1,5 @@ /* - * Copyright (C) 2009 Advanced Micro Devices, Inc. All Rights Reserved. + * Copyright (C) 2009, 2011 Advanced Micro Devices, Inc. All Rights Reserved. */ /* @@ -3046,7 +3046,9 @@ wr = WN_COPY_Tree(wt); wt = F90_Wrap_ARREXP(wt); } else { - wt = cwh_expr_temp(ty,WN_COPY_Tree(rsz),f_T_PASSED); + /* character array case */ + WN *e_sz = WN_Intconst(MTYPE_I4,1); + wt = cwh_expr_temp(ty,WN_COPY_Tree(e_sz),f_T_PASSED); wr = WN_COPY_Tree(wt) ; } -David Coakley / AMD Open Source Compiler Engineering ------------------------------------------------------------------------------ All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2d-c2 _______________________________________________ Open64-devel mailing list Open64-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/open64-devel ------------------------------------------------------------------------------ All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2d-c2 _______________________________________________ Open64-devel mailing list Open64-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/open64-devel
