Suspicious warning in W64 build

Suspicious warning in W64 build

[Angelo Graziosi]

In the log build of current master on MSYS2/MINGW64, I see:

[...]
CC       w32term.o
C:/msys64/tmp/src/src/w32font.c: In function 'w32font_text_extents':
C:/msys64/tmp/src/src/w32font.c:547:9: warning: argument 1 range [18446744065119617024, 18446744073709551612] exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=]
   wcode = alloca (nglyphs * sizeof (WORD) * 2);
         ^
C:/msys64/tmp/src/src/w32font.c:547:9: note: in a call to built-in allocation function '__builtin_alloca'
  CC       w32xfns.o
[...]

I wonder if we can be comfortable with it...

BTW, I see also a lot a lot of messages (surely harmless) like this:

C:/msys64/tmp/src/lisp/calendar/cal-loaddefs.el and c:/msys64/tmp/src/lisp/calendar/cal-loaddefs.el are the same file

Angelo

Re: Suspicious warning in W64 build

[Richard Copley]

On 5 September 2017 at 14:38, Angelo Graziosi <angelo.g0@libero.it> wrote:
> I wonder if we can be comfortable with it...

No comment from me ...

> BTW, I see also a lot a lot of messages (surely harmless) like this:
>
> C:/msys64/tmp/src/lisp/calendar/cal-loaddefs.el and c:/msys64/tmp/src/lisp/calendar/cal-loaddefs.el are the same file

Yes, they are harmless. The call to 'find-file-noselect' in
'autoload-find-generated-file' in "autoloads.el" should pass second
argument 't', but it doesn't. Please report a bug! ;)

The next biggest source of noise from the Windows build is warnings
about printf format-strings, but that's due to GCC/MSVC compatibility
issues and I don't know a solution for it.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Date: Tue, 5 Sep 2017 15:38:15 +0200 (CEST)
> From: Angelo Graziosi <angelo.g0@libero.it>
> 
> In the log build of current master on MSYS2/MINGW64, I see:
> 
> [...]
> CC       w32term.o
> C:/msys64/tmp/src/src/w32font.c: In function 'w32font_text_extents':
> C:/msys64/tmp/src/src/w32font.c:547:9: warning: argument 1 range [18446744065119617024, 18446744073709551612] exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=]
>    wcode = alloca (nglyphs * sizeof (WORD) * 2);
>          ^
> C:/msys64/tmp/src/src/w32font.c:547:9: note: in a call to built-in allocation function '__builtin_alloca'

What version of GCC produces this warning?  I don't see it with GCC
6.3.0, or maybe it's because my builds are 32-bit, not 64-bit.

In any case, I have difficulty understanding this part:

  argument 1 range [18446744065119617024, 18446744073709551612]

Where did it take these 2 numbers?

Re: Suspicious warning in W64 build

[Angelo Graziosi]

> Il 7 settembre 2017 alle 17.16 Eli Zaretskii <eliz@gnu.org> ha scritto:
> 
> 
> > Date: Tue, 5 Sep 2017 15:38:15 +0200 (CEST)
> > From: Angelo Graziosi <angelo.g0@libero.it>
> > 
> > In the log build of current master on MSYS2/MINGW64, I see:
> > 
> > [...]
> > CC       w32term.o
> > C:/msys64/tmp/src/src/w32font.c: In function 'w32font_text_extents':
> > C:/msys64/tmp/src/src/w32font.c:547:9: warning: argument 1 range [18446744065119617024, 18446744073709551612] exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=]
> >    wcode = alloca (nglyphs * sizeof (WORD) * 2);
> >          ^
> > C:/msys64/tmp/src/src/w32font.c:547:9: note: in a call to built-in allocation function '__builtin_alloca'
> 
> What version of GCC produces this warning?  I don't see it with GCC

MSYS2/MINGW64 uses 7.2.0:

$ gcc -v
Using built-in specs.
COLLECT_GCC=C:\msys64\mingw64\bin\gcc.exe
COLLECT_LTO_WRAPPER=C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/7.2.0/lto-wrapper.exe
Target: x86_64-w64-mingw32
[...]
Thread model: posix
gcc version 7.2.0 (Rev1, Built by MSYS2 project)


> 6.3.0, or maybe it's because my builds are 32-bit, not 64-bit.

here 64-bit

> 
> In any case, I have difficulty understanding this part:
> 
>   argument 1 range [18446744065119617024, 18446744073709551612]
> 
> Where did it take these 2 numbers?

Bohh.. :-)

Re: Suspicious warning in W64 build

[Richard Copley]

On 7 September 2017 at 16:42, Angelo Graziosi <angelo.g0@libero.it> wrote:
>> Il 7 settembre 2017 alle 17.16 Eli Zaretskii <eliz@gnu.org> ha scritto:
>> > Date: Tue, 5 Sep 2017 15:38:15 +0200 (CEST)
>> > From: Angelo Graziosi <angelo.g0@libero.it>
>> >
>> > In the log build of current master on MSYS2/MINGW64, I see:
>> >
>> > [...]
>> > CC       w32term.o
>> > C:/msys64/tmp/src/src/w32font.c: In function 'w32font_text_extents':
>> > C:/msys64/tmp/src/src/w32font.c:547:9: warning: argument 1 range [18446744065119617024, 18446744073709551612] exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=]
>> >    wcode = alloca (nglyphs * sizeof (WORD) * 2);
>> >          ^
>> > C:/msys64/tmp/src/src/w32font.c:547:9: note: in a call to built-in allocation function '__builtin_alloca'
>>

>> Where did it take these 2 numbers?

I also see the warning (I use the same toolchain).

18446744065119617024 is 1<<64.
The other number is (1<<64) - (1<<33) - (1<<2).

That's just crazy. It must be a GCC bug.

Re: Suspicious warning in W64 build

[Richard Copley]

On 7 September 2017 at 18:52, Richard Copley <rcopley@gmail.com> wrote:
> On 7 September 2017 at 16:42, Angelo Graziosi <angelo.g0@libero.it> wrote:
>>> Il 7 settembre 2017 alle 17.16 Eli Zaretskii <eliz@gnu.org> ha scritto:
>>> > Date: Tue, 5 Sep 2017 15:38:15 +0200 (CEST)
>>> > From: Angelo Graziosi <angelo.g0@libero.it>
>>> >
>>> > In the log build of current master on MSYS2/MINGW64, I see:
>>> >
>>> > [...]
>>> > CC       w32term.o
>>> > C:/msys64/tmp/src/src/w32font.c: In function 'w32font_text_extents':
>>> > C:/msys64/tmp/src/src/w32font.c:547:9: warning: argument 1 range [18446744065119617024, 18446744073709551612] exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=]
>>> >    wcode = alloca (nglyphs * sizeof (WORD) * 2);
>>> >          ^
>>> > C:/msys64/tmp/src/src/w32font.c:547:9: note: in a call to built-in allocation function '__builtin_alloca'
>>>
>
>>> Where did it take these 2 numbers?
>
> I also see the warning (I use the same toolchain).
>
> 18446744065119617024 is 1<<64.
> The other number is (1<<64) - (1<<33) - (1<<2).
>
> That's just crazy. It must be a GCC bug.

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

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Thu, 7 Sep 2017 18:52:35 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>, Emacs Development <emacs-devel@gnu.org>
> 
> 18446744065119617024 is 1<<64.
> The other number is (1<<64) - (1<<33) - (1<<2).

Yes, I know.  I just don't understand why GCC things that

   nglyphs * sizeof (WORD) * 2

can have a value in this range.  The type of nglyphs is 'int', and
sizeof(WORD) gives 2.  So the range should be between 0 (or 4) and 4
times INT_MAX, which is nowhere near the values GCC displays.

Re: Suspicious warning in W64 build

[Angelo Graziosi]

Richard.

> Il 7 settembre 2017 alle 19.58 Richard Copley <rcopley@gmail.com> ha scritto:
> 
> 
> On 7 September 2017 at 18:52, Richard Copley <rcopley@gmail.com> wrote:
> > On 7 September 2017 at 16:42, Angelo Graziosi <angelo.g0@libero.it> wrote:
> >>> Il 7 settembre 2017 alle 17.16 Eli Zaretskii <eliz@gnu.org> ha scritto:
> >>> > Date: Tue, 5 Sep 2017 15:38:15 +0200 (CEST)
> >>> > From: Angelo Graziosi <angelo.g0@libero.it>
> >>> >
> >>> > In the log build of current master on MSYS2/MINGW64, I see:
> >>> >
> >>> > [...]
> >>> > CC       w32term.o
> >>> > C:/msys64/tmp/src/src/w32font.c: In function 'w32font_text_extents':
> >>> > C:/msys64/tmp/src/src/w32font.c:547:9: warning: argument 1 range [18446744065119617024, 18446744073709551612] exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=]
> >>> >    wcode = alloca (nglyphs * sizeof (WORD) * 2);
> >>> >          ^
> >>> > C:/msys64/tmp/src/src/w32font.c:547:9: note: in a call to built-in allocation function '__builtin_alloca'
> >>>
> >
> >>> Where did it take these 2 numbers?
> >
> > I also see the warning (I use the same toolchain).
> >
> > 18446744065119617024 is 1<<64.
> > The other number is (1<<64) - (1<<33) - (1<<2).
> >
> > That's just crazy. It must be a GCC bug.
> 
> "https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79929".

the link seems to have an "invalid" id...

Re: Suspicious warning in W64 build

[Richard Copley]

On 7 September 2017 at 20:00, Angelo Graziosi <angelo.g0@libero.it> wrote:
> Richard.
>
>> Il 7 settembre 2017 alle 19.58 Richard Copley <rcopley@gmail.com> ha scritto:
>>
>>
>> On 7 September 2017 at 18:52, Richard Copley <rcopley@gmail.com> wrote:
>> > On 7 September 2017 at 16:42, Angelo Graziosi <angelo.g0@libero.it> wrote:
>> >>> Il 7 settembre 2017 alle 17.16 Eli Zaretskii <eliz@gnu.org> ha scritto:
>> >>> > Date: Tue, 5 Sep 2017 15:38:15 +0200 (CEST)
>> >>> > From: Angelo Graziosi <angelo.g0@libero.it>
>> >>> >
>> >>> > In the log build of current master on MSYS2/MINGW64, I see:
>> >>> >
>> >>> > [...]
>> >>> > CC       w32term.o
>> >>> > C:/msys64/tmp/src/src/w32font.c: In function 'w32font_text_extents':
>> >>> > C:/msys64/tmp/src/src/w32font.c:547:9: warning: argument 1 range [18446744065119617024, 18446744073709551612] exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=]
>> >>> >    wcode = alloca (nglyphs * sizeof (WORD) * 2);
>> >>> >          ^
>> >>> > C:/msys64/tmp/src/src/w32font.c:547:9: note: in a call to built-in allocation function '__builtin_alloca'
>> >>>
>> >
>> >>> Where did it take these 2 numbers?
>> >
>> > I also see the warning (I use the same toolchain).
>> >
>> > 18446744065119617024 is 1<<64.
>> > The other number is (1<<64) - (1<<33) - (1<<2).
>> >
>> > That's just crazy. It must be a GCC bug.
>>
>> "https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79929".
>
> the link seems to have an "invalid" id...

No. My guess is that some software interpreted the link to include the
quotation mark and/or the full stop as part of the URL.

I put the URL in quotation marks to avoid the full stop being
interpreted as part of the URL. I suppose I'll start surrounding
URLs with newlines instead. Yuck!

Re: Suspicious warning in W64 build

[Paul Eggert]

Eli Zaretskii wrote:
>> 18446744065119617024 is 1<<64.
>> The other number is (1<<64) - (1<<33) - (1<<2).
> Yes, I know.  I just don't understand why GCC things that
> 
>     nglyphs * sizeof (WORD) * 2
> 
> can have a value in this range.  The type of nglyphs is 'int', and
> sizeof(WORD) gives 2.  So the range should be between 0 (or 4) and 4
> times INT_MAX, which is nowhere near the values GCC displays.
> 

If nglyphs is negative the first multiplication is done using unsigned 
arithmetic, which could result in huge results. Presumably GCC does not know 
that nglyphs must be nonnegative. Try putting an 'eassume (0 <= nglyphs);' 
before the line in question. That is, if you're sure that nglyphs is 
nonnegative: if you're not, GCC has found a real bug here.

Re: Suspicious warning in W64 build

[Richard Copley]

On 7 September 2017 at 20:26, Paul Eggert <eggert@cs.ucla.edu> wrote:
> Eli Zaretskii wrote:
>>>
>>> 18446744065119617024 is 1<<64.
>>> The other number is (1<<64) - (1<<33) - (1<<2).
>>
>> Yes, I know.  I just don't understand why GCC things that
>>
>>     nglyphs * sizeof (WORD) * 2
>>
>> can have a value in this range.  The type of nglyphs is 'int', and
>> sizeof(WORD) gives 2.  So the range should be between 0 (or 4) and 4
>> times INT_MAX, which is nowhere near the values GCC displays.
>>
>
> If nglyphs is negative the first multiplication is done using unsigned
> arithmetic, which could result in huge results. Presumably GCC does not know
> that nglyphs must be nonnegative. Try putting an 'eassume (0 <= nglyphs);'
> before the line in question. That is, if you're sure that nglyphs is
> nonnegative: if you're not, GCC has found a real bug here.

That makes sense if GCC has deduced that there is a call which always
passes a negative number (though it would be even better if the
warning identified where such call(s) are).

But if it's possible to pass a small positive number for nglyphs at
all the call sites, then the range in the warning is certainly wrong.

Hmm, actually now I notice that A>B in the quoted range [A, B]. That's
not normal mathematical notation.

Is it a Computer Science thing for types that wrap? In that case I'm
not sure why a warning is merited.

Is it a Computer Science thing for types that wrap? In that case I'm
not sure why a warning is merited. Could it be a special GCC notation?
I rather suspect it's some mistake. But I'm probably missing
something, if Paul and the people on the GCC bug thread don't see an
issue.

Re: Suspicious warning in W64 build

[Richard Copley]

 On 7 September 2017 at 20:26, Paul Eggert <eggert@cs.ucla.edu> wrote:
> Try putting an 'eassume (0 <= nglyphs);' before the line in question.

The warning goes away.

By the way, there's another warning which is both true and easy to fix.
"(1 << ((n) % 32)))" should read "(1u << ((n) % 32)))" on line 2191.

  CC       w32font.o
w32font.c: In function 'font_supported_scripts':
w32font.c:2191:32: warning: result of '1 << 31' requires 33 bits to
represent, but 'int' only has 32 bits [-Wshift-overflow=]
   if (subranges[(n) / 32] & (1 << ((n) % 32))) \
                                ^
w32font.c:2246:3: note: in expansion of macro 'SUBRANGE'
   SUBRANGE (31, Qsymbol);
   ^~~~~~~~
w32font.c:2191:32: warning: result of '1 << 31' requires 33 bits to
represent, but 'int' only has 32 bits [-Wshift-overflow=]
   if (subranges[(n) / 32] & (1 << ((n) % 32))) \
                                ^
w32font.c:2308:3: note: in expansion of macro 'SUBRANGE'
   SUBRANGE (95, Qtai_le);
   ^~~~~~~~

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Cc: angelo.g0@libero.it, emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Thu, 7 Sep 2017 12:26:06 -0700
> 
> > Yes, I know.  I just don't understand why GCC things that
> > 
> >     nglyphs * sizeof (WORD) * 2
> > 
> > can have a value in this range.  The type of nglyphs is 'int', and
> > sizeof(WORD) gives 2.  So the range should be between 0 (or 4) and 4
> > times INT_MAX, which is nowhere near the values GCC displays.
> > 
> 
> If nglyphs is negative the first multiplication is done using unsigned 
> arithmetic, which could result in huge results. Presumably GCC does not know 
> that nglyphs must be nonnegative. Try putting an 'eassume (0 <= nglyphs);' 
> before the line in question. That is, if you're sure that nglyphs is 
> nonnegative: if you're not, GCC has found a real bug here.

nglyphs is always positive, it comes from nchars of a glyph_string, or
similar.

Angelo and Richard, can you please try that eassume and see if it
shuts up the warning?

Re: Suspicious warning in W64 build

[Angelo Graziosi]

> Il 7 settembre 2017 alle 22.20 Eli Zaretskii <eliz@gnu.org> ha scritto:
> 
> 
> > Cc: angelo.g0@libero.it, emacs-devel@gnu.org
> > From: Paul Eggert <eggert@cs.ucla.edu>
> > Date: Thu, 7 Sep 2017 12:26:06 -0700
> > 
> > > Yes, I know.  I just don't understand why GCC things that
> > > 
> > >     nglyphs * sizeof (WORD) * 2
> > > 
> > > can have a value in this range.  The type of nglyphs is 'int', and
> > > sizeof(WORD) gives 2.  So the range should be between 0 (or 4) and 4
> > > times INT_MAX, which is nowhere near the values GCC displays.
> > > 
> > 
> > If nglyphs is negative the first multiplication is done using unsigned 
> > arithmetic, which could result in huge results. Presumably GCC does not know 
> > that nglyphs must be nonnegative. Try putting an 'eassume (0 <= nglyphs);' 
> > before the line in question. That is, if you're sure that nglyphs is 
> > nonnegative: if you're not, GCC has found a real bug here.
> 
> nglyphs is always positive, it comes from nchars of a glyph_string, or
> similar.
> 
> Angelo and Richard, can you please try that eassume and see if it
> shuts up the warning?

I have applied this patch:

--- w32font.c~  2017-09-07 21:49:47.000000000 +0200
+++ w32font.c   2017-09-07 23:14:22.997647400 +0200
@@ -544,6 +544,7 @@
      information.  */

   /* Make array big enough to hold surrogates.  */
