Re: [PATCH, PR94769] fortran/io.c: Fix use of uninitialized variable num

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH, PR94769] fortran/io.c: Fix use of uninitialized variable num

Tobias Burnus-3
Hi Stefan,

I prefer Jakub's suggestion – his change LGTM.

Cheers,

Tobias

On 4/28/20 2:30 PM, Jakub Jelinek via Gcc-patches wrote:

> On Tue, Apr 28, 2020 at 01:53:07PM +0200, Stefan Schulze Frielinghaus via Gcc-patches wrote:
>> gcc/fortran/ChangeLog:
>>
>> 2020-04-28  Stefan Schulze Frielinghaus  <[hidden email]>
>>
>>          PR fortran/94769
>>          * io.c (check_io_constraints): Initialize local variable num.
>> ---
>>   gcc/fortran/io.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gcc/fortran/io.c b/gcc/fortran/io.c
>> index e066666e01d..4526f729d1d 100644
>> --- a/gcc/fortran/io.c
>> +++ b/gcc/fortran/io.c
>> @@ -3840,7 +3840,7 @@ if (condition) \
>>
>>     if (dt->asynchronous)
>>       {
>> -      int num;
>> +      int num = 2;
>>         static const char * asynchronous[] = { "YES", "NO", NULL };
> Just nitpicking, wouldn't -1 be more usual value?
> And, I think there should be an assertion that it didn't remain -1 after the
> call, above
>        /* For "YES", mark related symbols as asynchronous.  */
> do
>        gcc_checking (num != -1);
> or so.
>
> Note, the reason why this triggers only on s390x is the vastly different
> inlining parameters the target uses, that causes a lot of headaches
> everywhere.
>
>       Jakub
>
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH, PR94769] fortran/io.c: Fix use of uninitialized variable num

gcc - fortran mailing list
On Wed, Apr 29, 2020 at 01:53:01PM +0200, Stefan Schulze Frielinghaus wrote:
> Hi Tobias,
>
> On Tue, Apr 28, 2020 at 05:06:15PM +0200, Tobias Burnus wrote:
> > Hi Stefan,
> >
> > I prefer Jakub's suggestion – his change LGTM.
>
> Please find attached an updated patch.  Bootstrapped and regtested on
> S/390.  Ok for master?

Ok, thanks.

> gcc/fortran/ChangeLog:
>
> 2020-04-28  Stefan Schulze Frielinghaus  <[hidden email]>
>
>         PR fortran/94769
>         * io.c (check_io_constraints): Initialize local variable num to
>         -1 and assert that it receives a meaningful value by function
>         compare_to_allowed_values.
> ---
>  gcc/fortran/io.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/fortran/io.c b/gcc/fortran/io.c
> index e066666e01d..981cf9e88dd 100644
> --- a/gcc/fortran/io.c
> +++ b/gcc/fortran/io.c
> @@ -3840,7 +3840,7 @@ if (condition) \
>  
>    if (dt->asynchronous)
>      {
> -      int num;
> +      int num = -1;
>        static const char * asynchronous[] = { "YES", "NO", NULL };
>  
>        /* Note: gfc_reduce_init_expr reports an error if not init-expr.  */
> @@ -3853,6 +3853,8 @@ if (condition) \
>   io_kind_name (k), warn, &dt->asynchronous->where, &num))
>   return false;
>  
> +      gcc_checking_assert (num != -1);
> +
>        /* For "YES", mark related symbols as asynchronous.  */
>        if (num == 0)
>   {
> --
> 2.25.3
>


        Jakub