emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Suspicious warning in W64 build


From: Philipp Stephani
Subject: Re: Suspicious warning in W64 build
Date: Sun, 17 Sep 2017 14:52:16 +0000



Eli Zaretskii <address@hidden> schrieb am So., 17. Sep. 2017 um 16:31 Uhr:
> Cc: address@hidden, address@hidden
> From: Paul Eggert <address@hidden>
> Date: Sun, 17 Sep 2017 00:01:09 -0700
>
> Why was the attached patch needed? What warning did it suppress?

I wrote about that in

  http://lists.gnu.org/archive/html/emacs-devel/2017-09/msg00448.html

The warning is this:

  ../../emacs/src/data.c: In function 'minmax_driver':
  ../../emacs/src/data.c:3022:9: warning: 'accum.i' may be used uninitialized in this function [-Wmaybe-uninitialized]
  return accum;
  ^~~~~

Which seems to mean that even eassume is sometimes not enough to
convince GCC 7 that the code is correct:

  static Lisp_Object
  minmax_driver (ptrdiff_t nargs, Lisp_Object *args,
                 enum Arith_Comparison comparison)
  {
    eassume (0 < nargs);  <<<<<<<<<<<<<<<<<<<<<<<
    Lisp_Object accum;
    for (ptrdiff_t argnum = 0; argnum < nargs; argnum++)
      {
        Lisp_Object val = args[argnum];
        CHECK_NUMBER_OR_FLOAT_COERCE_MARKER (val);
        if (argnum == 0 || !NILP (arithcompare (val, accum, comparison)))
          accum = val;
        else if (FLOATP (accum) && isnan (XFLOAT_DATA (accum)))
          return accum;
      }
    return accum;
  }

Since nargs > 0, the loop is always entered, but GCC seems to miss that.

How about rewriting the function body like this:

{
  Lisp_Object accum = args[0];
  CHECK_NUMBER_OR_FLOAT_COERCE_MARKER (accum);
  for (ptrdiff_t argnum = 1; ...)
    {
      if (FLOATP (accum) && isnan (...)) break;
      ...
    }
  return accum;

reply via email to

[Prev in Thread] Current Thread [Next in Thread]