+  eassume (0 <= nglyphs);
   wcode = alloca (nglyphs * sizeof (WORD) * 2);
   for (i = 0; i < nglyphs; i++)
     {

..and didn't find warning about w32font.c!

Ciao,
Angelo.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Thu, 7 Sep 2017 21:02:05 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>, Angelo Graziosi <angelo.g0@libero.it>, 
> 	Emacs Development <emacs-devel@gnu.org>
> 
>  On 7 September 2017 at 20:26, Paul Eggert <eggert@cs.ucla.edu> wrote:
> > Try putting an 'eassume (0 <= nglyphs);' before the line in question.
> 
> The warning goes away.

OK, I will commit that change, then.

> By the way, there's another warning which is both true and easy to fix.
> "(1 << ((n) % 32)))" should read "(1u << ((n) % 32)))" on line 2191.
> 
>   CC       w32font.o
> w32font.c: In function 'font_supported_scripts':
> w32font.c:2191:32: warning: result of '1 << 31' requires 33 bits to
> represent, but 'int' only has 32 bits [-Wshift-overflow=]
>    if (subranges[(n) / 32] & (1 << ((n) % 32))) \
>                                 ^
> w32font.c:2246:3: note: in expansion of macro 'SUBRANGE'
>    SUBRANGE (31, Qsymbol);
>    ^~~~~~~~
> w32font.c:2191:32: warning: result of '1 << 31' requires 33 bits to
> represent, but 'int' only has 32 bits [-Wshift-overflow=]
>    if (subranges[(n) / 32] & (1 << ((n) % 32))) \
>                                 ^
> w32font.c:2308:3: note: in expansion of macro 'SUBRANGE'
>    SUBRANGE (95, Qtai_le);
>    ^~~~~~~~

Yes, I've seen this and have a fix for it.  Thanks.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Date: Thu, 7 Sep 2017 23:59:32 +0200 (CEST)
> From: Angelo Graziosi <angelo.g0@libero.it>
> Cc: emacs-devel@gnu.org, rcopley@gmail.com
> 
> > nglyphs is always positive, it comes from nchars of a glyph_string, or
> > similar.
> > 
> > Angelo and Richard, can you please try that eassume and see if it
> > shuts up the warning?
> 
> I have applied this patch:
> 
> --- w32font.c~  2017-09-07 21:49:47.000000000 +0200
> +++ w32font.c   2017-09-07 23:14:22.997647400 +0200
> @@ -544,6 +544,7 @@
>       information.  */
> 
>    /* Make array big enough to hold surrogates.  */
> +  eassume (0 <= nglyphs);
>    wcode = alloca (nglyphs * sizeof (WORD) * 2);
>    for (i = 0; i < nglyphs; i++)
>      {
> 
> ..and didn't find warning about w32font.c!

Thanks.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Date: Fri, 08 Sep 2017 09:49:44 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: eggert@cs.ucla.edu, angelo.g0@libero.it, emacs-devel@gnu.org
> 
> > From: Richard Copley <rcopley@gmail.com>
> > Date: Thu, 7 Sep 2017 21:02:05 +0100
> > Cc: Eli Zaretskii <eliz@gnu.org>, Angelo Graziosi <angelo.g0@libero.it>, 
> > 	Emacs Development <emacs-devel@gnu.org>
> > 
> >  On 7 September 2017 at 20:26, Paul Eggert <eggert@cs.ucla.edu> wrote:
> > > Try putting an 'eassume (0 <= nglyphs);' before the line in question.
> > 
> > The warning goes away.
> 
> OK, I will commit that change, then.

Done.

> > By the way, there's another warning which is both true and easy to fix.
> > "(1 << ((n) % 32)))" should read "(1u << ((n) % 32)))" on line 2191.
> > 
> >   CC       w32font.o
> > w32font.c: In function 'font_supported_scripts':
> > w32font.c:2191:32: warning: result of '1 << 31' requires 33 bits to
> > represent, but 'int' only has 32 bits [-Wshift-overflow=]
> >    if (subranges[(n) / 32] & (1 << ((n) % 32))) \
> >                                 ^
> > w32font.c:2246:3: note: in expansion of macro 'SUBRANGE'
> >    SUBRANGE (31, Qsymbol);
> >    ^~~~~~~~
> > w32font.c:2191:32: warning: result of '1 << 31' requires 33 bits to
> > represent, but 'int' only has 32 bits [-Wshift-overflow=]
> >    if (subranges[(n) / 32] & (1 << ((n) % 32))) \
> >                                 ^
> > w32font.c:2308:3: note: in expansion of macro 'SUBRANGE'
> >    SUBRANGE (95, Qtai_le);
> >    ^~~~~~~~
> 
> Yes, I've seen this and have a fix for it.

Pushed to master.

Re: Suspicious warning in W64 build

[Richard Copley]

On 8 September 2017 at 09:02, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Fri, 08 Sep 2017 09:49:44 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>>
>> OK, I will commit that change, then.
>
> Done.
>
>>
>> Yes, I've seen this and have a fix for it.
>
> Pushed to master.

Thanks for taking the time. Would you mind also suppressing warnings
from find-file-noselect when called from autoload-find-generated-file,
please, if you don't object? I don't think it would be a cludge. It
seems logical to me.

BTW, to correct a couple of my mistakes and perhaps save others
confusion,

On 7 September 2017 at 18:52, Richard Copley <rcopley@gmail.com> wrote:
>
> 18446744065119617024 is 1<<64.
> The other number is (1<<64) - (1<<33) - (1<<2).
> That's just crazy. It must be a GCC bug.

On 7 September 2017 at 20:50, Richard Copley <rcopley@gmail.com> wrote:
>
> Hmm, actually now I notice that A>B in the quoted range [A, B]. That's
> not normal mathematical notation.

Actually,

A = 18 446 744 065 119 617 024 = FFFF FFFE 0000 0000
B = 18 446 744 073 709 551 612 = FFFF FFFF FFFF FFFC

A = 1111111111111111 1111111111111110 0000000000000000 0000000000000000
B = 1111111111111111 1111111111111111 1111111111111111 1111111111111100

A =  = 2⁶⁴ - 2³³ = (1 << 64) - (1 << 33)
B =  = 2⁶⁴ - 2²  = (1 << 64) - (1 << 2)

and indeed A is less than B.

As Paul said, to evaluate the "nglyphs * sizeof (WORD) * 2", since
"sizeof(WORD)" is of type size_t,

* convert nglyphs from int to size_t (by repeatedly adding or
subtracting one more than the maximum value that can be represented in
the new type until the value is in the range of the new type).

* multiply it by four.

Assuming 64-bit unsigned size_t and 32-bit signed int,
if nglyphs is -2³¹ (INT_MIN) the value of the expression is A
and if nglyphs is -1 then the value is B.

"Crazy" was too strong (not to mention insensitive) a word to use,
but I still can't see that GCC is justified to warn when an
expression's value _might_ be out of range based on operand types;
taken to extremes that would rule out "alloca((size_t)n)". So either
I'm still missing something, or there's a GCC bug, or the compiler
correctly deduced that a negative number is passed in. I'm probably
still missing something.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Fri, 8 Sep 2017 20:31:08 +0100
> Cc: Paul Eggert <eggert@cs.ucla.edu>, Angelo Graziosi <angelo.g0@libero.it>, 
> 	Emacs Development <emacs-devel@gnu.org>
> 
> Would you mind also suppressing warnings from find-file-noselect
> when called from autoload-find-generated-file, please, if you don't
> object?

Sorry, I'm not sure that's the right solution in that case.  We should
try to understand why the problem happens in the first place.  E.g., I
don't see it here (but I don't use MinGW64 or MSYS2).

> "Crazy" was too strong (not to mention insensitive) a word to use,
> but I still can't see that GCC is justified to warn when an
> expression's value _might_ be out of range based on operand types;
> taken to extremes that would rule out "alloca((size_t)n)". So either
> I'm still missing something, or there's a GCC bug, or the compiler
> correctly deduced that a negative number is passed in. I'm probably
> still missing something.

The only thing you might be missing is the fact that we passed the
compiler the -Walloc-size-larger-than=9223372036854775807 option.

Re: Suspicious warning in W64 build

[Richard Copley]

On 8 September 2017 at 21:17, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Richard Copley <rcopley@gmail.com>
>> Date: Fri, 8 Sep 2017 20:31:08 +0100
>> Cc: Paul Eggert <eggert@cs.ucla.edu>, Angelo Graziosi <angelo.g0@libero.it>,
>>       Emacs Development <emacs-devel@gnu.org>
>>
>> Would you mind also suppressing warnings from find-file-noselect
>> when called from autoload-find-generated-file, please, if you don't
>> object?
>
> Sorry, I'm not sure that's the right solution in that case.  We should
> try to understand why the problem happens in the first place.  E.g., I
> don't see it here (but I don't use MinGW64 or MSYS2).

Seriously (not being rude)? It seems a lot of work for not much
benefit. That conversion happens often, e.g., in normalize_filename.
And it's unlikely a warning from find_file_noselect there is ever
going to be helpful in catching a new bug.

>> "Crazy" was too strong (not to mention insensitive) a word to use,
>> but I still can't see that GCC is justified to warn when an
>> expression's value _might_ be out of range based on operand types;
>> taken to extremes that would rule out "alloca((size_t)n)". So either
>> I'm still missing something, or there's a GCC bug, or the compiler
>> correctly deduced that a negative number is passed in. I'm probably
>> still missing something.
>
> The only thing you might be missing is the fact that we passed the
> compiler the -Walloc-size-larger-than=9223372036854775807 option.

No, I don't think so. That's just the flag that enables that warning,
isn't it? I've read and re-read its documentation and I still don't
see why the warning is correct. The warning is intended for the case
when the argument is always out of range, if I understand correctly.

Re: Suspicious warning in W64 build

[Richard Copley]

On 8 September 2017 at 22:08, Richard Copley <rcopley@gmail.com> wrote:
> On 8 September 2017 at 21:17, Eli Zaretskii <eliz@gnu.org> wrote:
>>> From: Richard Copley <rcopley@gmail.com>
>>> Date: Fri, 8 Sep 2017 20:31:08 +0100
>>> Cc: Paul Eggert <eggert@cs.ucla.edu>, Angelo Graziosi <angelo.g0@libero.it>,
>>>       Emacs Development <emacs-devel@gnu.org>
>>>
>>> Would you mind also suppressing warnings from find-file-noselect
>>> when called from autoload-find-generated-file, please, if you don't
>>> object?
>>
>> Sorry, I'm not sure that's the right solution in that case.  We should
>> try to understand why the problem happens in the first place.  E.g., I
>> don't see it here (but I don't use MinGW64 or MSYS2).
>
> Seriously (not being rude)? It seems a lot of work for not much
> benefit. That conversion happens often, e.g., in normalize_filename.
> And it's unlikely a warning from find_file_noselect there is ever
> going to be helpful in catching a new bug.
>

As a short-cut/side-step, could we normalize one or both filenames
explicitly, somewhere nearer to the call site (perhaps for w32 only)?
But I don't know if there's a lisp function for that.

Re: Suspicious warning in W64 build

[Richard Copley]

On 8 September 2017 at 22:08, Richard Copley <rcopley@gmail.com> wrote:
> On 8 September 2017 at 21:17, Eli Zaretskii <eliz@gnu.org> wrote:
>> The only thing you might be missing is the fact that we passed the
>> compiler the -Walloc-size-larger-than=9223372036854775807 option.
>
> No, I don't think so. That's just the flag that enables that warning,
> isn't it? I've read and re-read its documentation and I still don't
> see why the warning is correct. The warning is intended for the case
> when the argument is always out of range, if I understand correctly.

No you're right, thanks. I think I get it. It's not so complicated. We
asked the compiler for advice, it took a good guess, and if we give it
more information it can do even better.

Re: Suspicious warning in W64 build

[Herring, Davis]

> I put the URL in quotation marks to avoid the full stop being
> interpreted as part of the URL. I suppose I'll start surrounding
> URLs with newlines instead. Yuck!

The usual convention is angles: <http://www.example.org/>.

Davis

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Fri, 8 Sep 2017 22:08:04 +0100
> Cc: Paul Eggert <eggert@cs.ucla.edu>, Angelo Graziosi <angelo.g0@libero.it>, 
> 	Emacs Development <emacs-devel@gnu.org>
> 
> >> Would you mind also suppressing warnings from find-file-noselect
> >> when called from autoload-find-generated-file, please, if you don't
> >> object?
> >
> > Sorry, I'm not sure that's the right solution in that case.  We should
> > try to understand why the problem happens in the first place.  E.g., I
> > don't see it here (but I don't use MinGW64 or MSYS2).
> 
> Seriously (not being rude)?

Yes, seriously.  You'd expect that much of me, won't you?

> It seems a lot of work for not much benefit.

Not IME.  I frequently find that the real root cause of such obscure
problems is something altogether unexpected, and sometimes it's the
tip of an iceberg.  So I generally find looking into the root causes
of such problems beneficial.

> That conversion happens often, e.g., in normalize_filename.

What conversion is that?

> And it's unlikely a warning from find_file_noselect there is ever
> going to be helpful in catching a new bug.

You may be right.  But we will only know for sure when we have
understood the root cause.  It could very well be that there is indeed
a bug, and this warning has already caught it, except that it went
unnoticed until now, because it was never reported before.

For starters, can you or Angelo tell where did that upper-case "C:/"
originate from?  Also, are there other places in the build process
that display file names with the upper-case drive letter?

Thanks.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Fri, 8 Sep 2017 22:37:28 +0100
> Cc: Paul Eggert <eggert@cs.ucla.edu>, Angelo Graziosi <angelo.g0@libero.it>, 
> 	Emacs Development <emacs-devel@gnu.org>
> 
> As a short-cut/side-step, could we normalize one or both filenames
> explicitly, somewhere nearer to the call site (perhaps for w32 only)?

Once again, this could be the right solution, but we can only decide
that when we understand the problem a bit better.

> But I don't know if there's a lisp function for that.

Almost all of the functions that work with file names do that, AFAIK.
Which is why I don't yet see how this file name with an upper-case
drive letter came into existence.  It must have originated somewhere,
most probably in user-space.  Let's find that out before we decide on
the right course of action.

Re: Suspicious warning in W64 build

[Angelo Graziosi]

> Il 8 settembre 2017 alle 22.17 Eli Zaretskii <eliz@gnu.org> ha scritto:
>  E.g., I
> don't see it here (but I don't use MinGW64 or MSYS2).

Hmm... Have you tried to do an out of tree build? Maybe I am wrong but in a tree build it only uses relative path: ../../path etc.

Re: Suspicious warning in W64 build

[Richard Copley]

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

On 9 September 2017 at 08:33, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Richard Copley <rcopley@gmail.com>
>> Date: Fri, 8 Sep 2017 22:08:04 +0100
>> Cc: Paul Eggert <eggert@cs.ucla.edu>, Angelo Graziosi <angelo.g0@libero.it>,
>>       Emacs Development <emacs-devel@gnu.org>
>>
>> >> Would you mind also suppressing warnings from find-file-noselect
>> >> when called from autoload-find-generated-file, please, if you don't
>> >> object?
>> >
>> > Sorry, I'm not sure that's the right solution in that case.  We should
>> > try to understand why the problem happens in the first place.  E.g., I
>> > don't see it here (but I don't use MinGW64 or MSYS2).
>>
>> Seriously (not being rude)?
>
> Yes, seriously.  You'd expect that much of me, won't you?

Yes Eli.

>> It seems a lot of work for not much benefit.
>
> Not IME.  I frequently find that the real root cause of such obscure
> problems is something altogether unexpected, and sometimes it's the
> tip of an iceberg.  So I generally find looking into the root causes
> of such problems beneficial.
>
>> That conversion happens often, e.g., in normalize_filename.
>
> What conversion is that?

The down-casing of the drive letter in a native or MSYS file name.

>> And it's unlikely a warning from find_file_noselect there is ever
>> going to be helpful in catching a new bug.
>
> You may be right.  But we will only know for sure when we have
> understood the root cause.  It could very well be that there is indeed
> a bug, and this warning has already caught it, except that it went
> unnoticed until now, because it was never reported before.
>
> For starters, can you or Angelo tell where did that upper-case "C:/"
> originate from?

You can see the warnings, for example, by invoking configure by
absolute file name with an upper-case drive letter before running make.

Usually one uses a relative path and the problem can't arise.

I used to see this when I used a complicated Perl script to build
Emacs. The upper-case drive letter was just an artifact. It was
derived from the current directory of the calling command prompt.

> Also, are there other places in the build process
> that display file names with the upper-case drive letter?

Yes. Transcript attached.

> Thanks.

Thanks.

[-- Attachment #2: build.log.gz --]
[-- Type: application/x-gzip, Size: 40363 bytes --]

Re: Suspicious warning in W64 build

[Richard Copley]

On 9 September 2017 at 05:58, Herring, Davis <herring@lanl.gov> wrote:
>> I put the URL in quotation marks to avoid the full stop being
>> interpreted as part of the URL. I suppose I'll start surrounding
>> URLs with newlines instead. Yuck!
>
> The usual convention is angles: <http://www.example.org/>.

Hmm, thanks.
See <https://www.w3.org/Addressing/URL/5.1_Wrappers.html>.

Angelo, what did happen? Did some software pick up the quote
as part of the URL? If so, what happens with angle brackets?

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Sat, 9 Sep 2017 10:55:07 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>, Angelo Graziosi <angelo.g0@libero.it>,
> 	Emacs Development <emacs-devel@gnu.org>
> 
> On 9 September 2017 at 05:58, Herring, Davis <herring@lanl.gov> wrote:
> >> I put the URL in quotation marks to avoid the full stop being
> >> interpreted as part of the URL. I suppose I'll start surrounding
> >> URLs with newlines instead. Yuck!
> >
> > The usual convention is angles: <http://www.example.org/>.
> 
> Hmm, thanks.
> See <https://www.w3.org/Addressing/URL/5.1_Wrappers.html>.
> 
> Angelo, what did happen? Did some software pick up the quote
> as part of the URL? If so, what happens with angle brackets?

FWIW, I had no problems following your original URL.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Date: Sat, 9 Sep 2017 10:49:55 +0200 (CEST)
> From: Angelo Graziosi <angelo.g0@libero.it>
> Cc: eggert@cs.ucla.edu, emacs-devel@gnu.org
> 
> Hmm... Have you tried to do an out of tree build?

No.

> Maybe I am wrong but in a tree build it only uses relative path: ../../path etc.

That doesn't seem to be the problem.  If you configure Emacs with

  --prefix=/c/whatever

i.e. with a lower-case /c/, does the problem go away?

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Sat, 9 Sep 2017 10:36:28 +0100
> Cc: Angelo Graziosi <angelo.g0@libero.it>, Emacs Development <emacs-devel@gnu.org>
> 
> >> That conversion happens often, e.g., in normalize_filename.
> >
> > What conversion is that?
> 
> The down-casing of the drive letter in a native or MSYS file name.

But find-file-noselect insists on comparing file names as simple
case-sensitive strings.  Which will fail in a few more interesting
cases.

> > For starters, can you or Angelo tell where did that upper-case "C:/"
> > originate from?
> 
> You can see the warnings, for example, by invoking configure by
> absolute file name with an upper-case drive letter before running make.
> 
> Usually one uses a relative path and the problem can't arise.
> 
> I used to see this when I used a complicated Perl script to build
> Emacs. The upper-case drive letter was just an artifact. It was
> derived from the current directory of the calling command prompt.

So you are saying that upper-case C:/ comes from the user?  And if the
user configures Emacs with a lower-case c:/, the problem will never
happen?  If so, would binding find-file-suppress-same-file-warnings
non-nil in autoload-find-generated-file solve the problem?  (This is
less drastic than invoking find-file-noselect with a non-nil 2nd
argument.)

> > Also, are there other places in the build process
> > that display file names with the upper-case drive letter?
> 
> Yes. Transcript attached.

I see a lot of warnings in that log.  It's a pity no one reports them,
let alone works on fixing them.  (I don't see any of them on my
systems.)

Thanks.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Date: Sat, 09 Sep 2017 13:42:54 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: angelo.g0@libero.it, emacs-devel@gnu.org
> 
> would binding find-file-suppress-same-file-warnings non-nil in
> autoload-find-generated-file solve the problem?

More specifically, is this just a harmless warning, or do you really
wind up with autoloads files that mention each file twice?

Re: Suspicious warning in W64 build

[Angelo Graziosi]

> Il 9 settembre 2017 alle 11.55 Richard Copley <rcopley@gmail.com> ha scritto:
> 
> 
> On 9 September 2017 at 05:58, Herring, Davis <herring@lanl.gov> wrote:
> >> I put the URL in quotation marks to avoid the full stop being
> >> interpreted as part of the URL. I suppose I'll start surrounding
> >> URLs with newlines instead. Yuck!
> >
> > The usual convention is angles: <http://www.example.org/>.
> 
> Hmm, thanks.
> See <https://www.w3.org/Addressing/URL/5.1_Wrappers.html>.
> 
> Angelo, what did happen? Did some software pick up the quote
> as part of the URL? If so, what happens with angle brackets?

The link added an extra '"', removing it, works. Thanks.

Re: Suspicious warning in W64 build

[Richard Copley]

On 9 September 2017 at 11:42, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Richard Copley <rcopley@gmail.com>
>> Date: Sat, 9 Sep 2017 10:36:28 +0100
>> Cc: Angelo Graziosi <angelo.g0@libero.it>, Emacs Development <emacs-devel@gnu.org>
>>
>> >> That conversion happens often, e.g., in normalize_filename.
>> >
>> > What conversion is that?
>>
>> The down-casing of the drive letter in a native or MSYS file name.
>
> But find-file-noselect insists on comparing file names as simple
> case-sensitive strings.  Which will fail in a few more interesting
> cases.
>
>> > For starters, can you or Angelo tell where did that upper-case "C:/"
>> > originate from?
>>
>> You can see the warnings, for example, by invoking configure by
>> absolute file name with an upper-case drive letter before running make.
>>
>> Usually one uses a relative path and the problem can't arise.
>>
>> I used to see this when I used a complicated Perl script to build
>> Emacs. The upper-case drive letter was just an artifact. It was
>> derived from the current directory of the calling command prompt.
>
> So you are saying that upper-case C:/ comes from the user?  And if the
> user configures Emacs with a lower-case c:/, the problem will never
> happen?  If so, would binding find-file-suppress-same-file-warnings
> non-nil in autoload-find-generated-file solve the problem?  (This is
> less drastic than invoking find-file-noselect with a non-nil 2nd
> argument.)
>
>> > Also, are there other places in the build process
>> > that display file names with the upper-case drive letter?
>>
>> Yes. Transcript attached.
>
> I see a lot of warnings in that log.  It's a pity no one reports them,
> let alone works on fixing them.  (I don't see any of them on my
> systems.)

I assumed nobody was interested, since there were so many. My
mistake, sorry.

Reporting the warnings by email and answering questions about what
happens when one makes various changes isn't the most convenient
edit-compile-test cycle I've seen. You have your reasons for using an
old version of the compiler. Can you install MSYS2 and MinGW-W64
somewhere just for build testing? It would be less frustrating.

At least some of the "-Wformat=" warnings are misleading.
Emacs has to use MSVC's rules for format strings, but GCC
warns based on the C standard rules implemented in GCC.

Thanks.

Re: Suspicious warning in W64 build

[Angelo Graziosi]

> Il 9 settembre 2017 alle 12.20 Eli Zaretskii <eliz@gnu.org> ha scritto:
> FWIW, I had no problems following your original URL.

Hmm.. have you tried from a browser? I clicked the link reading the mailing list from Chrome.. I had to copy the link, adding it to the browser search box and then removing the trailing '"', then it worked..

Re: Suspicious warning in W64 build

[Angelo Graziosi]

> Il 9 settembre 2017 alle 12.37 Eli Zaretskii <eliz@gnu.org> ha scritto
> That doesn't seem to be the problem.  If you configure Emacs with
> 
>   --prefix=/c/whatever
> 
> i.e. with a lower-case /c/, does the problem go away?

No because I *already* use

  prefixname="c/LocalApps/Emacs"
  [...]
  "${src_dir}/configure" --prefix="/${prefixname}"
  [...]

in my script..

Meanwhile I did a bunch of builds

  ./autoge.sh
  ./configure
  make

in and out the tree, calling 

  ../PATH/configure

and

  /tmp/PATH/configure

and had not find any message  

  C:/.../foo and c:/.../foo are the same file

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Date: Sat, 9 Sep 2017 13:24:06 +0200 (CEST)
> From: Angelo Graziosi <angelo.g0@libero.it>
> Cc: emacs-devel@gnu.org, herring@lanl.gov
> 
> 
> > Il 9 settembre 2017 alle 12.20 Eli Zaretskii <eliz@gnu.org> ha scritto:
> > FWIW, I had no problems following your original URL.
> 
> Hmm.. have you tried from a browser?

Yes, of course.  But I did that from the Emacs buffer 9which is where
I read my mail).

