[PATCH] PR fortran/66942 -- avoid referencing a NULL C++ thing

[PATCH] PR fortran/66942 -- avoid referencing a NULL C++ thing

[Steve Kargl]

[-- Attachment #1: Type: text/plain, Size: 408 bytes --]

When C++ was injected into trans-expr.c in the form of vec,
it seems whomever did the conversion to vec forgot to check
for a NULL C++ thing.  This patch seems to avoid the problem,
but having zero knowledge of C++ I could be wrong.

OK for trunk?

2015-07-21  Steven G. Kargl  <kargl@gcc.gnu.org>

	PR fortran/66942
	* trans-expr.c (gfc_conv_procedure_call): Avoid dereferencing NULL
	C++ thing.

-- 
Steve

[-- Attachment #2: trans-expr.c.diff --]
[-- Type: text/x-diff, Size: 914 bytes --]

Index: trans-expr.c
===================================================================
--- trans-expr.c	(revision 226006)
+++ trans-expr.c	(working copy)
@@ -5921,13 +5921,16 @@ gfc_conv_procedure_call (gfc_se * se, gf
   vec_safe_reserve (retargs, arglen);
 
   /* Add the return arguments.  */
-  retargs->splice (arglist);
+  if (!vec_safe_is_empty (arglist))
+    retargs->splice (arglist);
 
   /* Add the hidden present status for optional+value to the arguments.  */
-  retargs->splice (optionalargs);
+  if (!vec_safe_is_empty (optionalargs))
+    retargs->splice (optionalargs);
 
   /* Add the hidden string length parameters to the arguments.  */
-  retargs->splice (stringargs);
+  if (!vec_safe_is_empty (stringargs))
+    retargs->splice (stringargs);
 
   /* We may want to append extra arguments here.  This is used e.g. for
      calls to libgfortran_matmul_??, which need extra information.  */

Re: [PATCH] PR fortran/66942 -- avoid referencing a NULL C++ thing

[Mikael Morin]

Le 21/07/2015 21:08, Steve Kargl a écrit :
> When C++ was injected into trans-expr.c in the form of vec,
> it seems whomever did the conversion to vec forgot to check
> for a NULL C++ thing.  This patch seems to avoid the problem,
> but having zero knowledge of C++ I could be wrong.
>
> OK for trunk?
>
> 2015-07-21  Steven G. Kargl  <kargl@gcc.gnu.org>
>
> 	PR fortran/66942
> 	* trans-expr.c (gfc_conv_procedure_call): Avoid dereferencing NULL
> 	C++ thing.
>
Hello Steve,

I believe the vec API should have all that is necessary to handle this 
automatically.
Did you try using vec_safe_splice?

Mikael

Re: [PATCH] PR fortran/66942 -- avoid referencing a NULL C++ thing

[Steve Kargl]

On Tue, Jul 28, 2015 at 03:04:52PM +0200, Mikael Morin wrote:
> Le 21/07/2015 21:08, Steve Kargl a ?crit :
> > When C++ was injected into trans-expr.c in the form of vec,
> > it seems whomever did the conversion to vec forgot to check
> > for a NULL C++ thing.  This patch seems to avoid the problem,
> > but having zero knowledge of C++ I could be wrong.
> >
> > OK for trunk?
> >
> > 2015-07-21  Steven G. Kargl  <kargl@gcc.gnu.org>
> >
> > 	PR fortran/66942
> > 	* trans-expr.c (gfc_conv_procedure_call): Avoid dereferencing NULL
> > 	C++ thing.
> >
> Hello Steve,
> 
> I believe the vec API should have all that is necessary to handle this 
> automatically.
> Did you try using vec_safe_splice?
> 

I know zero about vec and I know zero about C++.

-- 
Steve

Re: [PATCH] PR fortran/66942 -- avoid referencing a NULL C++ thing

[Richard Biener]

On Tue, Jul 28, 2015 at 3:19 PM, Steve Kargl
<sgk@troutmask.apl.washington.edu> wrote:
> On Tue, Jul 28, 2015 at 03:04:52PM +0200, Mikael Morin wrote:
>> Le 21/07/2015 21:08, Steve Kargl a ?crit :
>> > When C++ was injected into trans-expr.c in the form of vec,
>> > it seems whomever did the conversion to vec forgot to check
>> > for a NULL C++ thing.  This patch seems to avoid the problem,
>> > but having zero knowledge of C++ I could be wrong.
>> >
>> > OK for trunk?
>> >
>> > 2015-07-21  Steven G. Kargl  <kargl@gcc.gnu.org>
>> >
>> >     PR fortran/66942
>> >     * trans-expr.c (gfc_conv_procedure_call): Avoid dereferencing NULL
>> >     C++ thing.
>> >
>> Hello Steve,
>>
>> I believe the vec API should have all that is necessary to handle this
>> automatically.
>> Did you try using vec_safe_splice?

That handles NULL retargs, not NULL or empty arglist.

>
> I know zero about vec and I know zero about C++.

The patch looks ok to me.

Richard.

> --
> Steve

Re: [PATCH] PR fortran/66942 -- avoid referencing a NULL C++ thing

[Mikael Morin]

Le 29/07/2015 10:26, Richard Biener a écrit :
>>> Did you try using vec_safe_splice?
>
> That handles NULL retargs, not NULL or empty arglist.
>
I think retargs is NULL.

Re: [PATCH] PR fortran/66942 -- avoid referencing a NULL C++ thing

[Richard Biener]

On Wed, Jul 29, 2015 at 11:34 AM, Mikael Morin <mikael.morin@sfr.fr> wrote:
> Le 29/07/2015 10:26, Richard Biener a écrit :
>>>>
>>>> Did you try using vec_safe_splice?
>>
>>
>> That handles NULL retargs, not NULL or empty arglist.
>>
> I think retargs is NULL.

Not if the patch fixes anything.

Richard.

Re: [PATCH] PR fortran/66942 -- avoid referencing a NULL C++ thing

[Mikael Morin]

Le 29/07/2015 13:22, Richard Biener a écrit :
> On Wed, Jul 29, 2015 at 11:34 AM, Mikael Morin <mikael.morin@sfr.fr> wrote:
>> Le 29/07/2015 10:26, Richard Biener a écrit :
>>>>>
>>>>> Did you try using vec_safe_splice?
>>>
>>>
>>> That handles NULL retargs, not NULL or empty arglist.
>>>
>> I think retargs is NULL.
>
> Not if the patch fixes anything.
>
The case retargs == NULL is the case arglen == 0, which means every 
vector pointer we are about to splice is NULL.
So the patch fixes it.

Re: [PATCH] PR fortran/66942 -- avoid referencing a NULL C++ thing

[Richard Biener]

On Wed, Jul 29, 2015 at 1:59 PM, Mikael Morin <mikael.morin@sfr.fr> wrote:
> Le 29/07/2015 13:22, Richard Biener a écrit :
>>
>> On Wed, Jul 29, 2015 at 11:34 AM, Mikael Morin <mikael.morin@sfr.fr>
>> wrote:
>>>
>>> Le 29/07/2015 10:26, Richard Biener a écrit :
>>>>>>
>>>>>>
>>>>>> Did you try using vec_safe_splice?
>>>>
>>>>
>>>>
>>>> That handles NULL retargs, not NULL or empty arglist.
>>>>
>>> I think retargs is NULL.
>>
>>
>> Not if the patch fixes anything.
>>
> The case retargs == NULL is the case arglen == 0, which means every vector
> pointer we are about to splice is NULL.
> So the patch fixes it.

Ok, that wasn't obvious from reading the patch.

Richard.

Re: [PATCH] PR fortran/66942 -- avoid referencing a NULL C++ thing

[Steve Kargl]

On Wed, Jul 29, 2015 at 02:04:12PM +0200, Richard Biener wrote:
> On Wed, Jul 29, 2015 at 1:59 PM, Mikael Morin <mikael.morin@sfr.fr> wrote:
> > Le 29/07/2015 13:22, Richard Biener a écrit :
> >>
> >> On Wed, Jul 29, 2015 at 11:34 AM, Mikael Morin <mikael.morin@sfr.fr>
> >> wrote:
> >>>
> >>> Le 29/07/2015 10:26, Richard Biener a écrit :
> >>>>>>
> >>>>>>
> >>>>>> Did you try using vec_safe_splice?
> >>>>
> >>>>
> >>>>
> >>>> That handles NULL retargs, not NULL or empty arglist.
> >>>>
> >>> I think retargs is NULL.
> >>
> >>
> >> Not if the patch fixes anything.
> >>
> > The case retargs == NULL is the case arglen == 0, which means every vector
> > pointer we are about to splice is NULL.
> > So the patch fixes it.
> 
> Ok, that wasn't obvious from reading the patch.
> 

This builds and passes regression testing on x86_64-*-freebsd.
OP found the problem by running the sanatizers.  I don't
know how to build gcc with this as a build option.  I'll
commit whichever diff you recommend.
 
Index: trans-expr.c
===================================================================
--- trans-expr.c	(revision 226328)
+++ trans-expr.c	(working copy)
@@ -5921,18 +5921,18 @@ gfc_conv_procedure_call (gfc_se * se, gf
   vec_safe_reserve (retargs, arglen);
 
   /* Add the return arguments.  */
-  retargs->splice (arglist);
+  vec_safe_splice (retargs, arglist);
 
   /* Add the hidden present status for optional+value to the arguments.  */
-  retargs->splice (optionalargs);
+  vec_safe_splice (retargs, optionalargs);
 
   /* Add the hidden string length parameters to the arguments.  */
-  retargs->splice (stringargs);
+  vec_safe_splice (retargs, stringargs);
 
   /* We may want to append extra arguments here.  This is used e.g. for
      calls to libgfortran_matmul_??, which need extra information.  */
-  if (!vec_safe_is_empty (append_args))
-    retargs->splice (append_args);
+  vec_safe_splice (retargs, append_args);
+
   arglist = retargs;
 
   /* Generate the actual call.  */

-- 
Steve

Re: [PATCH] PR fortran/66942 -- avoid referencing a NULL C++ thing

[Mikael Morin]

Le 29/07/2015 18:45, Steve Kargl a écrit :
> On Wed, Jul 29, 2015 at 02:04:12PM +0200, Richard Biener wrote:
>> On Wed, Jul 29, 2015 at 1:59 PM, Mikael Morin <mikael.morin@sfr.fr> wrote:
>>> Le 29/07/2015 13:22, Richard Biener a écrit :
>>>>
>>>> On Wed, Jul 29, 2015 at 11:34 AM, Mikael Morin <mikael.morin@sfr.fr>
>>>> wrote:
>>>>>
>>>>> Le 29/07/2015 10:26, Richard Biener a écrit :
>>>>>>>>
>>>>>>>>
>>>>>>>> Did you try using vec_safe_splice?
>>>>>>
>>>>>>
>>>>>>
>>>>>> That handles NULL retargs, not NULL or empty arglist.
>>>>>>
>>>>> I think retargs is NULL.
>>>>
>>>>
>>>> Not if the patch fixes anything.
>>>>
>>> The case retargs == NULL is the case arglen == 0, which means every vector
>>> pointer we are about to splice is NULL.
>>> So the patch fixes it.
>>
>> Ok, that wasn't obvious from reading the patch.
>>
>
> This builds and passes regression testing on x86_64-*-freebsd.
> OP found the problem by running the sanatizers.  I don't
> know how to build gcc with this as a build option.  I'll
> commit whichever diff you recommend.
>
I prefer this second variant.
OK for trunk without further comment from Richi.
Thanks.

Mikael

Re: [PATCH] PR fortran/66942 -- avoid referencing a NULL C++ thing

[Steve Kargl]

On Thu, Jul 30, 2015 at 04:08:47PM +0200, Mikael Morin wrote:
> Le 29/07/2015 18:45, Steve Kargl a écrit :
> >
> > This builds and passes regression testing on x86_64-*-freebsd.
> > OP found the problem by running the sanatizers.  I don't
> > know how to build gcc with this as a build option.  I'll
> > commit whichever diff you recommend.
> >
> I prefer this second variant.
> OK for trunk without further comment from Richi.
> Thanks.
> 

Thanks Mikael.  Committed revision 226517.

-- 
Steve