[Bug fortran/94769] New: Possible use of uninitialized variable num

[Bug fortran/94769] New: Possible use of uninitialized variable num

[stefansf at linux dot ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94769

            Bug ID: 94769
           Summary: Possible use of uninitialized variable num
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: stefansf at linux dot ibm.com
  Target Milestone: ---

In function check_io_constraints local variable num is supposed to be
initialized by function compare_to_allowed_values. While bootstrapping GCC on
S/390 I get the following warning/error:

gcc/fortran/io.c: In function 'bool gfc_resolve_dt(gfc_code*, gfc_dt*,
locus*)':
gcc/fortran/io.c:3857:7: error: 'num' may be used uninitialized in this
function [-Werror=maybe-uninitialized]
 3857 |       if (num == 0)
      |       ^~       
gcc/fortran/io.c:3843:11: note: 'num' was declared here
 3843 |       int num;

I'm not sure whether a sensible default value for num should be 0 or 1.

[Bug fortran/94769] Possible use of uninitialized variable num

[kargl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94769

kargl at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kargl at gcc dot gnu.org

--- Comment #1 from kargl at gcc dot gnu.org ---
Looks like a false positive.  As you observe, num is set
in a previous line code.  You can set it to any value you
want.  A better option would be to have the option, which
causes the false positive, fixed or removed from the
command line.

[Bug fortran/94769] Possible use of uninitialized variable num

[schwab@linux-m68k.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94769

--- Comment #2 from Andreas Schwab <schwab@linux-m68k.org> ---
compare_to_allowed_values doesn't set *num most of the time even when returning
non-zero, especially if warn is true.

[Bug fortran/94769] Possible use of uninitialized variable num

[stefansf at linux dot ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94769

--- Comment #3 from Stefan Schulze Frielinghaus <stefansf at linux dot ibm.com> ---
Since one call chain is gfc_resolve_dt -> check_io_constraints ->
compare_to_allowed_values and at least one parameter of
compare_to_allowed_values, from which the initialization of variable num
depends, is passed through non-static function gfc_resolve_dt, the warning
seems valid to me. Although compare_to_allowed_values is supposed to initialize
num, it may return with a non-zero value although num is not initialized.

[Bug fortran/94769] Possible use of uninitialized variable num

[sgk at troutmask dot apl.washington.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94769

--- Comment #4 from Steve Kargl <sgk at troutmask dot apl.washington.edu> ---
On Mon, Apr 27, 2020 at 06:27:25AM +0000, stefansf at linux dot ibm.com wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94769
> 
> --- Comment #3 from Stefan Schulze Frielinghaus <stefansf at linux dot ibm.com> ---
> Since one call chain is gfc_resolve_dt -> check_io_constraints ->
> compare_to_allowed_values and at least one parameter of
> compare_to_allowed_values, from which the initialization of variable num
> depends, is passed through non-static function gfc_resolve_dt, the warning
> seems valid to me. Although compare_to_allowed_values is supposed to
> initialize num, it may return with a non-zero value although num is
> not initialized.
> 
compare_to_allowed_values() appears 25 times in io.c; once as
a prototype, once as the function, and 23 invocations.  This
is the function prototype:

compare_to_allowed_values (const char *specifier, const char *allowed[],
                           const char *allowed_f2003[],
                           const char *allowed_gnu[], gfc_char_t *value,
                           const char *statement, bool warn, locus *where,
                           int *num = NULL);

Of those 23 invocations, it is called once with a non-NULL &num argument.
That occurence is the one in question.  Here is the code:

      if (!compare_to_allowed_values
                 ("ASYNCHRONOUS", asynchronous, NULL, NULL,
                  dt->asynchronous->value.character.string,
                  io_kind_name (k), warn, &dt->asynchronous->where, &num))
         return false;

compare_to_allowed_values() returns either 0 or 1.  If it is 0,
then the if-statement will cause check_io_constraint() to return
false and num is never used.  If it returns 1, then one needs to
worry about num.

Prior to the invocation, it is noted that one has

 static const char *asynchronous[] = { "YES", "NO", NULL };

dt->asynchronous->value.character.string is the string parsed from
the Fortran program, e.g., OPEN(UNIT=10,FILE='foo', ASYNCHRONOUS='string')

The initial code in compare_to_allowed_values() is

  len = gfc_wide_strlen (value);
  if (len > 0)
    {
      for (len--; len > 0; len--)
        if (value[len] != ' ')
          break;
        len++;
    }

The above accounts for leading whitespace in value (aka
dt->asynchronous->value.character.string), and for a valid value of
ASYNCHRONOUS, len = 2 or 3.  The next chunk of code sets num for
a valid value.  Here allowed[] = asynchronous[].

  for (i = 0; allowed[i]; i++)
     if (len == strlen (allowed[i])
         && gfc_wide_strncasecmp (value, allowed[i], strlen (allowed[i])) == 0)
       {
         if (num)
           *num = i;
         return 1;
       }

If len does not equal 2 or 3, the block of code in the if statement is
not execute, num is not set, and the remainder of compare_to_allowed_values()
is executed, generating an error message, and returning 0.  A return of
0 means that num is not used in check_io_constraint as it returns false.

If len is equal to 2 or 3, then value is compared to "YES" and "NO".
If the comparison is true, then num (being a non-NULL pointer) has
it target set to either 0 or 1 (i.e., the  index of "YES" or "NO"
in allowed[]).  If the comparison is false, then the block of code in the
if statement is not execute, num is not set, and the remainder of
compare_to_allowed_values() is executed, generating an error message,
and returning 0.  A return of 0 means that num is not used in
check_io_constraint as it returns false.

In other words, a false positive.

There are now three choices.

1) Remove the command line option that causes the false positive.
2) Fix gcc to not generate a false positive.
3) Set num to anything you want to silence a false positive.

The above are the three options I offered in comment #1.

[Bug fortran/94769] Possible use of uninitialized variable num

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94769

Thomas Koenig <tkoenig at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tkoenig at gcc dot gnu.org

--- Comment #5 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
Steve,

is this sort of what you had in mind?

--- a/gcc/fortran/io.c
+++ b/gcc/fortran/io.c
@@ -3840,7 +3840,7 @@ if (condition) \

   if (dt->asynchronous)
     {
-      int num;
+      int num = 42; /* Fix stupid gcc warning.  */
       static const char * asynchronous[] = { "YES", "NO", NULL };

       /* Note: gfc_reduce_init_expr reports an error if not init-expr.  */
@@ -3853,6 +3853,7 @@ if (condition) \
                 io_kind_name (k), warn, &dt->asynchronous->where, &num))
        return false;