> I clicked the link reading the mailing list from Chrome.. I had to copy the link, adding it to the browser search box and then removing the trailing '"', then it worked..

Ah, that explains it.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Date: Sat, 9 Sep 2017 13:32:05 +0200 (CEST)
> From: Angelo Graziosi <angelo.g0@libero.it>
> Cc: eggert@cs.ucla.edu, emacs-devel@gnu.org, rcopley@gmail.com
> 
> > Il 9 settembre 2017 alle 12.37 Eli Zaretskii <eliz@gnu.org> ha scritto
> > That doesn't seem to be the problem.  If you configure Emacs with
> > 
> >   --prefix=/c/whatever
> > 
> > i.e. with a lower-case /c/, does the problem go away?
> 
> No because I *already* use
> 
>   prefixname="c/LocalApps/Emacs"
>   [...]
>   "${src_dir}/configure" --prefix="/${prefixname}"
>   [...]
> 
> in my script..

What is the value of src_dir in that script?

Re: Suspicious warning in W64 build

[Fabrice Popineau]

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

Sorry to interrupt but is your current problem related to this :
https://lists.gnu.org/archive/html/emacs-devel/2017-08/msg00030.html

Fabrice

2017-09-09 15:28 GMT+02:00 Eli Zaretskii <eliz@gnu.org>:

> > Date: Sat, 9 Sep 2017 13:32:05 +0200 (CEST)
> > From: Angelo Graziosi <angelo.g0@libero.it>
> > Cc: eggert@cs.ucla.edu, emacs-devel@gnu.org, rcopley@gmail.com
> >
> > > Il 9 settembre 2017 alle 12.37 Eli Zaretskii <eliz@gnu.org> ha scritto
> > > That doesn't seem to be the problem.  If you configure Emacs with
> > >
> > >   --prefix=/c/whatever
> > >
> > > i.e. with a lower-case /c/, does the problem go away?
> >
> > No because I *already* use
> >
> >   prefixname="c/LocalApps/Emacs"
> >   [...]
> >   "${src_dir}/configure" --prefix="/${prefixname}"
> >   [...]
> >
> > in my script..
>
> What is the value of src_dir in that script?
>
>


-- 
Fabrice Popineau
-----------------------------
CentraleSupelec
Département Informatique
3, rue Joliot Curie
91192 Gif/Yvette Cedex
Tel direct : +33 (0) 169851950
Standard : +33 (0) 169851212
------------------------------

