bug#32230: 27.0.50; GCC 7 warning due to -Wformat-overflow

bug#32230: 27.0.50; GCC 7 warning due to -Wformat-overflow

[Ken Brown]

The Cygwin-w32 build has turned up another GCC 7 warning:

../../master/src/w32term.c: In function ‘x_draw_glyph_string’:
../../master/src/w32term.c:1477:27: warning: ‘__builtin___sprintf_chk’ 
may write a terminating nul past the end of the destination 
[-Wformat-overflow=]
     sprintf ((char *) buf, "%0*X",
                            ^
../../master/src/w32term.c:1477:4: note: ‘__builtin___sprintf_chk’ 
output between 5 and 8 bytes into a destination of size 7
     sprintf ((char *) buf, "%0*X",
     ^
The method that Paul used in bug#32189 could be used here too, but I'm
not sure it's worth the trouble.  My inclination is to just increase the
buffer size from 7 to 8.

Ken

bug#32230: 27.0.50; GCC 7 warning due to -Wformat-overflow

[Eli Zaretskii]

> From: Ken Brown <kbrown@cornell.edu>
> Date: Fri, 20 Jul 2018 18:02:07 -0400
> Cc: Paul Eggert <eggert@cs.ucla.edu>
> 
> The Cygwin-w32 build has turned up another GCC 7 warning:
> 
> ../../master/src/w32term.c: In function ‘x_draw_glyph_string’:
> ../../master/src/w32term.c:1477:27: warning: ‘__builtin___sprintf_chk’ 
> may write a terminating nul past the end of the destination 
> [-Wformat-overflow=]
>      sprintf ((char *) buf, "%0*X",
>                             ^
> ../../master/src/w32term.c:1477:4: note: ‘__builtin___sprintf_chk’ 
> output between 5 and 8 bytes into a destination of size 7
>      sprintf ((char *) buf, "%0*X",
>      ^
> The method that Paul used in bug#32189 could be used here too, but I'm
> not sure it's worth the trouble.  My inclination is to just increase the
> buffer size from 7 to 8.

Yes, with a comment explaining why 8.

But do you understand why it says "between 5 and 8 bytes"?  The
corresponding argument to snprintf is either 4 or 6, so it should be
between 5 and 7.  Where could the extra byte come from?  Is that
because glyphless.ch is a 25-bit field?

bug#32230: 27.0.50; GCC 7 warning due to -Wformat-overflow

[Eli Zaretskii]

> Date: Sat, 21 Jul 2018 09:53:04 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 32230@debbugs.gnu.org, eggert@cs.ucla.edu
> 
> But do you understand why it says "between 5 and 8 bytes"?  The
> corresponding argument to snprintf is either 4 or 6, so it should be
> between 5 and 7.  Where could the extra byte come from?  Is that
> because glyphless.ch is a 25-bit field?

I meant sprintf, of course.  And if the above is the reason, then
perhaps using something like

  (unsigned int) glyph->u.glyphless.ch & 0xffffff

is a cleaner solution (assuming it shuts up the warning)?

bug#32230: 27.0.50; GCC 7 warning due to -Wformat-overflow

[Ken Brown]

On 7/21/2018 3:26 AM, Eli Zaretskii wrote:
>> Date: Sat, 21 Jul 2018 09:53:04 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: 32230@debbugs.gnu.org, eggert@cs.ucla.edu
>>
>> But do you understand why it says "between 5 and 8 bytes"?  The
>> corresponding argument to snprintf is either 4 or 6, so it should be
>> between 5 and 7.  Where could the extra byte come from?  Is that
>> because glyphless.ch is a 25-bit field?

Yes.  Changing the field size to 24 silences the warning.

> I meant sprintf, of course.  And if the above is the reason, then
> perhaps using something like
> 
>    (unsigned int) glyph->u.glyphless.ch & 0xffffff
> 
> is a cleaner solution (assuming it shuts up the warning)?

Yes, it does shut up the warning.  Thanks, I'll make that change.  I 
assume it should go to master, or would you rather have it in the 
emacs-26 branch?

Ken

bug#32230: 27.0.50; GCC 7 warning due to -Wformat-overflow

[Eli Zaretskii]

> Cc: 32230@debbugs.gnu.org, eggert@cs.ucla.edu
> From: Ken Brown <kbrown@cornell.edu>
> Date: Sat, 21 Jul 2018 08:42:09 -0400
> 
> > I meant sprintf, of course.  And if the above is the reason, then
> > perhaps using something like
> > 
> >    (unsigned int) glyph->u.glyphless.ch & 0xffffff
> > 
> > is a cleaner solution (assuming it shuts up the warning)?
> 
> Yes, it does shut up the warning.  Thanks, I'll make that change.  I 
> assume it should go to master, or would you rather have it in the 
> emacs-26 branch?

I think it should go to master, thanks.

bug#32230: 27.0.50; GCC 7 warning due to -Wformat-overflow

[Ken Brown]

On 7/21/2018 8:47 AM, Eli Zaretskii wrote:
> I think it should go to master, thanks.

Done, as commit f3f67cf.  But I forgot to reference the bug number in 
the commit message.  I guess there's no way to fix that, except to edit 
ChangeLog.3.  Sorry.

Ken

bug#32230: 27.0.50; GCC 7 warning due to -Wformat-overflow

[Eli Zaretskii]

> Cc: 32230-done@debbugs.gnu.org, eggert@cs.ucla.edu
> From: Ken Brown <kbrown@cornell.edu>
> Date: Sat, 21 Jul 2018 12:26:41 -0400
> 
> Done, as commit f3f67cf.  But I forgot to reference the bug number in 
> the commit message.

Happens to me as well.

> I guess there's no way to fix that, except to edit ChangeLog.3.
> Sorry.

No sweat.