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

[Paul Richard Thomas <paul dot richard dot thomas at gmail dot com>]

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



-- 
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.

Groucho Marx