[-- Attachment #2: Type: text/html, Size: 2458 bytes --]

Re: Suspicious warning in W64 build

[Angelo Graziosi]

> Il 9 settembre 2017 alle 15.28 Eli Zaretskii <eliz@gnu.org> ha scritto:
> 
> 
> > Date: Sat, 9 Sep 2017 13:32:05 +0200 (CEST)
> > From: Angelo Graziosi <angelo.g0@libero.it>
> > Cc: eggert@cs.ucla.edu, emacs-devel@gnu.org, rcopley@gmail.com
> > 
> > > Il 9 settembre 2017 alle 12.37 Eli Zaretskii <eliz@gnu.org> ha scritto
> > > That doesn't seem to be the problem.  If you configure Emacs with
> > > 
> > >   --prefix=/c/whatever
> > > 
> > > i.e. with a lower-case /c/, does the problem go away?
> > 
> > No because I *already* use
> > 
> >   prefixname="c/LocalApps/Emacs"
> >   [...]
> >   "${src_dir}/configure" --prefix="/${prefixname}"
> >   [...]
> > 
> > in my script..
> 
> What is the value of src_dir in that script?

The sequence is

work_dir=$(pwd)
...
srcname="src"
...
src_dir="${work_dir}/${srcname}"

Usually the script is invoked from /tmp, so

src_dir == /tmp/src/

Re: Suspicious warning in W64 build

[Angelo Graziosi]

> Il 9 settembre 2017 alle 12.37 Eli Zaretskii <eliz@gnu.org> ha scritto:
> 
> 
> > Date: Sat, 9 Sep 2017 10:49:55 +0200 (CEST)
> > From: Angelo Graziosi <angelo.g0@libero.it>
> > Cc: eggert@cs.ucla.edu, emacs-devel@gnu.org
> > 
> > Hmm... Have you tried to do an out of tree build?
> 
> No.
> 
> > Maybe I am wrong but in a tree build it only uses relative path: ../../path etc.
> 
> That doesn't seem to be the problem.  If you configure Emacs with
> 
>   --prefix=/c/whatever
> 
> i.e. with a lower-case /c/, does the problem go away?

OK, today I did at least five build, in tree and out tree, but only when I build OUT tree AND with no default prefix, I get the messages (a lot)

  C:/msys64/tmp/emacs-master/lisp/loaddefs.el and c:/msys64/tmp/emacs-master/lisp/loaddefs.el are the same file

in other words:

  wget http://git.savannah.gnu.org/cgit/emacs.git/snapshot/emacs-master.tar.gz
  mkdir build
  tar -xf emacs-master.tar.gz
  cd emacs-master
  ./autogen.sh
  cd ../build
  /tmp/emacs-master/configure --prefix=/c/LocalApps/Emacs
  make

prints the messages. If I omit --prefix=..., the messages are absent.

Notice, they are simple messages, not marked as warnings

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Sat, 9 Sep 2017 12:17:14 +0100
> Cc: Angelo Graziosi <angelo.g0@libero.it>, Emacs Development <emacs-devel@gnu.org>
> 
> > I see a lot of warnings in that log.  It's a pity no one reports them,
> > let alone works on fixing them.  (I don't see any of them on my
> > systems.)
> 
> I assumed nobody was interested, since there were so many. My
> mistake, sorry.
> 
> Reporting the warnings by email and answering questions about what
> happens when one makes various changes isn't the most convenient
> edit-compile-test cycle I've seen. You have your reasons for using an
> old version of the compiler.

I use the latest GCC version provided by mingw.org's MinGW
distribution.  Currently, that's 6.3.0.

> Can you install MSYS2 and MinGW-W64
> somewhere just for build testing? It would be less frustrating.

Sorry, this is unlikely to happen.  Being a co-maintainer eats up all
of my free time, so entertaining yet another incompatible development
environment and keeping it in good shape is not something I can
afford.

I expect others who use MinGW64 to care enough to report and fix these
problems.

> At least some of the "-Wformat=" warnings are misleading.
> Emacs has to use MSVC's rules for format strings, but GCC
> warns based on the C standard rules implemented in GCC.

Actually, these are the most worrisome, because they seem to tell your
MinGW headers might mismatch your GCC version.  Or maybe this is a
general MinGW64 problem that should be solved by MinGW64 developers.
Consider this warning, which is quite typical, and is seen in your log
many times:

    CC       frame.o
  frame.c: In function 'make_terminal_frame':
  frame.c:1098:46: warning: unknown conversion type character 'l' in format [-Wformat=]
     fset_name (f, make_formatted_string (name, "F%"pMd, ++tty_frame_count));
						^~~~
  In file included from C:/msys64/mingw64/x86_64-w64-mingw32/include/inttypes.h:299:0,
		   from C:/projects/emacs/nt/inc/inttypes.h:24,
		   from lisp.h:31,
		   from frame.c:29:
  C:/msys64/mingw64/x86_64-w64-mingw32/include/_mingw_print_pop.h:77:19: note: format string is defined here
   #define PRIdMAX "lld"
		     ^
  frame.c:1098:46: warning: too many arguments for format [-Wformat-extra-args]
     fset_name (f, make_formatted_string (name, "F%"pMd, ++tty_frame_count));
						^~~~

How come the compiler doesn't recognize format specifiers defined on
the system headers?  And note that as result GCC ignores some
arguments of fset_name, which might mean it actually generates wrong
code for this function.

This should be taken up with MinGW64 developers ASAP, because I don't
see how we can fix this in Emacs.

Another class of similar warnings is like this:

    CC       keyboard.o
  keyboard.c: In function 'cmd_error':
  keyboard.c:957:23: warning: format '%d' expects argument of type 'int', but argument 3 has type 'EMACS_INT {aka long long int}' [-Wformat=]
    sprintf (macroerror, "After %"pI"d kbd macro iterations: ",
			 ^~~~~~~~~
  keyboard.c:957:35: note: format string is defined here
    sprintf (macroerror, "After %"pI"d kbd macro iterations: ",
				~~~~~^
				%"pI"lld

You may think the compiler doesn't understand %lld, but it also
doesn't seem to understand the MS native %I64d:

  print.c: In function 'safe_debug_print':
  print.c:833:24: warning: unknown conversion type character 'I' in format [-Wformat=]
	 fprintf (stderr, "#<%s_LISP_OBJECT 0x%08"pI"x>\r\n",
			  ^~~~~~~~~~~~~~~~~~~~~~~~
  In file included from print.c:25:0:
  lisp.h:98:16: note: format string is defined here
   #   define pI "I64"
		  ^

This leaves us in a conundrum, because I don't understand what printf
format spec will MinGW64 understand and process correctly when a
64-bit integral value has to be printed.

Another problem to be taken up with MinGW64 developers is this:

    CCLD     addpm.exe
  C:/projects/emacs/nt/addpm.c:42:0: warning: "_WIN32_WINNT" redefined
   #define _WIN32_WINNT _WIN32_WINNT_WIN7

  In file included from C:/msys64/mingw64/x86_64-w64-mingw32/include/crtdefs.h:10:0,
		   from C:/msys64/mingw64/x86_64-w64-mingw32/include/stdlib.h:9,
		   from C:/projects/emacs/nt/addpm.c:37:
  C:/msys64/mingw64/x86_64-w64-mingw32/include/_mingw.h:225:0: note: this is the location of the previous definition
   #define _WIN32_WINNT 0x502

These are all MinGW system headers, so it sounds like they contradict
one another?  Maybe there's something Emacs does to trigger this, but
what is that?

Anyway, I fixed some warnings, so you should see fewer of them.
Hopefully, I didn't introduce new warnings aor problems.  If/when the
MinGW64 folks (or someone here who is "in the know") tells how to
resolve the problems with printf and _WIN32_WINNT, we can fix the
rest.

There are few warnings which are not specific to MS-Windows; I will
describe them in a separate message.

Thanks.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Date: Sat, 9 Sep 2017 16:55:25 +0200 (CEST)
> From: Angelo Graziosi <angelo.g0@libero.it>
> Cc: eggert@cs.ucla.edu, emacs-devel@gnu.org, rcopley@gmail.com
> 
> > > No because I *already* use
> > > 
> > >   prefixname="c/LocalApps/Emacs"
> > >   [...]
> > >   "${src_dir}/configure" --prefix="/${prefixname}"
> > >   [...]
> > > 
> > > in my script..
> > 
> > What is the value of src_dir in that script?
> 
> The sequence is
> 
> work_dir=$(pwd)
> ...
> srcname="src"
> ...
> src_dir="${work_dir}/${srcname}"
> 
> Usually the script is invoked from /tmp, so
> 
> src_dir == /tmp/src/

The configure script does this in the MinGW build:

  srcdir=`cd "${srcdir}" && pwd -W`

So what does "pwd -W" yield in /tmp/src on your system?

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Date: Sat, 9 Sep 2017 17:40:55 +0200 (CEST)
> From: Angelo Graziosi <angelo.g0@libero.it>
> Cc: eggert@cs.ucla.edu, emacs-devel@gnu.org, rcopley@gmail.com
> 
> OK, today I did at least five build, in tree and out tree, but only when I build OUT tree AND with no default prefix, I get the messages (a lot)
> 
>   C:/msys64/tmp/emacs-master/lisp/loaddefs.el and c:/msys64/tmp/emacs-master/lisp/loaddefs.el are the same file
> 
> in other words:
> 
>   wget http://git.savannah.gnu.org/cgit/emacs.git/snapshot/emacs-master.tar.gz
>   mkdir build
>   tar -xf emacs-master.tar.gz
>   cd emacs-master
>   ./autogen.sh
>   cd ../build
>   /tmp/emacs-master/configure --prefix=/c/LocalApps/Emacs
>   make
> 
> prints the messages. If I omit --prefix=..., the messages are absent.

Thanks.  I still don't see where did C:/, upper-case, came from.

> Notice, they are simple messages, not marked as warnings

They are warnings, see find-file-noselect.  They just don't have an
explicit "warning" in their text.

Re: Suspicious warning in W64 build

[Fabrice Popineau]

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

2017-09-09 18:40 GMT+02:00 Eli Zaretskii <eliz@gnu.org>:

> > Date: Sat, 9 Sep 2017 17:40:55 +0200 (CEST)
> > From: Angelo Graziosi <angelo.g0@libero.it>
> > Cc: eggert@cs.ucla.edu, emacs-devel@gnu.org, rcopley@gmail.com
> >
> > OK, today I did at least five build, in tree and out tree, but only when
> I build OUT tree AND with no default prefix, I get the messages (a lot)
> >
> >   C:/msys64/tmp/emacs-master/lisp/loaddefs.el and
> c:/msys64/tmp/emacs-master/lisp/loaddefs.el are the same file
> >
> > in other words:
> >
> >   wget http://git.savannah.gnu.org/cgit/emacs.git/snapshot/emacs-
> master.tar.gz
> >   mkdir build
> >   tar -xf emacs-master.tar.gz
> >   cd emacs-master
> >   ./autogen.sh
> >   cd ../build
> >   /tmp/emacs-master/configure --prefix=/c/LocalApps/Emacs
> >   make
> >
> > prints the messages. If I omit --prefix=..., the messages are absent.
>
> Thanks.  I still don't see where did C:/, upper-case, came from.
>

At the risk of being impolite and/or out of topic, I repeat here what I
already posted a while ago.

I have something quite disturbing wrt to paths and case: starting 'emacs
-Q' from the same directory,
I can get 3 different values for default-directory:

1. "D:/Source/emacs/build-master/"
This is with a mingw64 shell, after doing 'cd /d/Source/emacs/build-master'
and running 'emacs -Q'

2. "D:/source/emacs/build-master/"
This is with a mingw64 shell, after doing 'cd /d/source/emacs/build-master'
and running 'emacs -Q'

3. "d:/Source/emacs/build-master/"
This is with a cmd shell, after doing 'd:' and 'cd
d:/Source/emacs/build-master'

I think this should be traced and fixed (but didn't get a chance to do it)


Fabrice

[-- Attachment #2: Type: text/html, Size: 2756 bytes --]

Re: Suspicious warning in W64 build

[Angelo Graziosi]

> Il 9 settembre 2017 alle 18.37 Eli Zaretskii <eliz@gnu.org> ha scritto:
> 
> So what does "pwd -W" yield in /tmp/src on your system?

It prints : C:/msys64/tmp/src

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Date: Sat, 9 Sep 2017 20:38:17 +0200 (CEST)
> From: Angelo Graziosi <angelo.g0@libero.it>
> Cc: emacs-devel@gnu.org, rcopley@gmail.com
> 
> > So what does "pwd -W" yield in /tmp/src on your system?
> 
> It prints : C:/msys64/tmp/src

Thanks, I guess this is the root cause of the problem.  And
consequently, lisp/Makefile says something like

 top_srcdir = C:/msys64/tmp/src

I presume, is that right?  And if you change that to
c:/msys64/tmp/src, the problem goes away?

If so, maybe we should downcase the drive letter where we invoke
"pwd -W" in the configure script.

Re: Suspicious warning in W64 build

[Angelo Graziosi]

> Il 9 settembre 2017 alle 20.59 Eli Zaretskii <eliz@gnu.org> ha scritto:
> 
> 
> > Date: Sat, 9 Sep 2017 20:38:17 +0200 (CEST)
> > From: Angelo Graziosi <angelo.g0@libero.it>
> > Cc: emacs-devel@gnu.org, rcopley@gmail.com
> > 
> > > So what does "pwd -W" yield in /tmp/src on your system?
> > 
> > It prints : C:/msys64/tmp/src
> 
> Thanks, I guess this is the root cause of the problem.  And
> consequently, lisp/Makefile says something like
> 
>  top_srcdir = C:/msys64/tmp/src

top_srcdir = /C/msys64/tmp/src

> 
> I presume, is that right?  And if you change that to
> c:/msys64/tmp/src, the problem goes away?

top_srcdir = /c/msys64/tmp/src

Yes, it goes away..

Re: Suspicious warning in W64 build

[Richard Copley]

On 9 September 2017 at 17:07, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Richard Copley <rcopley@gmail.com>
>> Date: Sat, 9 Sep 2017 12:17:14 +0100
>> Cc: Angelo Graziosi <angelo.g0@libero.it>, Emacs Development <emacs-devel@gnu.org>
>>
>> > I see a lot of warnings in that log.  It's a pity no one reports them,
>> > let alone works on fixing them.  (I don't see any of them on my
>> > systems.)
>>
>> I assumed nobody was interested, since there were so many. My
>> mistake, sorry.
>>
>> Reporting the warnings by email and answering questions about what
>> happens when one makes various changes isn't the most convenient
>> edit-compile-test cycle I've seen. You have your reasons for using an
>> old version of the compiler.
>
> I use the latest GCC version provided by mingw.org's MinGW
> distribution.  Currently, that's 6.3.0.
>
>> Can you install MSYS2 and MinGW-W64
>> somewhere just for build testing? It would be less frustrating.
>
> Sorry, this is unlikely to happen.  Being a co-maintainer eats up all
> of my free time, so entertaining yet another incompatible development
> environment and keeping it in good shape is not something I can
> afford.

No sweat.

> I expect others who use MinGW64 to care enough to report and fix these
> problems.

I care and I'm encouraged by your words. I will do what I can.

>> At least some of the "-Wformat=" warnings are misleading.
>> Emacs has to use MSVC's rules for format strings, but GCC
>> warns based on the C standard rules implemented in GCC.
>
> Actually, these are the most worrisome, because they seem to tell your
> MinGW headers might mismatch your GCC version.  Or maybe this is a
> general MinGW64 problem that should be solved by MinGW64 developers.
> Consider this warning, which is quite typical, and is seen in your log
> many times:
>
>     CC       frame.o
>   frame.c: In function 'make_terminal_frame':
>   frame.c:1098:46: warning: unknown conversion type character 'l' in format [-Wformat=]
>      fset_name (f, make_formatted_string (name, "F%"pMd, ++tty_frame_count));
>                                                 ^~~~
>   In file included from C:/msys64/mingw64/x86_64-w64-mingw32/include/inttypes.h:299:0,
>                    from C:/projects/emacs/nt/inc/inttypes.h:24,
>                    from lisp.h:31,
>                    from frame.c:29:
>   C:/msys64/mingw64/x86_64-w64-mingw32/include/_mingw_print_pop.h:77:19: note: format string is defined here
>    #define PRIdMAX "lld"
>                      ^
>   frame.c:1098:46: warning: too many arguments for format [-Wformat-extra-args]
>      fset_name (f, make_formatted_string (name, "F%"pMd, ++tty_frame_count));
>                                                 ^~~~
>
> How come the compiler doesn't recognize format specifiers defined on
> the system headers?  And note that as result GCC ignores some
> arguments of fset_name, which might mean it actually generates wrong
> code for this function.
>
> This should be taken up with MinGW64 developers ASAP, because I don't
> see how we can fix this in Emacs.

I don't disagree. My impression is that Alexey and co. at MinGW-W64
are well aware of it. I don't know if there's a solution in the
pipeline. I'm not the best person to press the point there (but ask me
one more time and I will try).

> Another class of similar warnings is like this:
>
>     CC       keyboard.o
>   keyboard.c: In function 'cmd_error':
>   keyboard.c:957:23: warning: format '%d' expects argument of type 'int', but argument 3 has type 'EMACS_INT {aka long long int}' [-Wformat=]
>     sprintf (macroerror, "After %"pI"d kbd macro iterations: ",
>                          ^~~~~~~~~
>   keyboard.c:957:35: note: format string is defined here
>     sprintf (macroerror, "After %"pI"d kbd macro iterations: ",
>                                 ~~~~~^
>                                 %"pI"lld
>
> You may think the compiler doesn't understand %lld, but it also
> doesn't seem to understand the MS native %I64d:
>
>   print.c: In function 'safe_debug_print':
>   print.c:833:24: warning: unknown conversion type character 'I' in format [-Wformat=]
>          fprintf (stderr, "#<%s_LISP_OBJECT 0x%08"pI"x>\r\n",
>                           ^~~~~~~~~~~~~~~~~~~~~~~~
>   In file included from print.c:25:0:
>   lisp.h:98:16: note: format string is defined here
>    #   define pI "I64"
>                   ^
> This leaves us in a conundrum, because I don't understand what printf
> format spec will MinGW64 understand and process correctly when a
> 64-bit integral value has to be printed.

Yep. Sucks.

> Another problem to be taken up with MinGW64 developers is this:
>
>     CCLD     addpm.exe
>   C:/projects/emacs/nt/addpm.c:42:0: warning: "_WIN32_WINNT" redefined
>    #define _WIN32_WINNT _WIN32_WINNT_WIN7
>
>   In file included from C:/msys64/mingw64/x86_64-w64-mingw32/include/crtdefs.h:10:0,
>                    from C:/msys64/mingw64/x86_64-w64-mingw32/include/stdlib.h:9,
>                    from C:/projects/emacs/nt/addpm.c:37:
>   C:/msys64/mingw64/x86_64-w64-mingw32/include/_mingw.h:225:0: note: this is the location of the previous definition
>    #define _WIN32_WINNT 0x502
>
> These are all MinGW system headers, so it sounds like they contradict
> one another?  Maybe there's something Emacs does to trigger this, but
> what is that?

This is due to a local patch. I'm really very sorry to have distracted you
with it, when you don't have the source.

If you want, I will get you a transcript from the unpatched Emacs git
master branch as I should have done in the first place.

> Anyway, I fixed some warnings, so you should see fewer of them.
> Hopefully, I didn't introduce new warnings aor problems.  If/when the
> MinGW64 folks (or someone here who is "in the know") tells how to
> resolve the problems with printf and _WIN32_WINNT, we can fix the
> rest.
>
> There are few warnings which are not specific to MS-Windows; I will
> describe them in a separate message.
>
> Thanks.

Thanks very much.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Sun, 10 Sep 2017 02:01:09 +0100
> Cc: Angelo Graziosi <angelo.g0@libero.it>, Emacs Development <emacs-devel@gnu.org>
> 
> > I expect others who use MinGW64 to care enough to report and fix these
> > problems.
> 
> I care and I'm encouraged by your words. I will do what I can.

Thank you.

> >     CC       frame.o
> >   frame.c: In function 'make_terminal_frame':
> >   frame.c:1098:46: warning: unknown conversion type character 'l' in format [-Wformat=]
> >      fset_name (f, make_formatted_string (name, "F%"pMd, ++tty_frame_count));
> >                                                 ^~~~
> >   In file included from C:/msys64/mingw64/x86_64-w64-mingw32/include/inttypes.h:299:0,
> >                    from C:/projects/emacs/nt/inc/inttypes.h:24,
> >                    from lisp.h:31,
> >                    from frame.c:29:
> >   C:/msys64/mingw64/x86_64-w64-mingw32/include/_mingw_print_pop.h:77:19: note: format string is defined here
> >    #define PRIdMAX "lld"
> >                      ^
> >   frame.c:1098:46: warning: too many arguments for format [-Wformat-extra-args]
> >      fset_name (f, make_formatted_string (name, "F%"pMd, ++tty_frame_count));
> >                                                 ^~~~
> >
> > How come the compiler doesn't recognize format specifiers defined on
> > the system headers?  And note that as result GCC ignores some
> > arguments of fset_name, which might mean it actually generates wrong
> > code for this function.
> >
> > This should be taken up with MinGW64 developers ASAP, because I don't
> > see how we can fix this in Emacs.
> 
> I don't disagree. My impression is that Alexey and co. at MinGW-W64
> are well aware of it. I don't know if there's a solution in the
> pipeline. I'm not the best person to press the point there (but ask me
> one more time and I will try).

I can write to them myself, if you tell me what would be the best
forum to do so.

> If you want, I will get you a transcript from the unpatched Emacs git
> master branch as I should have done in the first place.

No need for now.  Let's try to fix the problems with formats first.

There are also a few problems in unexw32.c, but they only affect
debugging code, so it's not urgent to fix them (and the solution also
involves format specifications).

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Date: Sat, 9 Sep 2017 23:29:19 +0200 (CEST)
> From: Angelo Graziosi <angelo.g0@libero.it>
> Cc: emacs-devel@gnu.org, rcopley@gmail.com
> 
> > > > So what does "pwd -W" yield in /tmp/src on your system?
> > > 
> > > It prints : C:/msys64/tmp/src
> > 
> > Thanks, I guess this is the root cause of the problem.  And
> > consequently, lisp/Makefile says something like
> > 
> >  top_srcdir = C:/msys64/tmp/src
> 
> top_srcdir = /C/msys64/tmp/src
> 
> > 
> > I presume, is that right?  And if you change that to
> > c:/msys64/tmp/src, the problem goes away?
> 
> top_srcdir = /c/msys64/tmp/src
> 
> Yes, it goes away..

Thanks.

So I think these problems happen when you are building outside of the
source directory, and your source directory is inside the MSYS tree,
i.e. under the MSYS installation directory (in your case, C:/msys64).

I've just installed a small change that is supposed to fix this,
please try the latest master.

Re: Suspicious warning in W64 build

[Angelo Graziosi]

> Il 10 settembre 2017 alle 16.56 Eli Zaretskii <eliz@gnu.org> ha scritto:
> 
> I've just installed a small change that is supposed to fix this,
> please try the latest master.

Oops.. Now the build fails out the tree at configure time:

tar -xf emacs-master.tar.gz
cd emacs-master
./autogen.sh

cd ..
mkdir build
cd build
/tmp/emacs-master/configure

configure: loading site script /mingw64/etc/config.site
checking for xcrun... no
checking for GNU Make... make
checking build system type... x86_64-w64-mingw32
checking host system type... x86_64-w64-mingw32
checking the compiler's target... x86_64-w64-mingw32
/tmp/emacs-master/configure: command substitution: line 4102: unexpected EOF while looking for matching `"'
/tmp/emacs-master/configure: command substitution: line 4103: syntax error: unexpected end of file

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[...]
  Does Emacs use toolkit scroll bars?                     yes
  Does Emacs support Xwidgets (requires gtk3)?            no
  Does Emacs have threading support in lisp?              yes


configure: creating ./config.status
config.status: error: cannot find input file: `nt/emacs.rc.in'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Angelo

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Date: Sun, 10 Sep 2017 17:45:08 +0200 (CEST)
> From: Angelo Graziosi <angelo.g0@libero.it>
> Cc: emacs-devel@gnu.org, rcopley@gmail.com
> 
> Oops.. Now the build fails out the tree at configure time:

Sorry, should be fixed now.

Re: Suspicious warning in W64 build

[Angelo Graziosi]

> Il 10 settembre 2017 alle 18.02 Eli Zaretskii <eliz@gnu.org> ha scritto:
> 
> 
> > Date: Sun, 10 Sep 2017 17:45:08 +0200 (CEST)
> > From: Angelo Graziosi <angelo.g0@libero.it>
> > Cc: emacs-devel@gnu.org, rcopley@gmail.com
> > 
> > Oops.. Now the build fails out the tree at configure time:
> 
> Sorry, should be fixed now.

Yes, now it works, and as expected.

Thank you!

Re: Suspicious warning in W64 build

[Richard Copley]

On 10 September 2017 at 15:40, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Richard Copley <rcopley@gmail.com>
>> Date: Sun, 10 Sep 2017 02:01:09 +0100
>> Cc: Angelo Graziosi <angelo.g0@libero.it>, Emacs Development <emacs-devel@gnu.org>
>> > This should be taken up with MinGW64 developers ASAP, because I don't
>> > see how we can fix this in Emacs.
>>
>> I don't disagree. [...]
>
> I can write to them myself, if you tell me what would be the best
> forum to do so.

I suppose that would be their Sourceforge mailing list. See the
project's contact page:

https://mingw-w64.org/doku.php/support

Thanks.

Re: Suspicious warning in W64 build

[Angelo Graziosi]

> Il 10 settembre 2017 alle 21.14 Richard Copley <rcopley@gmail.com> ha scritto:
> 
> 
> On 10 September 2017 at 15:40, Eli Zaretskii <eliz@gnu.org> wrote:
> >> From: Richard Copley <rcopley@gmail.com>
> >> Date: Sun, 10 Sep 2017 02:01:09 +0100
> >> Cc: Angelo Graziosi <angelo.g0@libero.it>, Emacs Development <emacs-devel@gnu.org>
> >> > This should be taken up with MinGW64 developers ASAP, because I don't
> >> > see how we can fix this in Emacs.
> >>
> >> I don't disagree. [...]
> >
> > I can write to them myself, if you tell me what would be the best
> > forum to do so.
> 
> I suppose that would be their Sourceforge mailing list. See the
> project's contact page:
> 
> https://mingw-w64.org/doku.php/support

They use a 'mixing' of Sourceforge and GitHub. The 'Tickets' have been migrated to GitHub,

  https://github.com/Alexpux/MSYS2-packages  (for MSYS2, strictly)
  https://github.com/Alexpux/MINGW-packages  (for MINGW, strictly)

and closed on SF (one can only read old post).

There is also this mailing list,

  https://sourceforge.net/p/msys2/mailman/msys2-users

but for what I know, they prefer reports on GitHub

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Date: Sun, 10 Sep 2017 20:45:25 +0200 (CEST)
> From: Angelo Graziosi <angelo.g0@libero.it>
> Cc: rcopley@gmail.com, emacs-devel@gnu.org
> 
> > > Oops.. Now the build fails out the tree at configure time:
> > 
> > Sorry, should be fixed now.
> 
> Yes, now it works, and as expected.
> 
> Thank you!

Great, thanks for testing.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Date: Sun, 10 Sep 2017 21:38:58 +0200 (CEST)
> From: Angelo Graziosi <angelo.g0@libero.it>
> Cc: Emacs Development <emacs-devel@gnu.org>
> 
> > I suppose that would be their Sourceforge mailing list. See the
> > project's contact page:
> > 
> > https://mingw-w64.org/doku.php/support
> 
> They use a 'mixing' of Sourceforge and GitHub. The 'Tickets' have been migrated to GitHub,
> 
>   https://github.com/Alexpux/MSYS2-packages  (for MSYS2, strictly)
>   https://github.com/Alexpux/MINGW-packages  (for MINGW, strictly)
> 
> and closed on SF (one can only read old post).
> 
> There is also this mailing list,
> 
>   https://sourceforge.net/p/msys2/mailman/msys2-users
> 
> but for what I know, they prefer reports on GitHub

Thanks, I submitted an issue to https://github.com/Alexpux/MINGW-packages.

Re: Suspicious warning in W64 build

[Óscar Fuentes]

Angelo Graziosi <angelo.g0@libero.it> writes:

>> Il 10 settembre 2017 alle 21.14 Richard Copley <rcopley@gmail.com> ha scritto:
>> 
>> 
>> On 10 September 2017 at 15:40, Eli Zaretskii <eliz@gnu.org> wrote:
>> >> From: Richard Copley <rcopley@gmail.com>
>> >> Date: Sun, 10 Sep 2017 02:01:09 +0100
>> >> Cc: Angelo Graziosi <angelo.g0@libero.it>, Emacs Development <emacs-devel@gnu.org>
>> >> > This should be taken up with MinGW64 developers ASAP, because I don't
>> >> > see how we can fix this in Emacs.
>> >>
>> >> I don't disagree. [...]
>> >
>> > I can write to them myself, if you tell me what would be the best
>> > forum to do so.
>> 
>> I suppose that would be their Sourceforge mailing list. See the
>> project's contact page:
>> 
>> https://mingw-w64.org/doku.php/support
>
> They use a 'mixing' of Sourceforge and GitHub. The 'Tickets' have been migrated to GitHub,
>
>   https://github.com/Alexpux/MSYS2-packages  (for MSYS2, strictly)
>   https://github.com/Alexpux/MINGW-packages  (for MINGW, strictly)
>
> and closed on SF (one can only read old post).
>
> There is also this mailing list,
>
>   https://sourceforge.net/p/msys2/mailman/msys2-users
>
> but for what I know, they prefer reports on GitHub

You are confusing mingw-w64 with MSYS2. They are different projects.
MSYS2 releases binary packages built from mingw-w64 sources. The
maintainers of the mingw-w64 gcc are on http://www.mingw-w64.org, which
accepts bug reports on SF following the link mentioned above.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Óscar Fuentes <ofv@wanadoo.es>
> Date: Mon, 11 Sep 2017 18:39:16 +0200
> 
> >> https://mingw-w64.org/doku.php/support
> >
> > They use a 'mixing' of Sourceforge and GitHub. The 'Tickets' have been migrated to GitHub,
> >
> >   https://github.com/Alexpux/MSYS2-packages  (for MSYS2, strictly)
> >   https://github.com/Alexpux/MINGW-packages  (for MINGW, strictly)
> >
> > and closed on SF (one can only read old post).
> >
> > There is also this mailing list,
> >
> >   https://sourceforge.net/p/msys2/mailman/msys2-users
> >
> > but for what I know, they prefer reports on GitHub
> 
> You are confusing mingw-w64 with MSYS2. They are different projects.
> MSYS2 releases binary packages built from mingw-w64 sources. The
> maintainers of the mingw-w64 gcc are on http://www.mingw-w64.org, which
> accepts bug reports on SF following the link mentioned above.

Thanks, I posted to mingw-w64-public@lists.sourceforge.net as well.

Re: Suspicious warning in W64 build

[Angelo Graziosi]

Just for the record, this morning there was an MSYS2 update of msys2-runtime packages (aka Cygwin DLL), from 2.8.2 to 2.9.0, and now building Emacs takes more time, about > +50%, from 35 to 55 minutes. I have verified this rebuilding a previous snapshot both in 2.9.0 and 2.8.2 (downgrading).

Sorry if this is not directly related to Emacs but can be of interest for who builds it.

 Angelo

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Date: Sat, 09 Sep 2017 19:07:23 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: angelo.g0@libero.it, emacs-devel@gnu.org
> 
> > At least some of the "-Wformat=" warnings are misleading.
> > Emacs has to use MSVC's rules for format strings, but GCC
> > warns based on the C standard rules implemented in GCC.
> 
> Actually, these are the most worrisome, because they seem to tell your
> MinGW headers might mismatch your GCC version.  Or maybe this is a
> general MinGW64 problem that should be solved by MinGW64 developers.

OK, I got some information from the MinGW64 developers, but now I need
your help, Richard.  I need you to produce preprocessed versions of
frame.c, keyboard.c, and print.c, and send them to me.

Do you know how to produce such preprocessed versions?  They need to
be produced using the exact GCC switches used when these files are
compiled as part of the build.  Let me know if you need instructions.

I will then try to figure out how to get ourself out of this mess,
given what they explained to me.

Thanks.

Re: Suspicious warning in W64 build

[Fabrice Popineau]

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

2017-09-12 19:49 GMT+02:00 Eli Zaretskii <eliz@gnu.org>:

> > Date: Sat, 09 Sep 2017 19:07:23 +0300
> > From: Eli Zaretskii <eliz@gnu.org>
> > Cc: angelo.g0@libero.it, emacs-devel@gnu.org
> >
> > > At least some of the "-Wformat=" warnings are misleading.
> > > Emacs has to use MSVC's rules for format strings, but GCC
> > > warns based on the C standard rules implemented in GCC.
> >
> > Actually, these are the most worrisome, because they seem to tell your
> > MinGW headers might mismatch your GCC version.  Or maybe this is a
> > general MinGW64 problem that should be solved by MinGW64 developers.
>
> OK, I got some information from the MinGW64 developers, but now I need
> your help, Richard.  I need you to produce preprocessed versions of
> frame.c, keyboard.c, and print.c, and send them to me.
>
> Do you know how to produce such preprocessed versions?  They need to
> be produced using the exact GCC switches used when these files are
> compiled as part of the build.  Let me know if you need instructions.
>
>
As an alternative, I can provide them.
I have the same install with the same warnings.

Should you want them, they are at this address :
https://drive.google.com/file/d/0BzJyP_aI_ouHOWR6YXZ1LTRiam8/view?usp=sharing

Compilation flags :

gcc  -E -mtune=generic  -I/mingw64/include -DUSE_CRT_DLL=1 -I
/d/source/emacs/build-master/../emacs/nt/inc -Demacs  -I. -I../../emacs/src
-I../lib -I../../emacs/lib  -mtune=generic    -pthread -mms-bitfields
-isystem C:/MSys64/mingw64/include/librsvg-2.0 -isystem
C:/MSys64/mingw64/include/gdk-pixbuf-2.0 -isystem
C:/MSys64/mingw64/include/libpng16 -isystem C:/MSys64/mingw64/include/cairo
-isystem C:/MSys64/mingw64/include/pixman-1 -isystem
C:/MSys64/mingw64/include -isystem C:/MSys64/mingw64/include/freetype2
-isystem C:/MSys64/mingw64/include/libpng16 -isystem
C:/MSys64/mingw64/include/harfbuzz -isystem
C:/MSys64/mingw64/include/glib-2.0 -isystem
C:/MSys64/mingw64/lib/glib-2.0/include -isystem C:/MSys64/mingw64/include
-isystem C:/MSys64/mingw64/include/freetype2 -isystem
C:/MSys64/mingw64/include -isystem C:/MSys64/mingw64/include/harfbuzz
-isystem C:/MSys64/mingw64/include/glib-2.0 -isystem
C:/MSys64/mingw64/lib/glib-2.0/include -isystem C:/MSys64/mingw64/include
-isystem C:/MSys64/mingw64/include/libpng16 -isystem
C:/MSys64/mingw64/include -fopenmp -DMAGICKCORE_HDRI_ENABLE=1
-DMAGICKCORE_QUANTUM_DEPTH=16 -D_DLL -D_MT -fopenmp
-DMAGICKCORE_HDRI_ENABLE=1 -DMAGICKCORE_QUANTUM_DEPTH=16 -D_DLL -D_MT
-isystem C:/MSys64/mingw64/include/ImageMagick-7  -isystem
C:/MSys64/mingw64/include/libxml2            -MMD -MF deps/print.d -MP
 -isystem C:/MSys64/mingw64/include -isystem
C:/MSys64/mingw64/include/p11-kit-1    -fno-common -W -Wabi -Waddress
-Waggressive-loop-optimizations -Wall -Wattributes -Wbool-compare
-Wbool-operation -Wbuiltin-declaration-mismatch -Wbuiltin-macro-redefined
-Wcast-align -Wchar-subscripts -Wchkp -Wclobbered -Wcomment -Wcomments
-Wcoverage-mismatch -Wcpp -Wdangling-else -Wdate-time -Wdeprecated
-Wdeprecated-declarations -Wdesignated-init -Wdisabled-optimization
-Wdiscarded-array-qualifiers -Wdiscarded-qualifiers -Wdiv-by-zero
-Wdouble-promotion -Wduplicated-cond -Wduplicate-decl-specifier
-Wempty-body -Wendif-labels -Wenum-compare -Wexpansion-to-defined -Wextra
-Wformat-contains-nul -Wformat-extra-args -Wformat-security
-Wformat-signedness -Wformat-y2k -Wformat-zero-length -Wframe-address
-Wfree-nonheap-object -Whsa -Wignored-attributes -Wignored-qualifiers
-Wimplicit -Wimplicit-function-declaration -Wimplicit-int
-Wincompatible-pointer-types -Winit-self -Wint-conversion
-Wint-in-bool-context -Wint-to-pointer-cast -Winvalid-memory-model
-Winvalid-pch -Wjump-misses-init -Wlogical-not-parentheses -Wlogical-op
-Wmain -Wmaybe-uninitialized -Wmemset-elt-size -Wmemset-transposed-args
-Wmisleading-indentation -Wmissing-braces -Wmissing-declarations
-Wmissing-include-dirs -Wmissing-parameter-type -Wmissing-prototypes
-Wmultichar -Wnarrowing -Wnested-externs -Wnonnull -Wnonnull-compare
-Wnull-dereference -Wodr -Wold-style-declaration -Wold-style-definition
-Wopenmp-simd -Woverflow -Wpacked -Wpacked-bitfield-compat -Wparentheses
-Wpointer-arith -Wpointer-compare -Wpointer-to-int-cast -Wpragmas -Wpsabi
-Wrestrict -Wreturn-local-addr -Wreturn-type -Wscalar-storage-order
-Wsequence-point -Wshift-count-negative -Wshift-count-overflow
-Wshift-negative-value -Wsizeof-array-argument -Wsizeof-pointer-memaccess
-Wstrict-aliasing -Wstrict-prototypes -Wsuggest-attribute=noreturn
-Wsuggest-final-methods -Wsuggest-final-types -Wswitch-bool
-Wswitch-unreachable -Wtautological-compare -Wtrampolines -Wtrigraphs
-Wuninitialized -Wunknown-pragmas -Wunused -Wunused-but-set-parameter
-Wunused-but-set-variable -Wunused-function -Wunused-label
-Wunused-local-typedefs -Wunused-macros -Wunused-result -Wunused-value
-Wunused-variable -Wvarargs -Wvariadic-macros
-Wvector-operation-performance -Wvolatile-register-var -Wwrite-strings
-Walloc-size-larger-than=2147483647 -Warray-bounds=2 -Wformat-truncation=2
-Wimplicit-fallthrough=5 -Wnormalized=nfc -Wshift-overflow=2
-Wstringop-overflow=2 -Wvla-larger-than=4031
-Wno-missing-field-initializers -Wno-override-init -Wno-sign-compare
-Wno-type-limits -Wno-unused-parameter -Wno-format-nonliteral
-Wno-pointer-sign -I/mingw64/include -O3 -g0 -mtune=corei7
-fomit-frame-pointer  ../../emacs/src/frame.c

Fabrice

[-- Attachment #2: Type: text/html, Size: 6222 bytes --]

Re: Suspicious warning in W64 build

[Richard Copley]

On 12 September 2017 at 19:01, Fabrice Popineau
<fabrice.popineau@centralesupelec.fr> wrote:
>
> 2017-09-12 19:49 GMT+02:00 Eli Zaretskii <eliz@gnu.org>:
>>
>> > Date: Sat, 09 Sep 2017 19:07:23 +0300
>> > From: Eli Zaretskii <eliz@gnu.org>
>> > Cc: angelo.g0@libero.it, emacs-devel@gnu.org
>> >
>> > > At least some of the "-Wformat=" warnings are misleading.
>> > > Emacs has to use MSVC's rules for format strings, but GCC
>> > > warns based on the C standard rules implemented in GCC.
>> >
>> > Actually, these are the most worrisome, because they seem to tell your
>> > MinGW headers might mismatch your GCC version.  Or maybe this is a
>> > general MinGW64 problem that should be solved by MinGW64 developers.
>>
>> OK, I got some information from the MinGW64 developers, but now I need
>> your help, Richard.  I need you to produce preprocessed versions of
>> frame.c, keyboard.c, and print.c, and send them to me.
>>
>> Do you know how to produce such preprocessed versions?  They need to
>> be produced using the exact GCC switches used when these files are
>> compiled as part of the build.  Let me know if you need instructions.
>>
>
> As an alternative, I can provide them.
> I have the same install with the same warnings.

Mine are similar but contain #define directives. Let me know if you
think they might be of use. They add up to >1.5MB compressed. I can
send you a URL, or attach them, or whatever else you prefer.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>
> Date: Tue, 12 Sep 2017 20:01:58 +0200
> Cc: Richard Copley <rcopley@gmail.com>, Emacs developers <emacs-devel@gnu.org>
> 
> As an alternative, I can provide them. 
> I have the same install with the same warnings.
> 
> Should you want them, they are at this address :
> https://drive.google.com/file/d/0BzJyP_aI_ouHOWR6YXZ1LTRiam8/view?usp=sharing

Thanks.  I think I see the problems, or at least some of them, but I
need to think how to solve this for all the supported MinGW versions
to work correctly.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Tue, 12 Sep 2017 19:37:06 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>, Emacs developers <emacs-devel@gnu.org>
> 
> Mine are similar but contain #define directives. Let me know if you
> think they might be of use. They add up to >1.5MB compressed. I can
> send you a URL, or attach them, or whatever else you prefer.

A URL is best, thanks.

Re: Suspicious warning in W64 build

[Richard Copley]

On 12 September 2017 at 19:59, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Richard Copley <rcopley@gmail.com>
>> Date: Tue, 12 Sep 2017 19:37:06 +0100
>> Cc: Eli Zaretskii <eliz@gnu.org>, Emacs developers <emacs-devel@gnu.org>
>>
>> Mine are similar but contain #define directives. Let me know if you
>> think they might be of use. They add up to >1.5MB compressed. I can
>> send you a URL, or attach them, or whatever else you prefer.
>
> A URL is best, thanks.

It's at https://buster.me.uk/
with base name emacs-format-warnings-preprocessed-sources
and a .tar.gz extension.

(Apologies, this is just a Raspberry Pi on domestic broadband,
so I prefer not to attract robots. I'll delete the files after a decent
interval.)

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Date: Tue, 12 Sep 2017 21:38:24 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: rcopley@gmail.com, emacs-devel@gnu.org
> 
> > From: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>
> > Date: Tue, 12 Sep 2017 20:01:58 +0200
> > Cc: Richard Copley <rcopley@gmail.com>, Emacs developers <emacs-devel@gnu.org>
> > 
> > As an alternative, I can provide them. 
> > I have the same install with the same warnings.
> > 
> > Should you want them, they are at this address :
> > https://drive.google.com/file/d/0BzJyP_aI_ouHOWR6YXZ1LTRiam8/view?usp=sharing
> 
> Thanks.  I think I see the problems, or at least some of them, but I
> need to think how to solve this for all the supported MinGW versions
> to work correctly.

It was a mess, but I hope I fixed these warnings now.  Please try the
latest master and see if any fallout remains.

Re: Suspicious warning in W64 build

[Richard Copley]

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

On 14 September 2017 at 18:47, Eli Zaretskii <eliz@gnu.org> wrote:
> It was a mess, but I hope I fixed these warnings now.  Please try the
> latest master and see if any fallout remains.

Not much. Here's another transcript. It's a lot better.
Thanks.

[-- Attachment #2: transcript.txt.gz --]
[-- Type: application/x-gzip, Size: 37807 bytes --]

Re: Suspicious warning in W64 build

[Fabrice Popineau]

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

2017-09-14 19:47 GMT+02:00 Eli Zaretskii <eliz@gnu.org>:

> > Date: Tue, 12 Sep 2017 21:38:24 +0300
> > From: Eli Zaretskii <eliz@gnu.org>
> > Cc: rcopley@gmail.com, emacs-devel@gnu.org
> >
> > > From: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>
> > > Date: Tue, 12 Sep 2017 20:01:58 +0200
> > > Cc: Richard Copley <rcopley@gmail.com>, Emacs developers <
> emacs-devel@gnu.org>
> > >
> > > As an alternative, I can provide them.
> > > I have the same install with the same warnings.
> > >
> > > Should you want them, they are at this address :
> > > https://drive.google.com/file/d/0BzJyP_aI_ouHOWR6YXZ1LTRiam8/view?usp=
> sharing
> >
> > Thanks.  I think I see the problems, or at least some of them, but I
> > need to think how to solve this for all the supported MinGW versions
> > to work correctly.
>
> It was a mess, but I hope I fixed these warnings now.  Please try the
> latest master and see if any fallout remains.
>

Thanks for your efforts. (I wouldn't sort that out myself)
However, there are yet problems.

The ones related to pI are removed if

#define pI "ll"

which somehow fails to happen here.

-- 
Fabrice

[-- Attachment #2: Type: text/html, Size: 2473 bytes --]

Re: Suspicious warning in W64 build

[Andy Moreton]

On Thu 14 Sep 2017, Eli Zaretskii wrote:

>> Date: Tue, 12 Sep 2017 21:38:24 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: rcopley@gmail.com, emacs-devel@gnu.org
>> 
>> > From: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>
>> > Date: Tue, 12 Sep 2017 20:01:58 +0200
>> > Cc: Richard Copley <rcopley@gmail.com>, Emacs developers <emacs-devel@gnu.org>
>> > 
>> > As an alternative, I can provide them. 
>> > I have the same install with the same warnings.
>> > 
>> > Should you want them, they are at this address :
>> > https://drive.google.com/file/d/0BzJyP_aI_ouHOWR6YXZ1LTRiam8/view?usp=sharing
>> 
>> Thanks.  I think I see the problems, or at least some of them, but I
>> need to think how to solve this for all the supported MinGW versions
>> to work correctly.
>
> It was a mess, but I hope I fixed these warnings now.  Please try the
> latest master and see if any fallout remains.

There are still some issues with the pI format. This seems to fix them:

diff --git a/src/lisp.h b/src/lisp.h
index c5aea9c34c..f522e5ee1c 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -99,7 +99,7 @@ enum { EMACS_INT_WIDTH = LLONG_WIDTH, EMACS_UINT_WIDTH = ULLONG_WIDTH };
    later and the runtime version is 5.0.0 or later.  Otherwise,
    printf-like functions are declared with __ms_printf__ attribute,
    which will cause a warning for %lld etc.  */
-#  if defined __MINGW32__						\
+#  if defined __MINGW32__ && !defined MINGW_W64				\
   && (!defined __USE_MINGW_ANSI_STDIO					\
       || !(GNUC_PREREQ (6, 0, 0) && __MINGW32_MAJOR_VERSION >= 5))
 #   define pI "I64"


There are also several similar warnings in unexw32.c due to signed/unsigned mismatch:

../../src/unexw32.c: In function 'copy_executable_and_dump_data':
../../src/unexw32.c:503:10: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 2 has type 'long long int' [-Wformat=]
  printf ("\t0x%"pDWP" Offset in input file.\n", s - p_infile->file_base);  \
          ^                                      ~~~~~~~~~~~~~~~
../../src/unexw32.c:553:3: note: in expansion of macro 'COPY_CHUNK'
   COPY_CHUNK ("Copying DOS header...", dos_header,
   ^~~~~~~~~~
../../src/unexw32.c:475:21: note: format string is defined here
 # define pDWP  "16llx"
                     ^

Should these be using the ptrdiff_t formatter pD from lisp.h ?

    AndyM

Re: Suspicious warning in W64 build

[Fabrice Popineau]

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

2017-09-14 23:17 GMT+02:00 Andy Moreton <andrewjmoreton@gmail.com>:

> On Thu 14 Sep 2017, Eli Zaretskii wrote:
>
> >> Date: Tue, 12 Sep 2017 21:38:24 +0300
> >> From: Eli Zaretskii <eliz@gnu.org>
> >> Cc: rcopley@gmail.com, emacs-devel@gnu.org
> >>
> >> > From: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>
> >> > Date: Tue, 12 Sep 2017 20:01:58 +0200
> >> > Cc: Richard Copley <rcopley@gmail.com>, Emacs developers <
> emacs-devel@gnu.org>
> >> >
> >> > As an alternative, I can provide them.
> >> > I have the same install with the same warnings.
> >> >
> >> > Should you want them, they are at this address :
> >> > https://drive.google.com/file/d/0BzJyP_aI_ouHOWR6YXZ1LTRiam8
> /view?usp=sharing
> >>
> >> Thanks.  I think I see the problems, or at least some of them, but I
> >> need to think how to solve this for all the supported MinGW versions
> >> to work correctly.
> >
> > It was a mess, but I hope I fixed these warnings now.  Please try the
> > latest master and see if any fallout remains.
>
> There are still some issues with the pI format. This seems to fix them:
>
> diff --git a/src/lisp.h b/src/lisp.h
> index c5aea9c34c..f522e5ee1c 100644
> --- a/src/lisp.h
> +++ b/src/lisp.h
> @@ -99,7 +99,7 @@ enum { EMACS_INT_WIDTH = LLONG_WIDTH, EMACS_UINT_WIDTH =
> ULLONG_WIDTH };
>     later and the runtime version is 5.0.0 or later.  Otherwise,
>     printf-like functions are declared with __ms_printf__ attribute,
>     which will cause a warning for %lld etc.  */
> -#  if defined __MINGW32__                                              \
> +#  if defined __MINGW32__ && !defined MINGW_W64
>       \
>    && (!defined __USE_MINGW_ANSI_STDIO                                  \
>        || !(GNUC_PREREQ (6, 0, 0) && __MINGW32_MAJOR_VERSION >= 5))
>  #   define pI "I64"
>
>
Confirmed


> There are also several similar warnings in unexw32.c due to
> signed/unsigned mismatch:
>
> ../../src/unexw32.c: In function 'copy_executable_and_dump_data':
> ../../src/unexw32.c:503:10: warning: format '%llx' expects argument of
> type 'long long unsigned int', but argument 2 has type 'long long int'
> [-Wformat=]
>   printf ("\t0x%"pDWP" Offset in input file.\n", s -
> p_infile->file_base);  \
>           ^                                      ~~~~~~~~~~~~~~~
> ../../src/unexw32.c:553:3: note: in expansion of macro 'COPY_CHUNK'
>    COPY_CHUNK ("Copying DOS header...", dos_header,
>    ^~~~~~~~~~
> ../../src/unexw32.c:475:21: note: format string is defined here
>  # define pDWP  "16llx"
>                      ^
>
> Should these be using the ptrdiff_t formatter pD from lisp.h ?
>

I'd rather cast the values to "long long unsigned int" as these values are
assumed to be positive,
and would be meaningless if printed in decimal.

There are also a bunch of warnings in dispnew.c:adjust_glyph_matrix()
because the w parameter could be NULL. Actually, some parts of the code
are protected with if (w) { ... } but much more than those ones assume that
w is not NULL.

I wonder if the whole function should not just return if w is NULL.

Fabrice

[-- Attachment #2: Type: text/html, Size: 4843 bytes --]

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Thu, 14 Sep 2017 20:34:42 +0100
> Cc: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>, 
> 	Emacs Development <emacs-devel@gnu.org>
> 
> Not much. Here's another transcript. It's a lot better.

Thanks, please try the latest master.  I hope I fixed most of the
warnings in Windows specific code there.

I must say that GCC 7 static code analysis is really imperfect, not
good enough for producing useful warnings from its results, because
some of the warnings are outright wrong.  For example this one:

  w32term.c: In function 'w32_read_socket':
  w32term.c:4979:10: warning: 'button' is used uninitialized in this function [-Wuninitialized]
	int button;
	    ^~~~~~
  w32term.c:3099:16: warning: 'button' may be used uninitialized in this function [-Wmaybe-uninitialized]
     result->code = button;
     ~~~~~~~~~~~~~^~~~~~~~
  w32term.c:3090:7: note: 'button' was declared here
     int button;
	 ^~~~~~
  w32term.c:3104:6: warning: 'up' may be used uninitialized in this function [-Wmaybe-uninitialized]
	    | (up
	      ~~~
	? up_modifier
	~~~~~~~~~~~~~
	: down_modifier));
	^~~~~~~~~~~~~~~~
  w32term.c:3091:7: note: 'up' was declared here
     int up;
	 ^~
  w32term.c:5018:9: warning: 'up' may be used uninitialized in this function [-Wmaybe-uninitialized]
	if (up)
	   ^

Evidently, GCC 7 doesn't see that the function parse_button
initializes these 2 variables.

About this warning:

  w32.c:7550:1: warning: no previous prototype for 'sys_strerror' [-Wmissing-prototypes]
   sys_strerror (int error_no)
   ^~~~~~~~~~~~

What MinGW64 header has the prototype of strerror?  I thought it was
string.h, like in mingw.org's headers, but evidently that's not true?
If the prototype is in string.h, then why is this warning being
displayed?

A few warnings are not specific to Windows, I will describe them in a
separate message.

Thanks for the transcript, it was really useful.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Thu, 14 Sep 2017 20:34:42 +0100
> Cc: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>, 
> 	Emacs Development <emacs-devel@gnu.org>
> 
> On 14 September 2017 at 18:47, Eli Zaretskii <eliz@gnu.org> wrote:
> > It was a mess, but I hope I fixed these warnings now.  Please try the
> > latest master and see if any fallout remains.
> 
> Not much. Here's another transcript. It's a lot better.

Some of the warnings are not Windows specific, so I'm CC'ing Paul.

    CC       eval.o
  eval.c: In function 'internal_catch':
  eval.c:1431:19: warning: variable 'c' might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]
     struct handler *c = handlerlist->nextfree;
		     ^
  eval.c: In function 'internal_condition_case':
  eval.c:1431:19: warning: variable 'c' might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]
  eval.c: In function 'internal_condition_case_1':
  eval.c:1431:19: warning: variable 'c' might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]
  eval.c: In function 'internal_condition_case_2':
  eval.c:1431:19: warning: variable 'c' might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]
  eval.c: In function 'internal_condition_case_n':
  eval.c:1431:19: warning: variable 'c' might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]
  eval.c: In function 'internal_catch.constprop':
  eval.c:1431:19: warning: variable 'c' might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]

Not sure what to do about this.  Add 'volatile' to the declaration of
'c'?

    CC       search.o
  search.c: In function 'Freplace_match':
  search.c:2621:15: warning: argument 1 value '2305843009213693951' exceeds maximum object size 2147483647 [-Walloc-size-larger-than=]
	 substed = xmalloc (substed_alloc_size);
	 ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  In file included from search.c:24:0:
  lisp.h:4439:14: note: in a call to allocation function 'xmalloc' declared here
   extern void *xmalloc (size_t) ATTRIBUTE_MALLOC_SIZE ((1));
		^~~~~~~

This seems to imply that m4/manywarnings.m4 has a bug: it somehow
deduces that a 64-bit Windows build can only allocate up to LONG_MAX
bytes.  But 'long' is a 32-bit type on 64-bit MS-Windows, whereas
'size_t' is a 64-bit type, as is 'ptrdiff_t', which might be the
reason for this problem, if manywarnings.m4 somehow assumes that this
combination cannot happen.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Thu, 14 Sep 2017 22:17:26 +0100
> 
> There are still some issues with the pI format. This seems to fix them:
> 
> diff --git a/src/lisp.h b/src/lisp.h
> index c5aea9c34c..f522e5ee1c 100644
> --- a/src/lisp.h
> +++ b/src/lisp.h
> @@ -99,7 +99,7 @@ enum { EMACS_INT_WIDTH = LLONG_WIDTH, EMACS_UINT_WIDTH = ULLONG_WIDTH };
>     later and the runtime version is 5.0.0 or later.  Otherwise,
>     printf-like functions are declared with __ms_printf__ attribute,
>     which will cause a warning for %lld etc.  */
> -#  if defined __MINGW32__						\
> +#  if defined __MINGW32__ && !defined MINGW_W64				\
>    && (!defined __USE_MINGW_ANSI_STDIO					\
>        || !(GNUC_PREREQ (6, 0, 0) && __MINGW32_MAJOR_VERSION >= 5))
>  #   define pI "I64"

Thanks, but that's not quite right, because it will cause the MinGW64
build to use "%lld" even when __USE_MINGW_ANSI_STDIO is _not_ defined,
which will cause warnings of a different kind.  (Right now, we define
__USE_MINGW_ANSI_STDIO in config.h, but I wanted to make this
future-proof.)

So I pushed a slightly different change, which will hopefully fix the
problem.  Too bad MinGW64 uses a different versioning scheme...

> There are also several similar warnings in unexw32.c due to signed/unsigned mismatch:
> 
> ../../src/unexw32.c: In function 'copy_executable_and_dump_data':
> ../../src/unexw32.c:503:10: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 2 has type 'long long int' [-Wformat=]
>   printf ("\t0x%"pDWP" Offset in input file.\n", s - p_infile->file_base);  \
>           ^                                      ~~~~~~~~~~~~~~~
> ../../src/unexw32.c:553:3: note: in expansion of macro 'COPY_CHUNK'
>    COPY_CHUNK ("Copying DOS header...", dos_header,
>    ^~~~~~~~~~
> ../../src/unexw32.c:475:21: note: format string is defined here
>  # define pDWP  "16llx"
>                      ^

Darn that GCC 7 paranoia with signed/unsigned mismatches in printf!
GCC 6 doesn't warn here.

> Should these be using the ptrdiff_t formatter pD from lisp.h ?

No, that would cause the values be printed in decimal.  I installed
what should be a fix.

Thanks.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>
> Date: Fri, 15 Sep 2017 08:55:06 +0200
> Cc: Emacs developers <emacs-devel@gnu.org>
> 
>  # define pDWP "16llx"
>  ^
> 
>  Should these be using the ptrdiff_t formatter pD from lisp.h ?
> 
> I'd rather cast the values to "long long unsigned int" as these values are assumed to be positive,
> and would be meaningless if printed in decimal. 

That's what I did.

> There are also a bunch of warnings in dispnew.c:adjust_glyph_matrix()
> because the w parameter could be NULL. Actually, some parts of the code
> are protected with if (w) { ... } but much more than those ones assume that
> w is not NULL.

That's because they really cannot be NULL at that point.  GCC 7
doesn't really understand the code, so it shouldn't emit these
warnings, if it wants them to be useful.

> I wonder if the whole function should not just return if w is NULL.

No!  That function handles 2 different cases: the window-based
redisplay and the frame-based redisplay.  The former is used in GUI
sessions, where each window has its own glyph matrices, and manages
their memory.  The latter is used on TTYs, where window glyph matrices
are sub-matrices of the frame glyph matrices.  In the latter case,
allocation and reallocation of glyphs doesn't need the pointer to the
window, because it's meaningless for glyph allocation.  So w is
legitimately NULL in that case, but the function should still do its
job.

The condition matrix->pool == NULL implicitly also says that w must
not be NULL, but GCC 7 doesn't understand the control flow enough to
deduce that.  It just saw the calls like this:

	  adjust_glyph_matrix (NULL, f->desired_matrix, 0, 0, matrix_dim);

and deduced that the first argument of adjust_glyph_matrix can be NULL
(which is correct), but it didn't realize that in that case
matrix->pool is never NULL.  Note that the calls like the above one
are made from functions that manage frame-based redisplay.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Date: Fri, 15 Sep 2017 11:59:47 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: fabrice.popineau@centralesupelec.fr, emacs-devel@gnu.org
> 
> Some of the warnings are not Windows specific, so I'm CC'ing Paul.

Here's one more:

    CC       indent.o
  indent.c: In function 'scan_for_column':
  indent.c:69:10: warning: potential null pointer dereference [-Wnull-dereference]
     return 0;
	    ^
  indent.c:69:10: warning: potential null pointer dereference [-Wnull-dereference]
  indent.c:69:10: warning: potential null pointer dereference [-Wnull-dereference]
  indent.c:69:10: warning: potential null pointer dereference [-Wnull-dereference]

I don't see how this could be anything but a GCC bug.  Yes,
buffer_display_table can return NULL, but every place that calls it
makes sure the result is non-NULL before dereferencing it.  Did I miss
something?

Re: Suspicious warning in W64 build

[Fabrice Popineau]

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

2017-09-15 11:12 GMT+02:00 Eli Zaretskii <eliz@gnu.org>:

> > From: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>
> > Date: Fri, 15 Sep 2017 08:55:06 +0200
> > Cc: Emacs developers <emacs-devel@gnu.org>
> >
> >  # define pDWP "16llx"
> >  ^
> >
> >  Should these be using the ptrdiff_t formatter pD from lisp.h ?
> >
> > I'd rather cast the values to "long long unsigned int" as these values
> are assumed to be positive,
> > and would be meaningless if printed in decimal.
>
> That's what I did.
>
>
That's ok and no more warnings now.


> > There are also a bunch of warnings in dispnew.c:adjust_glyph_matrix()
> > because the w parameter could be NULL. Actually, some parts of the code
> > are protected with if (w) { ... } but much more than those ones assume
> that
> > w is not NULL.
>
> That's because they really cannot be NULL at that point.  GCC 7
> doesn't really understand the code, so it shouldn't emit these
> warnings, if it wants them to be useful.
>
> > I wonder if the whole function should not just return if w is NULL.
>
> No!


Ok. I see your eassert() added at this point.
But clearly there is a problem with GCC 7, because it doesn't understand
the implications of this eassert() :

  CC       dispnew.o
../../emacs/src/dispnew.c: In function 'adjust_frame_glyphs':
../../emacs/src/dispnew.c:392:14: warning: null pointer dereference
[-Wnull-dereference]
       left = margin_glyphs_to_reserve (w, dim.width, w->left_margin_cols);
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../emacs/src/dispnew.c:322:11: warning: null pointer dereference
[-Wnull-dereference]
       int width = w->total_cols;
           ^~~~~
../../emacs/src/dispnew.c:393:15: warning: null pointer dereference
[-Wnull-dereference]
       right = margin_glyphs_to_reserve (w, dim.width,
w->right_margin_cols);
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../emacs/src/dispnew.c:322:11: warning: null pointer dereference
[-Wnull-dereference]
       int width = w->total_cols;
           ^~~~~
In file included from ../../emacs/src/frame.h:23:0,
                 from ../../emacs/src/dispnew.c:34:
../../emacs/src/window.h:651:38: warning: null pointer dereference
[-Wnull-dereference]
 #define WINDOW_LEFT_PIXEL_EDGE(W) (W)->pixel_left
                                   ~~~^~~
../../emacs/src/dispnew.c:401:36: note: in expansion of macro
'WINDOW_LEFT_PIXEL_EDGE'
    && matrix->window_pixel_left == WINDOW_LEFT_PIXEL_EDGE (w)
                                    ^~~~~~~~~~~~~~~~~~~~~~
../../emacs/src/window.h:660:37: warning: null pointer dereference
[-Wnull-dereference]
 #define WINDOW_TOP_PIXEL_EDGE(W) (W)->pixel_top
                                  ~~~^~~
../../emacs/src/dispnew.c:402:35: note: in expansion of macro
'WINDOW_TOP_PIXEL_EDGE'
    && matrix->window_pixel_top == WINDOW_TOP_PIXEL_EDGE (w)
                                   ^~~~~~~~~~~~~~~~~~~~~
../../emacs/src/dispnew.c:404:34: warning: null pointer dereference
[-Wnull-dereference]
    && matrix->window_vscroll == w->vscroll
                                 ~^~~~~~~~~
../../emacs/src/dispnew.c:392:14: warning: null pointer dereference
[-Wnull-dereference]
       left = margin_glyphs_to_reserve (w, dim.width, w->left_margin_cols);
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../emacs/src/dispnew.c:322:11: warning: null pointer dereference
[-Wnull-dereference]
       int width = w->total_cols;
           ^~~~~
../../emacs/src/dispnew.c:393:15: warning: null pointer dereference
[-Wnull-dereference]
       right = margin_glyphs_to_reserve (w, dim.width,
w->right_margin_cols);
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../emacs/src/dispnew.c:322:11: warning: null pointer dereference
[-Wnull-dereference]
       int width = w->total_cols;
           ^~~~~
In file included from ../../emacs/src/frame.h:23:0,
                 from ../../emacs/src/dispnew.c:34:
../../emacs/src/window.h:651:38: warning: null pointer dereference
[-Wnull-dereference]
 #define WINDOW_LEFT_PIXEL_EDGE(W) (W)->pixel_left
                                   ~~~^~~
../../emacs/src/dispnew.c:401:36: note: in expansion of macro
'WINDOW_LEFT_PIXEL_EDGE'
    && matrix->window_pixel_left == WINDOW_LEFT_PIXEL_EDGE (w)
                                    ^~~~~~~~~~~~~~~~~~~~~~
../../emacs/src/window.h:660:37: warning: null pointer dereference
[-Wnull-dereference]
 #define WINDOW_TOP_PIXEL_EDGE(W) (W)->pixel_top
                                  ~~~^~~
../../emacs/src/dispnew.c:402:35: note: in expansion of macro
'WINDOW_TOP_PIXEL_EDGE'
    && matrix->window_pixel_top == WINDOW_TOP_PIXEL_EDGE (w)
                                   ^~~~~~~~~~~~~~~~~~~~~
../../emacs/src/dispnew.c:404:34: warning: null pointer dereference
[-Wnull-dereference]
    && matrix->window_vscroll == w->vscroll
                                 ~^~~~~~~~~
cc1.exe: warning: null pointer dereference [-Wnull-dereference]
cc1.exe: warning: null pointer dereference [-Wnull-dereference]

-- 
Fabrice

[-- Attachment #2: Type: text/html, Size: 7372 bytes --]

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>
> Date: Fri, 15 Sep 2017 17:33:48 +0200
> Cc: Andy Moreton <andrewjmoreton@gmail.com>, Emacs developers <emacs-devel@gnu.org>
> 
> Ok. I see your eassert() added at this point.
> But clearly there is a problem with GCC 7, because it doesn't understand
> the implications of this eassert() :

Does eassume instead of eassert help?

Re: Suspicious warning in W64 build

[Fabrice Popineau]

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

2017-09-15 17:45 GMT+02:00 Eli Zaretskii <eliz@gnu.org>:

> > From: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>
> > Date: Fri, 15 Sep 2017 17:33:48 +0200
> > Cc: Andy Moreton <andrewjmoreton@gmail.com>, Emacs developers <
> emacs-devel@gnu.org>
> >
> > Ok. I see your eassert() added at this point.
> > But clearly there is a problem with GCC 7, because it doesn't understand
> > the implications of this eassert() :
>
> Does eassume instead of eassert help?
>

It seems to do it.

Thanks,

-- 
Fabrice

[-- Attachment #2: Type: text/html, Size: 1246 bytes --]

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>
> Date: Fri, 15 Sep 2017 20:15:24 +0200
> Cc: Andy Moreton <andrewjmoreton@gmail.com>, Emacs developers <emacs-devel@gnu.org>
> 
>  Does eassume instead of eassert help?
> 
> It seems to do it.

Thanks, pushed.

Re: Suspicious warning in W64 build

[Fabrice Popineau]

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

Remaining warnings :

- indent.c:scan_for_column() -> you reported it

- search.c:Freplace_match()

../../emacs/src/search.c: In function 'Freplace_match':
../../emacs/src/search.c:2621:15: warning: argument 1 value
'2305843009213693951' exceeds maximum object size 2147483647
[-Walloc-size-larger-than=]
       substed = xmalloc (substed_alloc_size);
       ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../../emacs/src/search.c:24:0:
../../emacs/src/lisp.h:4440:14: note: in a call to allocation function
'xmalloc' declared here
 extern void *xmalloc (size_t) ATTRIBUTE_MALLOC_SIZE ((1));
              ^~~~~~~

- data.c:

../../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;
         ^~~~~

- eval.c

../../emacs/src/eval.c: In function 'internal_catch':
../../emacs/src/eval.c:1431:19: warning: variable 'c' might be clobbered by
'longjmp' or 'vfork' [-Wclobbered]
   struct handler *c = handlerlist->nextfree;
                   ^
../../emacs/src/eval.c: In function 'internal_condition_case':
../../emacs/src/eval.c:1431:19: warning: variable 'c' might be clobbered by
'longjmp' or 'vfork' [-Wclobbered]
../../emacs/src/eval.c: In function 'internal_condition_case_1':
../../emacs/src/eval.c:1431:19: warning: variable 'c' might be clobbered by
'longjmp' or 'vfork' [-Wclobbered]
../../emacs/src/eval.c: In function 'internal_condition_case_2':
../../emacs/src/eval.c:1431:19: warning: variable 'c' might be clobbered by
'longjmp' or 'vfork' [-Wclobbered]
../../emacs/src/eval.c: In function 'internal_condition_case_n':
../../emacs/src/eval.c:1431:19: warning: variable 'c' might be clobbered by
'longjmp' or 'vfork' [-Wclobbered]
../../emacs/src/eval.c: In function 'internal_catch.constprop':
../../emacs/src/eval.c:1431:19: warning: variable 'c' might be clobbered by
'longjmp' or 'vfork' [-Wclobbered]

- w32.c

../../emacs/src/w32.c:7551:1: warning: no previous prototype for
'sys_strerror' [-Wmissing-prototypes]
 sys_strerror (int error_no)
 ^~~~~~~~~~~~

Fabrice


2017-09-15 21:00 GMT+02:00 Eli Zaretskii <eliz@gnu.org>:

> > From: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>
> > Date: Fri, 15 Sep 2017 20:15:24 +0200
> > Cc: Andy Moreton <andrewjmoreton@gmail.com>, Emacs developers <
> emacs-devel@gnu.org>
> >
> >  Does eassume instead of eassert help?
> >
> > It seems to do it.
>
> Thanks, pushed.
>



-- 
Fabrice Popineau
-----------------------------
CentraleSupelec
Département Informatique
3, rue Joliot Curie
91192 Gif/Yvette Cedex
Tel direct : +33 (0) 169851950
Standard : +33 (0) 169851212
------------------------------

[-- Attachment #2: Type: text/html, Size: 4649 bytes --]

Re: Suspicious warning in W64 build

[Richard Copley]

On 15 September 2017 at 09:54, Eli Zaretskii <eliz@gnu.org> wrote:
> About this warning:
>
>   w32.c:7550:1: warning: no previous prototype for 'sys_strerror' [-Wmissing-prototypes]
>    sys_strerror (int error_no)
>    ^~~~~~~~~~~~
>
> What MinGW64 header has the prototype of strerror?  I thought it was
> string.h, like in mingw.org's headers, but evidently that's not true?
> If the prototype is in string.h, then why is this warning being
> displayed?

It is in string.h.
Forgive me if I misunderstood, but isn't it because the warning is
about a prototype for sys_strerror, not a prototype for strerror?

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Sat, 16 Sep 2017 00:05:44 +0100
> Cc: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>, 
> 	Emacs Development <emacs-devel@gnu.org>
> 
> On 15 September 2017 at 09:54, Eli Zaretskii <eliz@gnu.org> wrote:
> > About this warning:
> >
> >   w32.c:7550:1: warning: no previous prototype for 'sys_strerror' [-Wmissing-prototypes]
> >    sys_strerror (int error_no)
> >    ^~~~~~~~~~~~
> >
> > What MinGW64 header has the prototype of strerror?  I thought it was
> > string.h, like in mingw.org's headers, but evidently that's not true?
> > If the prototype is in string.h, then why is this warning being
> > displayed?
> 
> It is in string.h.
> Forgive me if I misunderstood, but isn't it because the warning is
> about a prototype for sys_strerror, not a prototype for strerror?

If you look at the beginning of w32.c, you will see that we include
string.h before we #undef strerror.  The header nt/inc/ms-w32.h
#define's strerror to sys_strerror, so the inclusion of string.h
should have supplied the prototype for sys_strerror.  Why doesn't it
do that in MinGW64?  Does some system header #undef strerror too
early?

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>
> Date: Fri, 15 Sep 2017 23:02:45 +0200
> Cc: Andy Moreton <andrewjmoreton@gmail.com>, Emacs developers <emacs-devel@gnu.org>
> 
> Remaining warnings :
> 
> - indent.c:scan_for_column() -> you reported it
> 
> - search.c:Freplace_match()
> 
> ../../emacs/src/search.c: In function 'Freplace_match':
> ../../emacs/src/search.c:2621:15: warning: argument 1 value '2305843009213693951' exceeds maximum
> object size 2147483647 [-Walloc-size-larger-than=]
> substed = xmalloc (substed_alloc_size);
> ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from ../../emacs/src/search.c:24:0:
> ../../emacs/src/lisp.h:4440:14: note: in a call to allocation function 'xmalloc' declared here
> extern void *xmalloc (size_t) ATTRIBUTE_MALLOC_SIZE ((1));
> ^~~~~~~

I already reported this, it looks like a bug in Gnulib's
manywarnings.m4.

> - data.c:
> 
> ../../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;
> ^~~~~

So now even eassume is not enough?  Anyway, should be fixed now.

> - eval.c
> 
> ../../emacs/src/eval.c: In function 'internal_catch':
> ../../emacs/src/eval.c:1431:19: warning: variable 'c' might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]
> struct handler *c = handlerlist->nextfree;
> ^
> ../../emacs/src/eval.c: In function 'internal_condition_case':
> ../../emacs/src/eval.c:1431:19: warning: variable 'c' might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]
> ../../emacs/src/eval.c: In function 'internal_condition_case_1':
> ../../emacs/src/eval.c:1431:19: warning: variable 'c' might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]
> ../../emacs/src/eval.c: In function 'internal_condition_case_2':
> ../../emacs/src/eval.c:1431:19: warning: variable 'c' might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]
> ../../emacs/src/eval.c: In function 'internal_condition_case_n':
> ../../emacs/src/eval.c:1431:19: warning: variable 'c' might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]
> ../../emacs/src/eval.c: In function 'internal_catch.constprop':
> ../../emacs/src/eval.c:1431:19: warning: variable 'c' might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]

Already reported.

> - w32.c
> 
> ../../emacs/src/w32.c:7551:1: warning: no previous prototype for 'sys_strerror' [-Wmissing-prototypes]
> sys_strerror (int error_no)
> ^~~~~~~~~~~~

Already reported; I need the MinGW64 users figure why what works for
mingw.org doesn't for MinGW64.  See my response to Richard.

Re: Suspicious warning in W64 build

[Richard Copley]

On 16 September 2017 at 07:40, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Richard Copley <rcopley@gmail.com>
>> Date: Sat, 16 Sep 2017 00:05:44 +0100
>> Cc: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>,
>>       Emacs Development <emacs-devel@gnu.org>
>>
>> On 15 September 2017 at 09:54, Eli Zaretskii <eliz@gnu.org> wrote:
>> > About this warning:
>> >
>> >   w32.c:7550:1: warning: no previous prototype for 'sys_strerror' [-Wmissing-prototypes]
>> >    sys_strerror (int error_no)
>> >    ^~~~~~~~~~~~
>> >
>> > What MinGW64 header has the prototype of strerror?  I thought it was
>> > string.h, like in mingw.org's headers, but evidently that's not true?
>> > If the prototype is in string.h, then why is this warning being
>> > displayed?
>>
>> It is in string.h.
>> Forgive me if I misunderstood, but isn't it because the warning is
>> about a prototype for sys_strerror, not a prototype for strerror?
>
> If you look at the beginning of w32.c, you will see that we include
> string.h before we #undef strerror.  The header nt/inc/ms-w32.h
> #define's strerror to sys_strerror, so the inclusion of string.h
> should have supplied the prototype for sys_strerror.  Why doesn't it
> do that in MinGW64?  Does some system header #undef strerror too
> early?

I see, thanks. No, there's no #undef. We end up including string.h before
ms-w32.h (w32.c includes io.h and io.h unconditionally includes string.h).

Re: Suspicious warning in W64 build

[Richard Copley]

On 16 September 2017 at 09:19, Richard Copley <rcopley@gmail.com> wrote:
> On 16 September 2017 at 07:40, Eli Zaretskii <eliz@gnu.org> wrote:
>>
>> If you look at the beginning of w32.c, you will see that we include
>> string.h before we #undef strerror.  The header nt/inc/ms-w32.h
>> #define's strerror to sys_strerror, so the inclusion of string.h
>> should have supplied the prototype for sys_strerror.  Why doesn't it
>> do that in MinGW64?  Does some system header #undef strerror too
>> early?
>
> I see, thanks. No, there's no #undef. We end up including string.h before
> ms-w32.h (w32.c includes io.h and io.h unconditionally includes string.h).

Also beware fcntl.h includes io.h. But string.h is not included before
ms-w32.h otherwise than via fcntl.h and io.h.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Sat, 16 Sep 2017 09:19:23 +0100
> Cc: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>, 
> 	Emacs Development <emacs-devel@gnu.org>
> 
> > If you look at the beginning of w32.c, you will see that we include
> > string.h before we #undef strerror.  The header nt/inc/ms-w32.h
> > #define's strerror to sys_strerror, so the inclusion of string.h
> > should have supplied the prototype for sys_strerror.  Why doesn't it
> > do that in MinGW64?  Does some system header #undef strerror too
> > early?
> 
> I see, thanks. No, there's no #undef. We end up including string.h before
> ms-w32.h (w32.c includes io.h and io.h unconditionally includes string.h).

Ah, okay.  I added the prototype, so this problem should be gone now.

Thanks.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Sat, 16 Sep 2017 09:34:42 +0100
> Cc: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>, 
> 	Emacs Development <emacs-devel@gnu.org>
> 
> Also beware fcntl.h includes io.h. But string.h is not included before
> ms-w32.h otherwise than via fcntl.h and io.h.

Does this constitute any danger to the change I installed?

Re: Suspicious warning in W64 build

[Richard Copley]

On 16 September 2017 at 09:54, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Richard Copley <rcopley@gmail.com>
>> Date: Sat, 16 Sep 2017 09:34:42 +0100
>> Cc: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>,
>>       Emacs Development <emacs-devel@gnu.org>
>>
>> Also beware fcntl.h includes io.h. But string.h is not included before
>> ms-w32.h otherwise than via fcntl.h and io.h.
>
> Does this constitute any danger to the change I installed?

No, it doesn't seem to. There are no warnings for w32.c now.
Thanks for looking into all of this.

Re: Suspicious warning in W64 build

[Fabrice Popineau]

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

Thanks also.

While we are at cleaning this master branch, I get 2 failures in 'make
check' :

in ibuffer-tests:

Test ibuffer-filter-inclusion-8 condition:
    (file-error "Creating file with prefix" "Invalid argument"
"c:/Users/Fabrice/AppData/Roaming/Local/Temp/ibuf-test8c")
   FAILED  11/19  ibuffer-filter-inclusion-8

Test files-tests--make-directory condition:
    (file-error "Creating directory" "Permission denied" "d:/")
   FAILED  11/11  files-tests--make-directory

Any idea about these 2 failures ?

Fabrice



2017-09-16 11:07 GMT+02:00 Richard Copley <rcopley@gmail.com>:

> On 16 September 2017 at 09:54, Eli Zaretskii <eliz@gnu.org> wrote:
> >> From: Richard Copley <rcopley@gmail.com>
> >> Date: Sat, 16 Sep 2017 09:34:42 +0100
> >> Cc: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>,
> >>       Emacs Development <emacs-devel@gnu.org>
> >>
> >> Also beware fcntl.h includes io.h. But string.h is not included before
> >> ms-w32.h otherwise than via fcntl.h and io.h.
> >
> > Does this constitute any danger to the change I installed?
>
> No, it doesn't seem to. There are no warnings for w32.c now.
> Thanks for looking into all of this.
>
>


-- 
Fabrice Popineau
-----------------------------
CentraleSupelec
Département Informatique
3, rue Joliot Curie
91192 Gif/Yvette Cedex
Tel direct : +33 (0) 169851950
Standard : +33 (0) 169851212
------------------------------

[-- Attachment #2: Type: text/html, Size: 2915 bytes --]

Re: Suspicious warning in W64 build

[Andy Moreton]

On Fri 15 Sep 2017, Eli Zaretskii wrote:

>> From: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>
>> Date: Fri, 15 Sep 2017 08:55:06 +0200
>> Cc: Emacs developers <emacs-devel@gnu.org>
>> 
>>  # define pDWP "16llx"
>>  ^
>> 
>>  Should these be using the ptrdiff_t formatter pD from lisp.h ?
>> 
>> I'd rather cast the values to "long long unsigned int" as these values are assumed to be positive,
>> and would be meaningless if printed in decimal. 
>
> That's what I did.

The Mingw64 32bit build still has warnings in unexw32.c, which the
following fixes:

diff --git a/src/unexw32.c b/src/unexw32.c
index 0c6b48342e..e97a52ba07 100644
--- a/src/unexw32.c
+++ b/src/unexw32.c
@@ -471,7 +471,7 @@ get_section_info (file_data *p_infile)
 }
 
 /* Format to print a DWORD_PTR value.  */
-#ifdef MINGW_W64
+#if defined MINGW_W64 && defined _WIN64
 # define pDWP  "16llx"
 #else
 # define pDWP  "08lx"

With this change I get clean builds on mingw64 32bit and 64bit.

   AndyM

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Sat, 16 Sep 2017 14:17:15 +0100
> 
> diff --git a/src/unexw32.c b/src/unexw32.c
> index 0c6b48342e..e97a52ba07 100644
> --- a/src/unexw32.c
> +++ b/src/unexw32.c
> @@ -471,7 +471,7 @@ get_section_info (file_data *p_infile)
>  }
>  
>  /* Format to print a DWORD_PTR value.  */
> -#ifdef MINGW_W64
> +#if defined MINGW_W64 && defined _WIN64
>  # define pDWP  "16llx"
>  #else
>  # define pDWP  "08lx"
> 
> With this change I get clean builds on mingw64 32bit and 64bit.

Thanks, pushed to the emacs-26 branch.

Re: Suspicious warning in W64 build

[Richard Copley]

On 16 September 2017 at 08:45, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>
>> Date: Fri, 15 Sep 2017 23:02:45 +0200
>> Cc: Andy Moreton <andrewjmoreton@gmail.com>, Emacs developers <emacs-devel@gnu.org>
>>
>> Remaining warnings :
[...]

Bravo Eli. For reference these are the remaining C warnings I get at -O3.

make[1]: Entering directory '/c/projects/emacs/lib-src'
  CCLD     etags.exe
etags.c: In function 'process_file_name':
etags.c:7076:20: warning: null pointer dereference [-Wnull-dereference]
   for (p = templt; *p; p++)
                    ^~
make[1]: Leaving directory '/c/projects/emacs/lib-src'
make[1]: Entering directory '/c/projects/emacs/lib-src'
  CCLD     ctags.exe
In file included from ctags.c:2:0:
etags.c: In function 'process_file_name':
etags.c:7076:20: warning: null pointer dereference [-Wnull-dereference]
   for (p = templt; *p; p++)
                    ^~
make[1]: Leaving directory '/c/projects/emacs/lib-src'
make[1]: Entering directory '/c/projects/emacs/src'
  CC       indent.o
indent.c: In function 'scan_for_column':
indent.c:69:10: warning: potential null pointer dereference [-Wnull-dereference]
   return 0;
          ^
indent.c:69:10: warning: potential null pointer dereference [-Wnull-dereference]
indent.c:69:10: warning: potential null pointer dereference [-Wnull-dereference]
indent.c:69:10: warning: potential null pointer dereference [-Wnull-dereference]
make[1]: Leaving directory '/c/projects/emacs/src'
make[1]: Entering directory '/c/projects/emacs/src'
  CC       search.o
search.c: In function 'Freplace_match':
search.c:2621:15: warning: argument 1 value '2305843009213693951'
exceeds maximum object size 2147483647 [-Walloc-size-larger-than=]
       substed = xmalloc (substed_alloc_size);
       ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from search.c:24:0:
lisp.h:4440:14: note: in a call to allocation function 'xmalloc' declared here
 extern void *xmalloc (size_t) ATTRIBUTE_MALLOC_SIZE ((1));
              ^~~~~~~
make[1]: Leaving directory '/c/projects/emacs/src'
make[1]: Entering directory '/c/projects/emacs/src'
  CC       eval.o
eval.c: In function 'internal_catch':
eval.c:1431:19: warning: variable 'c' might be clobbered by 'longjmp'
or 'vfork' [-Wclobbered]
   struct handler *c = handlerlist->nextfree;
                   ^
eval.c: In function 'internal_condition_case':
eval.c:1431:19: warning: variable 'c' might be clobbered by 'longjmp'
or 'vfork' [-Wclobbered]
eval.c: In function 'internal_condition_case_1':
eval.c:1431:19: warning: variable 'c' might be clobbered by 'longjmp'
or 'vfork' [-Wclobbered]
eval.c: In function 'internal_condition_case_2':
eval.c:1431:19: warning: variable 'c' might be clobbered by 'longjmp'
or 'vfork' [-Wclobbered]
eval.c: In function 'internal_condition_case_n':
eval.c:1431:19: warning: variable 'c' might be clobbered by 'longjmp'
or 'vfork' [-Wclobbered]
eval.c: In function 'internal_catch.constprop':
eval.c:1431:19: warning: variable 'c' might be clobbered by 'longjmp'
or 'vfork' [-Wclobbered]
make[1]: Leaving directory '/c/projects/emacs/src'

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Sat, 16 Sep 2017 19:57:06 +0100
> Cc: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>, 
> 	Andy Moreton <andrewjmoreton@gmail.com>, Emacs Development <emacs-devel@gnu.org>
> 
> Bravo Eli. For reference these are the remaining C warnings I get at -O3.
> 
> make[1]: Entering directory '/c/projects/emacs/lib-src'
>   CCLD     etags.exe
> etags.c: In function 'process_file_name':
> etags.c:7076:20: warning: null pointer dereference [-Wnull-dereference]
>    for (p = templt; *p; p++)
>                     ^~

This one's real, for a change.  Fixed.

All the rest were already reported, I hope Paul will chime in at some
point.

Re: Suspicious warning in W64 build

[Paul Eggert]

Eli Zaretskii wrote:
> Not sure what to do about this.  Add 'volatile' to the declaration of
> 'c'?

Looks like a GCC bug. To work around it, please try CACHEABLE instead of 
volatile, to warn the reader that it's a compiler-bug workaround. Like this:

   struct handler *CACHEABLE c = handlerlist->nextfree;


> This seems to imply that m4/manywarnings.m4 has a bug: it somehow
> deduces that a 64-bit Windows build can only allocate up to LONG_MAX
> bytes.

That is due to a limitation in AC_COMPUTE_INT: it can't handle numbers larger 
than LONG_MAX. I attempted to work around this limitation by changing Gnulib to 
assume that the correct value is 2**63 - 1 if it doesn't fit in 'long', and by 
merging the latest Gnulib into the emacs-26 branch. Please give it a try.

Re: Suspicious warning in W64 build

[Paul Eggert]

Eli Zaretskii wrote:
> I don't see how this could be anything but a GCC bug.  Yes,
> buffer_display_table can return NULL, but every place that calls it
> makes sure the result is non-NULL before dereferencing it.  Did I miss
> something?

Older versions of GCC sometimes "forget" obvious things like that, if they are 
spilled into memory or something (sorry, don't know the details).

Perhaps we should insist on something newer than GCC 5.3 before enabling 
warnings by default, as there is a point of diminishing returns here.

Re: Suspicious warning in W64 build

[Paul Eggert]

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

Why was the attached patch needed? What warning did it suppress? As you may 
recall, I prefer using UNINIT to suppress uninitialized-variable warnings 
because it is more automatable (e.g., it is easier to automate removing it later 
once GCC gets fixed). If UNINIT does not work I would like to know why.

[-- Attachment #2: 0001-src-data.c-minmax_driver-Fix-last-change.patch --]
[-- Type: text/x-patch, Size: 724 bytes --]

From d25d2a9b2de1a9316e982fc383d8cff06cfb41b6 Mon Sep 17 00:00:00 2001
From: Eli Zaretskii <eliz@gnu.org>
Date: Sat, 16 Sep 2017 11:01:19 +0300
Subject: [PATCH] ; * src/data.c (minmax_driver): Fix last change.

---
 src/data.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/data.c b/src/data.c
index 9ccfa6475a..95bf06e510 100644
--- a/src/data.c
+++ b/src/data.c
@@ -3011,7 +3011,7 @@ minmax_driver (ptrdiff_t nargs, Lisp_Object *args,
 	       enum Arith_Comparison comparison)
 {
   eassume (0 < nargs);
-  Lisp_Object accum UNINIT;
+  Lisp_Object accum = args[0];	/* pacify GCC */
   for (ptrdiff_t argnum = 0; argnum < nargs; argnum++)
     {
       Lisp_Object val = args[argnum];
-- 
2.13.5

Re: Suspicious warning in W64 build

[Richard Copley]

On 17 September 2017 at 07:42, Paul Eggert <eggert@cs.ucla.edu> wrote:
> Eli Zaretskii wrote:
>>
>> I don't see how this could be anything but a GCC bug.  Yes,
>> buffer_display_table can return NULL, but every place that calls it
>> makes sure the result is non-NULL before dereferencing it.  Did I miss
>> something?
>
> Older versions of GCC sometimes "forget" obvious things like that, if they
> are spilled into memory or something (sorry, don't know the details).
>
> Perhaps we should insist on something newer than GCC 5.3 before enabling
> warnings by default, as there is a point of diminishing returns here.

The warning was from GCC 7.2.0.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Cc: fabrice.popineau@centralesupelec.fr, emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sat, 16 Sep 2017 23:40:05 -0700
> 
> Eli Zaretskii wrote:
> > Not sure what to do about this.  Add 'volatile' to the declaration of
> > 'c'?
> 
> Looks like a GCC bug. To work around it, please try CACHEABLE instead of 
> volatile, to warn the reader that it's a compiler-bug workaround. Like this:
> 
>    struct handler *CACHEABLE c = handlerlist->nextfree;

Richard, Fabrice -- could you try this, please?

> > This seems to imply that m4/manywarnings.m4 has a bug: it somehow
> > deduces that a 64-bit Windows build can only allocate up to LONG_MAX
> > bytes.
> 
> That is due to a limitation in AC_COMPUTE_INT: it can't handle numbers larger 
> than LONG_MAX. I attempted to work around this limitation by changing Gnulib to 
> assume that the correct value is 2**63 - 1 if it doesn't fit in 'long', and by 
> merging the latest Gnulib into the emacs-26 branch. Please give it a try.

Did you forget to push this to the Emacs repository?  Or am I blind?

Thanks.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Cc: andrewjmoreton@gmail.com, emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> 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.

> As you may recall, I prefer using UNINIT to suppress uninitialized-variable warnings because it is more automatable (e.g., it is easier to automate removing it later once GCC gets fixed). If UNINIT does not work I would like to know why.

UNINIT looked inappropriate to initialize a Lisp_Object which is
supposed to be a number.  Even initializing with Qnil didn't look
right to me (it could raise some brows), and using UNINIT is only
equivalent to Qnil under the current representation of Qnil, and so is
insufficiently future-proof.  E.g., it could become an invalid Lisp
object if we decide to change representation at some future point.  By
contrast, args[0] was available, of the right data type, and the
comment I added makes it clear what was the reason for the
initialization.

So using args[0] sounds like a better solution here than UNINIT.  In
any case, it's a valid solution.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Sun, 17 Sep 2017 08:14:41 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>, Fabrice Popineau <fabrice.popineau@centralesupelec.fr>, 
> 	Emacs Development <emacs-devel@gnu.org>
> 
> > Perhaps we should insist on something newer than GCC 5.3 before enabling
> > warnings by default, as there is a point of diminishing returns here.
> 
> The warning was from GCC 7.2.0.

Indeed.  GCC 5 and 6 don't emit this warning in those places (or maybe
this warning is only supported starting with GCC 7?).

Re: Suspicious warning in W64 build

[Philipp Stephani]

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

Eli Zaretskii <eliz@gnu.org> schrieb am So., 17. Sep. 2017 um 16:31 Uhr:

> > Cc: andrewjmoreton@gmail.com, emacs-devel@gnu.org
> > From: Paul Eggert <eggert@cs.ucla.edu>
> > 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;
}

[-- Attachment #2: Type: text/html, Size: 2669 bytes --]

Re: Suspicious warning in W64 build

[Fabrice Popineau]

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

2017-09-17 16:29 GMT+02:00 Eli Zaretskii <eliz@gnu.org>:

> > Cc: fabrice.popineau@centralesupelec.fr, emacs-devel@gnu.org
> > From: Paul Eggert <eggert@cs.ucla.edu>
> > Date: Sat, 16 Sep 2017 23:40:05 -0700
> >
> > Eli Zaretskii wrote:
> > > Not sure what to do about this.  Add 'volatile' to the declaration of
> > > 'c'?
> >
> > Looks like a GCC bug. To work around it, please try CACHEABLE instead of
> > volatile, to warn the reader that it's a compiler-bug workaround. Like
> this:
> >
> >    struct handler *CACHEABLE c = handlerlist->nextfree;
>
> Richard, Fabrice -- could you try this, please?
>
>
This one fixes the warnings in eval.c.

Fabrice

[-- Attachment #2: Type: text/html, Size: 1311 bytes --]

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>
> Date: Sun, 17 Sep 2017 18:39:01 +0200
> Cc: Paul Eggert <eggert@cs.ucla.edu>, Richard Copley <rcopley@gmail.com>, 
> 	Emacs developers <emacs-devel@gnu.org>
> 
>  > Looks like a GCC bug. To work around it, please try CACHEABLE instead of
>  > volatile, to warn the reader that it's a compiler-bug workaround. Like this:
>  >
>  > struct handler *CACHEABLE c = handlerlist->nextfree;
> 
>  Richard, Fabrice -- could you try this, please?
> 
> This one fixes the warnings in eval.c.

Thanks, pushed to the emacs-26 branch.

Re: Suspicious warning in W64 build

[Paul Eggert]

Eli Zaretskii wrote:
> UNINIT looked inappropriate to initialize a Lisp_Object which is
> supposed to be a number.
No, it's appropriate. UNINIT is merely a declaration directive. It tells GCC 
"This variable is not initialized and that is OK; don't warn me about it, I know 
that the initial value is never used even though you may not know." The current 
UNINIT implementation would still work even if we were to change the Lisp_Object 
representation so that all-zero would become invalid, because UNINIT declares 
that the invalid value will not be used.

No code should depend on UNINIT initializing to zero, as this is not true in 
general.

Although args[0] stops GCC from complaining, it is less useful than UNINIT 
because it will make cleanup more difficult in the future. We're already using 
UNINIT for Lisp_Object variables elsewhere, so we already know it's safe to use 
here.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Cc: fabrice.popineau@centralesupelec.fr, andrewjmoreton@gmail.com,
>  emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun, 17 Sep 2017 10:07:42 -0700
> 
> Although args[0] stops GCC from complaining, it is less useful than UNINIT 
> because it will make cleanup more difficult in the future.

I don't see it that way, but these are minor stylistic differences of
opinion that we don't have to agree about.

Re: Suspicious warning in W64 build

[Paul Eggert]

Eli Zaretskii wrote:
> these are minor stylistic differences

Although they are minor, they are semantic differences not stylistic. UNINIT is 
better on technical grounds; among other things, it causes GCC to generate 
more-efficient code in production builds.

As the main objection to UNINIT appears to be stylistic, perhaps we can 
ameliorate that somehow. We could change its name to ATTRIBUTE_UNINIT, say, so 
that declarations look like this:

   Lisp_Object dest ATTRIBUTE_UNINIT;

Would that be better?

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Cc: fabrice.popineau@centralesupelec.fr, andrewjmoreton@gmail.com,
>  emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun, 17 Sep 2017 11:53:26 -0700
> 
> As the main objection to UNINIT appears to be stylistic, perhaps we can 
> ameliorate that somehow. We could change its name to ATTRIBUTE_UNINIT, say, so 
> that declarations look like this:
> 
>    Lisp_Object dest ATTRIBUTE_UNINIT;
> 
> Would that be better?

Not really.  My problem with both of these constructs is that they are
syntactically not C.  IOW, we are using the preprocessor to change how
code looks syntactically, and that is bad IMO.  It runs afoul of CC
mode's fontifications, for example.

Re: Suspicious warning in W64 build

[Paul Eggert]

Eli Zaretskii wrote:
> [UNINIT] runs afoul of CC mode's fontifications

Fontification of UNINIT works for me in CC mode, because Emacs's .dir-locals.el 
file marks UNINIT as a noise macro name. Is there some reason it doesn't work 
for you? Perhaps we can fix that.

Re: Suspicious warning in W64 build

[Paul Eggert]

Fabrice Popineau wrote:
> But clearly there is a problem with GCC 7, because it doesn't understand
> the implications of this eassert() :

No, GCC 7 is correct here, because eassert (X) does not guarantee that X is true 
after eassert returns. There's a longstanding tradition for this in C, as assert 
(X) behaves similarly.

As you discovered later, you need eassume (X) to have a guarantee that GCC trusts.

Re: Suspicious warning in W64 build

[Paul Eggert]

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

Philipp Stephani wrote:
> How about rewriting the function body like this:

Yes, that should work and avoids either UNINIT or some other pacifier. Also, 
while looking at it, I noticed an actual bug in min and max, which is a plus. I 
installed the attached into emacs-26, to fix both problems.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-bug-with-min-and-max-and-NaNs.patch --]
[-- Type: text/x-patch; name="0001-Fix-bug-with-min-and-max-and-NaNs.patch", Size: 2126 bytes --]

From 5f28f0db73c03b98b27e04a458ebb209b5d9acde Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 17 Sep 2017 15:25:44 -0700
Subject: [PATCH] Fix bug with min and max and NaNs

* src/data.c (minmax_driver): Fix bug with (min 0 NaN), which
mistakenly yielded 0.  Also, pacify GCC in a better way.
* test/src/data-tests.el (data-tests-min): Test for the bug.
---
 src/data.c             | 12 ++++++------
 test/src/data-tests.el |  6 +++++-
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/data.c b/src/data.c
index 95bf06e..e070be6 100644
--- a/src/data.c
+++ b/src/data.c
@@ -3010,16 +3010,16 @@ static Lisp_Object
 minmax_driver (ptrdiff_t nargs, Lisp_Object *args,
 	       enum Arith_Comparison comparison)
 {
-  eassume (0 < nargs);
-  Lisp_Object accum = args[0];	/* pacify GCC */
-  for (ptrdiff_t argnum = 0; argnum < nargs; argnum++)
+  Lisp_Object accum = args[0];
+  CHECK_NUMBER_OR_FLOAT_COERCE_MARKER (accum);
+  for (ptrdiff_t argnum = 1; argnum < nargs; argnum++)
     {
       Lisp_Object val = args[argnum];
       CHECK_NUMBER_OR_FLOAT_COERCE_MARKER (val);
-      if (argnum == 0 || !NILP (arithcompare (val, accum, comparison)))
+      if (!NILP (arithcompare (val, accum, comparison)))
 	accum = val;
-      else if (FLOATP (accum) && isnan (XFLOAT_DATA (accum)))
-	return accum;
+      else if (FLOATP (val) && isnan (XFLOAT_DATA (val)))
+	return val;
     }
   return accum;
 }
diff --git a/test/src/data-tests.el b/test/src/data-tests.el
index 5dc2634..8de8c14 100644
--- a/test/src/data-tests.el
+++ b/test/src/data-tests.el
@@ -101,7 +101,11 @@
   (should (= 3 (apply #'min '(3 8 3))))
   (should-error (min 9 8 'foo))
   (should-error (min (make-marker)))
-  (should (eql 1 (min (point-min-marker) 1))))
+  (should (eql 1 (min (point-min-marker) 1)))
+  (should (isnan (min 0.0e+NaN)))
+  (should (isnan (min 0.0e+NaN 1 2)))
+  (should (isnan (min 1.0 0.0e+NaN)))
+  (should (isnan (min 1.0 0.0e+NaN 1.1))))
 
 ;; Bool vector tests.  Compactly represent bool vectors as hex
 ;; strings.
-- 
2.7.4

Re: Suspicious warning in W64 build

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > Looks like a GCC bug.

Who will report the GCC bug?

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

Re: Suspicious warning in W64 build

[Paul Eggert]

>> I attempted to work around this limitation by changing Gnulib to
>> assume that the correct value is 2**63 - 1 if it doesn't fit in 'long', and by
>> merging the latest Gnulib into the emacs-26 branch. Please give it a try.
> Did you forget to push this to the Emacs repository?

Yes. Actually I did a 'push', but didn't notice that the push was to another 
repository of mine, not to Savannah. It's a setup screwup that occurred when I 
adjusted to the new emacs-26 branch. It should be fixed now, in commit 
6bbbc38b3421723521f7cdd4fd617a4fc889aceb to the emacs-26 branch.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Cc: fabrice.popineau@centralesupelec.fr, andrewjmoreton@gmail.com,
>  emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun, 17 Sep 2017 13:34:30 -0700
> 
> Eli Zaretskii wrote:
> > [UNINIT] runs afoul of CC mode's fontifications
> 
> Fontification of UNINIT works for me in CC mode, because Emacs's .dir-locals.el 
> file marks UNINIT as a noise macro name. Is there some reason it doesn't work 
> for you? Perhaps we can fix that.

I don't know what "works" means in this context.  It doesn't fontify
UNINIT as an assignment that it is.

In any case, fontification is just part of the issue.  Syntax is the
important part.  I don't like macros that change syntax too much.

Re: Suspicious warning in W64 build

[Paul Eggert]

Eli Zaretskii wrote:
> I don't know what "works" means in this context.

It means that the rest of the C source code is fontified as usual, and 
fontification isn't confused by use of UNINIT.

> It doesn't fontify UNINIT as an assignment that it is.

UNINIT is an attribute for a declarator; it is not an assignment, and should not 
be fontified as an assignment. Although the UNINIT macro might be implemented as 
an '=' followed by an initializer at the low level, this is not always true and 
UNINIT's user should not assume this implementation detail.

Emacs uses lots of other macros for declaration attributes, such as 
ATTRIBUTE_UNUSED and ATTRIBUTE_CONST. These macros all "change syntax" but 
that's acceptable when there is no reasonable alternative using ordinary C 
syntax. So UNINIT is not doing anything unique here; it's got company.

Re: Suspicious warning in W64 build

[Fabrice Popineau]

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

2017-09-18 2:26 GMT+02:00 Paul Eggert <eggert@cs.ucla.edu>:

> I attempted to work around this limitation by changing Gnulib to
>>> assume that the correct value is 2**63 - 1 if it doesn't fit in 'long',
>>> and by
>>> merging the latest Gnulib into the emacs-26 branch. Please give it a try.
>>>
>> Did you forget to push this to the Emacs repository?
>>
>
> Yes. Actually I did a 'push', but didn't notice that the push was to
> another repository of mine, not to Savannah. It's a setup screwup that
> occurred when I adjusted to the new emacs-26 branch. It should be fixed
> now, in commit 6bbbc38b3421723521f7cdd4fd617a4fc889aceb to the emacs-26
> branch.
>

The warning in src/search.c has disappeared.

But why commit this to the emacs-26 branch and not to the master branch ?

-- 
Fabrice

[-- Attachment #2: Type: text/html, Size: 1569 bytes --]

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> Cc: fabrice.popineau@centralesupelec.fr, andrewjmoreton@gmail.com,
>  emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun, 17 Sep 2017 21:52:04 -0700
> 
> Eli Zaretskii wrote:
> > I don't know what "works" means in this context.
> 
> It means that the rest of the C source code is fontified as usual, and 
> fontification isn't confused by use of UNINIT.

It is here, because:

> > It doesn't fontify UNINIT as an assignment that it is.
> 
> UNINIT is an attribute for a declarator; it is not an assignment, and should not 
> be fontified as an assignment.

It expands into an assignment, so it's an assignment.

> Emacs uses lots of other macros for declaration attributes, such as 
> ATTRIBUTE_UNUSED and ATTRIBUTE_CONST. These macros all "change syntax" but 
> that's acceptable when there is no reasonable alternative using ordinary C 
> syntax. So UNINIT is not doing anything unique here; it's got company.

ATTRIBUTE_* is bad enough already, but at least it expands to
something very similar, so syntactically it's better than UNINIT.

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Fabrice Popineau <fabrice.popineau@centralesupelec.fr>
> Date: Mon, 18 Sep 2017 13:47:28 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>, Richard Copley <rcopley@gmail.com>, 
> 	Emacs developers <emacs-devel@gnu.org>
> 
>  fixed now, in commit 6bbbc38b3421723521f7cdd4fd617a4fc889aceb to the emacs-26 branch.
> 
> The warning in src/search.c has disappeared.

Thanks for telling us.

> But why commit this to the emacs-26 branch and not to the master branch ?

Because that's our practice for bugs relevant to a release branch when
that branch is active.

Re: Suspicious warning in W64 build

[Paul Eggert]

Eli Zaretskii wrote:
> It expands into an assignment, so it's an assignment.

UNINIT does not expand into an assignment. Sometime it expands into nothing. At 
other times it expands into '=' followed by an initializer. Neither is an 
assignment. And either way, the user of UNINIT is not supposed to care what 
UNINIT expands into; that's an implementation detail. From the user's point of 
view, UNINIT is a declarator attribute.

There are lots more syntactically-incorrect macros other than the UNINIT and the 
ATTRIBUTE_* ones. They include DECLARE_POINTER_ALIAS, FALLTHROUGH, INLINE, 
FOR_EACH_TAIL, and the list goes on and on. I agree that we should avoid such 
constructs if possible, but each of these constructs is useful and is 
unavoidably outside the usual C syntax. UNINIT is similar.

Re: Suspicious warning in W64 build

[Andy Moreton]

On Mon 18 Sep 2017, Paul Eggert wrote:

> Eli Zaretskii wrote:
>> It expands into an assignment, so it's an assignment.
>
> UNINIT does not expand into an assignment. Sometime it expands into nothing.
> At other times it expands into '=' followed by an initializer. Neither is an
> assignment. And either way, the user of UNINIT is not supposed to care what
> UNINIT expands into; that's an implementation detail. From the user's point of
> view, UNINIT is a declarator attribute.
>
> There are lots more syntactically-incorrect macros other than the UNINIT and
> the ATTRIBUTE_* ones. They include DECLARE_POINTER_ALIAS, FALLTHROUGH, INLINE,
> FOR_EACH_TAIL, and the list goes on and on. I agree that we should avoid such
> constructs if possible, but each of these constructs is useful and is
> unavoidably outside the usual C syntax. UNINIT is similar.

However the whole idea of UNINIT is dubious, and invites overly clever
compilers to exploit undefined behaviour in the name of optimisation,
and emit broken code that does not meet the expectations of the
programmer.

If the compiler complains about an uninitialised use, then fix the code to
initialise the variable and remove the UB rather than disguising it with
an UNINIT macro.

    AndyM

Re: Suspicious warning in W64 build

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Mon, 18 Sep 2017 10:35:13 -0700
> Cc: fabrice.popineau@centralesupelec.fr, andrewjmoreton@gmail.com,
> 	emacs-devel@gnu.org
> 
> There are lots more syntactically-incorrect macros other than the UNINIT and the 
> ATTRIBUTE_* ones. They include DECLARE_POINTER_ALIAS, FALLTHROUGH, INLINE, 
> FOR_EACH_TAIL, and the list goes on and on. I agree that we should avoid such 
> constructs if possible, but each of these constructs is useful and is 
> unavoidably outside the usual C syntax. UNINIT is similar.

Like I said before: let's not argue about minor stylistic issues about
which we don't have to agree.

Re: Suspicious warning in W64 build

[Paul Eggert]

Andy Moreton wrote:
> the whole idea of UNINIT is dubious, and invites overly clever
> compilers to exploit undefined behaviour in the name of optimisation,
> and emit broken code that does not meet the expectations of the
> programmer.

There's no such invitation, as UNINIT is used only when GCC incorrectly warns 
about variables being uninitialized. Although undefined behavior would occur if 
the variables were used before initialization, they are not used that way. 
Clever compilers cannot exploit undefined behavior that does not exist.

These glitches arise because GCC does not warn only when it can prove that an 
variable will be used before being initialized (as that wouldn't be all that 
useful). Instead, GCC uses not-completely-reliable heuristics that sometimes 
result in false positives. Although GCC cannot exploit these heuristics to 
optimize code (because in general code optimized via unreliable heuristics might 
behave incorrectly), GCC can use these heuristics to generate warnings (because 
false-positive warnings are an acceptable price to pay if they're rare enough 
and if the correct warnings are useful enough).

UNINIT is not the only macro used to pacify GCC, by the way. There is also 
CACHEABLE, and I think others.

> If the compiler complains about an uninitialised use, then fix the code to
> initialise the variable and remove the UB rather than disguising it with
> an UNINIT macro.

The code is not broken, and if "fixing" it makes it worse then we should leave 
it alone. The main goal here is to write clear and correct and efficient code; 
pacifying false positives is secondary.