+      gcc_assert (num != 42);
       /* For "YES", mark related symbols as asynchronous.  */
       if (num == 0)
        {

[Bug fortran/94769] Possible use of uninitialized variable num

[sgk at troutmask dot apl.washington.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94769

--- Comment #6 from Steve Kargl <sgk at troutmask dot apl.washington.edu> ---
On Mon, Apr 27, 2020 at 05:12:50PM +0000, tkoenig at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94769
> 
> Thomas Koenig <tkoenig at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |tkoenig at gcc dot gnu.org
> 
> --- Comment #5 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
> Steve,
> 
> is this sort of what you had in mind?
> 

Yes, that should work.

Unfortunately, io.c has changed from svn r280157, so I
can only look at the current code through gitweb.  The
blame feature shows that Fritz added the 'if (num == 0)'
block of code on 2020-04-09 while Jerry introduced the
passing of &num in 2008.

[Bug fortran/94769] Possible use of uninitialized variable num

[stefansf at linux dot ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94769

--- Comment #7 from Stefan Schulze Frielinghaus <stefansf at linux dot ibm.com> ---
Since gfc_resolve_dt is a non-static function we cannot assume anything about
argument DT.  Argument DT gets passed to function check_io_constraints which
passes values depending on DT, namely dt->asynchronous->value.character.string
to function compare_to_allowed_values as well as argument warn which is true as
soon as DT->dterr is true.

If function compare_to_allowed_values is called with
dt->asynchronous->value.character.string not beeing an allowed value, and
ALLOWED_F2003 as well as ALLOWED_GNU beeing NULL (which is the case at the
mentioned call side), and WARN equals true, then

    if (warn)
      { 
        char *s = gfc_widechar_to_char (value, -1);
        gfc_warning (0,
                     "%s specifier in %s statement at %L has invalid value
%qs",
                     specifier, statement, where, s);
        free (s);
        return 1;
      } 

will fire and return a non-zero value which renders the compiler warning true.

[Bug fortran/94769] Possible use of uninitialized variable num

[stefansf at linux dot ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94769

--- Comment #8 from Stefan Schulze Frielinghaus <stefansf at linux dot ibm.com> ---
Patch posted:

https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544716.html

[Bug fortran/94769] Possible use of uninitialized variable num

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94769

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Stefan Schulze Frielinghaus
<stefansf@gcc.gnu.org>:

https://gcc.gnu.org/g:27594524d8a93cddb197ad8c9d4075c5870f1473

commit r10-8053-g27594524d8a93cddb197ad8c9d4075c5870f1473
Author: Stefan Schulze Frielinghaus <stefansf@linux.ibm.com>
Date:   Tue Apr 28 13:14:28 2020 +0200

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

    While bootstrapping GCC on S/390 the following warning occurs:

    gcc/fortran/io.c: In function 'bool gfc_resolve_dt(gfc_code*, gfc_dt*,
locus*)':
    gcc/fortran/io.c:3857:7: error: 'num' may be used uninitialized in this
function [-Werror=maybe-uninitialized]
     3857 |       if (num == 0)
          |       ^~
    gcc/fortran/io.c:3843:11: note: 'num' was declared here
     3843 |       int num;

    Since gfc_resolve_dt is a non-static function we cannot assume anything
about
    argument DT.  Argument DT gets passed to function check_io_constraints
which
    passes values depending on DT, namely
dt->asynchronous->value.character.string
    to function compare_to_allowed_values as well as argument warn which is
true as
    soon as DT->dterr is true.  Thus both arguments depend on DT.

    If function compare_to_allowed_values is called with
    dt->asynchronous->value.character.string not being an allowed value, and
    ALLOWED_F2003 as well as ALLOWED_GNU being NULL (which is the case at the
    particular call side), and WARN equals true, then the function returns with
a
    non-zero value and leaves num uninitialized which renders the warning true.

    Initialized num to -1 and added an assert statement.

    gcc/fortran/ChangeLog:

    2020-04-29  Stefan Schulze Frielinghaus  <stefansf@linux.ibm.com>

            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.

[Bug fortran/94769] Possible use of uninitialized variable num

[stefansf at linux dot ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94769

Stefan Schulze Frielinghaus <stefansf at linux dot ibm.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #10 from Stefan Schulze Frielinghaus <stefansf at linux dot ibm.com> ---
Closing since fixed by 27594524d8a