This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Fortran, Patch] PR60334 - Segmentation fault on character pointer assignments


Hi,

please don't commit this patch yet. I am seeing non-explainable ICEs and wrong
code now. Just have to investigate.

- Andre

On Tue, 13 Jan 2015 12:42:13 +0100 Andre Vehreschild <vehre@gmx.de> wrote:

> Hi Paul,
> 
> thanks for the reviewed and the valued comments. 
> 
> Just for completeness I have attached the patch with the changes requested.
> 
> Bootstraps and regtests ok on x86_64-linux-gnu.
> 
> Regards,
> 	Andre
> 
> 
> On Mon, 12 Jan 2015 22:07:29 +0100
> Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:
> 
> > Hi Andre,
> > 
> > +      if (INDIRECT_REF_P (parmse.string_length))
> > +        /* In chains of functions/procedure calls the string_length already
> > +           is a pointer to the variable holding the length.  Therefore
> > +           remove the deref on call.  */
> > +        parmse.string_length = TREE_OPERAND (parmse.string_length, 0);
> > 
> > This is OK but I would use instead:
> > +      if (POINTER_TYPE_P (parmse.string_length))
> > +        /* In chains of functions/procedure calls the string_length already
> > +           is a pointer to the variable holding the length.  Therefore
> > +           remove the deref on call.  */
> > +        parmse.string_length = build_fold_indirect_ref
> > (parmse.string_length);
> > 
> > If you look in ~/gcc/fold-const.c:15751, you will see that
> > TREE_OPERAND (parmse.string_length, 0) but that it is preceded by
> > cleaning up of NOOPS and, in any case, its usage will preserve the
> > standard API.... just in case the internals change :-)
> > 
> > of course, using TREE_OPERAND (xxx, 0) in the various fortran class
> > functions makes such an assumption ;-)
> > 
> > Apart from that, the patch is fine.
> > 
> > I'll have a session of doing some commits later this week and will do
> > this patch at that time.
> > 
> > Cheers
> > 
> > Paul
> > 
> > On 11 January 2015 at 16:21, Andre Vehreschild <vehre@gmx.de> wrote:
> > > Hi Paul,
> > >
> > > thanks for the review. I do not have commits rights.
> > >
> > > Unfortunately is the patch not ok. I figured today, that it needs an
> > > extension when function calls that return deferred char len arrays are
> > > nested. In this special case the string length would have been lost. The
> > > attached extended version fixes this issue.
> > >
> > > Sorry for the duplicate work.
> > >
> > > Bootstraps and regtests ok on x86_64-linux-gnu.
> > >
> > > Regards,
> > >         Andre
> > >
> > > On Sun, 11 Jan 2015 16:11:10 +0100
> > > Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:
> > >
> > >> Dear Andre,
> > >>
> > >> This is OK for trunk. I have not been keeping track of whether or not
> > >> you have commit rights yet. If not, I will get to it sometime this
> > >> week.
> > >>
> > >> Thanks for the patch.
> > >>
> > >> Paul
> > >>
> > >> On 10 January 2015 at 15:59, Andre Vehreschild <vehre@gmx.de> wrote:
> > >> > Hi all,
> > >> >
> > >> > attached patch fixes the bug reported in pr 60334. The issue here was
> > >> > that the function's result being (a pointer to) a deferred length char
> > >> > array. The string length for the result value was wrapped in a local
> > >> > variable, whose value was never written back to the string length of
> > >> > the result. This lead the calling routine to take the length of the
> > >> > result to be random leading to a crash.
> > >> >
> > >> > This patch addresses the issue by preventing the instantiation of the
> > >> > local var and instead using a reference to the parameter. This not only
> > >> > saves one value on the stack, but also because for small functions the
> > >> > compiler will hold all parameters in registers for a significant level
> > >> > of optimization, all the overhead of memory access (I hope :-).
> > >> >
> > >> > Bootstraps and regtests ok on x86_64-linux-gnu.
> > >> >
> > >> > - Andre
> > >> > --
> > >> > Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen
> > >> > Tel.: +49 241 9291018 * Email: vehre@gmx.de
> > >>
> > >>
> > >>
> > >
> > >
> > > --
> > > Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen
> > > Tel.: +49 241 9291018 * Email: vehre@gmx.de
> > 
> > 
> > 
> 
> 


-- 
Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen
Tel.: +49 241 9291018 * Email: vehre@gmx.de 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]