Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"-Wformat-overflow=' warnings in sv.c when compiling with gcc or g++ #16843

Open
p5pRT opened this issue Feb 6, 2019 · 9 comments
Open

"-Wformat-overflow=' warnings in sv.c when compiling with gcc or g++ #16843

p5pRT opened this issue Feb 6, 2019 · 9 comments

Comments

@p5pRT
Copy link

p5pRT commented Feb 6, 2019

Migrated from rt.perl.org#133830 (status was 'open')

Searchable as RT133830$

@p5pRT
Copy link
Author

p5pRT commented Feb 6, 2019

From @jkeenan

Since we're approaching code freeze, let's call attention to
a small number of build-time ('make') warnings.

On the FreeBSD-11.2 box where I regularly run smoke-tests of
blead, I built blead at commit 4a1f176 (Feb 05 2019) with
each of the three compilers I customarily use during
smoke-testing. Hence, I configured in these ways​:

#####
sh ./Configure -des -Dusedevel -Duseithreads -Doptimize="-O2 -pipe
-fstack-protector -fno-strict-aliasing" -Dcc="clang -Qunused-arguments"

sh ./Configure -des -Dusedevel -Duseithreads -Doptimize="-O2 -pipe
-fstack-protector -fno-strict-aliasing" -Dcc="gcc"

sh ./Configure -des -Dusedevel -Duseithreads -Doptimize="-O2 -pipe
-fstack-protector -fno-strict-aliasing" -Dcc=g++
#####

In each case I recorded the output of 'make test_prep',
grepped the file for warnings, then set aside warnings
generated during the build of modules distributed with core
but maintained upstream (e.g., Encode; IO​::Compress).

That left 0 build-time warnings directly attributable to p5p
when I built with clang, and only a handful when I built
with gcc or g++. See attached files. Indeed, I suspect
that these '-Wformat-overflow=' warnings recorded
differently in the gcc and g++ files probably express the
same underlying problem. If anyone wants to take a crack at
eliminating those warnings (without damaging anything else),
please do so.

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Feb 6, 2019

From @jkeenan

g++ -c -DPERL_CORE -DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_FORTIFY_SOURCE=2 -ansi -O2 -pipe -fstack-protector -fno-strict-aliasing -Wall -Werror=declaration-after-statement -Werror=pointer-arith -Wextra -Wwrite-strings sv.c
sv.c​: In function 'void Perl_sv_vcatpvfn_flags(PerlInterpreter*, SV*, const char*, STRLEN, __va_list_tag (*)[1], SV**, size_t, bool*, U32)'​:
sv.c​:11872​:1​: warning​: '%.*g' directive writing between 1 and 133 bytes into a region of size 127 [-Wformat-overflow=]
Perl_sv_vcatpvfn_flags(pTHX_ SV *const sv, const char *const pat, const STRLEN patlen,
^~~~~~~~~~~~~~~~~~~~~~
sv.c​:11872​:1​: note​: assuming directive output of 132 bytes
In file included from sv.c​:32​:0​:
perl.h​:392​:68​: note​: 'sprintf' output between 2 and 134 bytes into a destination of size 127
# define PERL_UNUSED_RESULT(v) STMT_START { __typeof__(v) z = (v); (void)sizeof(z); } STMT_END
  ^
sv.c​:48​:5​: note​: in expansion of macro 'PERL_UNUSED_RESULT'
  PERL_UNUSED_RESULT(Gconvert((NV)(nv), (int)ndig, 0, buffer))
  ^~~~~~~~~~~~~~~~~~
sv.c​:13060​:17​: note​: in expansion of macro 'SNPRINTF_G'
  SNPRINTF_G(fv, ebuf, sizeof(ebuf), precis);
  ^

@p5pRT
Copy link
Author

p5pRT commented Feb 6, 2019

From @jkeenan

gcc -c -DPERL_CORE -DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_FORTIFY_SOURCE=2 -std=c89 -O2 -pipe -fstack-protector -fno-strict-aliasing -Wall -Werror=declaration-after-statement -Werror=pointer-arith -Wextra -Wc++-compat -Wwrite-strings sv.c
In file included from sv.c​:32​:0​:
sv.c​: In function 'Perl_sv_vcatpvfn_flags'​:
config.h​:915​:39​: warning​: '%.*g' directive writing between 1 and 133 bytes into a region of size 127 [-Wformat-overflow=]
#define Gconvert(x,n,t,b) sprintf((b),"%.*g",(n),(x))
  ^
perl.h​:392​:67​: note​: in definition of macro 'PERL_UNUSED_RESULT'
# define PERL_UNUSED_RESULT(v) STMT_START { __typeof__(v) z = (v); (void)sizeof(z); } STMT_END
  ^
sv.c​:48​:24​: note​: in expansion of macro 'Gconvert'
  PERL_UNUSED_RESULT(Gconvert((NV)(nv), (int)ndig, 0, buffer))
  ^~~~~~~~
sv.c​:13060​:17​: note​: in expansion of macro 'SNPRINTF_G'
  SNPRINTF_G(fv, ebuf, sizeof(ebuf), precis);
  ^
config.h​:915​:39​: note​: assuming directive output of 132 bytes
#define Gconvert(x,n,t,b) sprintf((b),"%.*g",(n),(x))
  ^
perl.h​:392​:67​: note​: in definition of macro 'PERL_UNUSED_RESULT'
# define PERL_UNUSED_RESULT(v) STMT_START { __typeof__(v) z = (v); (void)sizeof(z); } STMT_END
  ^
sv.c​:48​:24​: note​: in expansion of macro 'Gconvert'
  PERL_UNUSED_RESULT(Gconvert((NV)(nv), (int)ndig, 0, buffer))
  ^~~~~~~~
sv.c​:13060​:17​: note​: in expansion of macro 'SNPRINTF_G'
  SNPRINTF_G(fv, ebuf, sizeof(ebuf), precis);
  ^
perl.h​:392​:62​: note​: 'sprintf' output between 2 and 134 bytes into a destination of size 127
# define PERL_UNUSED_RESULT(v) STMT_START { __typeof__(v) z = (v); (void)sizeof(z); } STMT_END
  ^
sv.c​:48​:5​: note​: in expansion of macro 'PERL_UNUSED_RESULT'
  PERL_UNUSED_RESULT(Gconvert((NV)(nv), (int)ndig, 0, buffer))
  ^~~~~~~~~~~~~~~~~~
sv.c​:13060​:17​: note​: in expansion of macro 'SNPRINTF_G'
  SNPRINTF_G(fv, ebuf, sizeof(ebuf), precis);
  ^

@p5pRT
Copy link
Author

p5pRT commented Feb 6, 2019

From @jkeenan

On Wed, 06 Feb 2019 19​:23​:15 GMT, jkeenan@​pobox.com wrote​:

Since we're approaching code freeze, let's call attention to
a small number of build-time ('make') warnings.

On the FreeBSD-11.2 box where I regularly run smoke-tests of
blead, I built blead at commit 4a1f176 (Feb 05 2019) with
each of the three compilers I customarily use during
smoke-testing. Hence, I configured in these ways​:

#####
sh ./Configure -des -Dusedevel -Duseithreads -Doptimize="-O2 -pipe
-fstack-protector -fno-strict-aliasing" -Dcc="clang -Qunused-arguments"

sh ./Configure -des -Dusedevel -Duseithreads -Doptimize="-O2 -pipe
-fstack-protector -fno-strict-aliasing" -Dcc="gcc"

sh ./Configure -des -Dusedevel -Duseithreads -Doptimize="-O2 -pipe
-fstack-protector -fno-strict-aliasing" -Dcc=g++
#####

In each case I recorded the output of 'make test_prep',
grepped the file for warnings, then set aside warnings
generated during the build of modules distributed with core
but maintained upstream (e.g., Encode; IO​::Compress).

That left 0 build-time warnings directly attributable to p5p
when I built with clang, and only a handful when I built
with gcc or g++. See attached files. Indeed, I suspect
that these '-Wformat-overflow=' warnings recorded
differently in the gcc and g++ files probably express the
same underlying problem. If anyone wants to take a crack at
eliminating those warnings (without damaging anything else),
please do so.

Thank you very much.
Jim Keenan

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Feb 6, 2019

From @jkeenan

Summary of my perl5 (revision 5 version 29 subversion 8) configuration​:
  Commit id​: 4a1f176
  Platform​:
  osname=freebsd
  osvers=11.2-stable
  archname=amd64-freebsd-thread-multi
  uname='freebsd perlmonger.nycbug.org 11.2-stable freebsd 11.2-stable #0 r339445​: sat oct 20 00​:08​:11 utc 2018 root@​perlmonger.nycbug.org​:usrobjusrsrcsysgeneric amd64 '
  config_args='-des -Dusedevel -Duseithreads -Doptimize=-O2 -pipe -fstack-protector -fno-strict-aliasing -Dcc=g++'
  hint=recommended
  useposix=true
  d_sigaction=define
  useithreads=define
  usemultiplicity=define
  use64bitint=define
  use64bitall=define
  uselongdouble=undef
  usemymalloc=n
  default_inc_excludes_dot=define
  bincompat5005=undef
  Compiler​:
  cc='g++'
  ccflags ='-DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_FORTIFY_SOURCE=2'
  optimize='-O2 -pipe -fstack-protector -fno-strict-aliasing'
  cppflags='-DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
  ccversion=''
  gccversion='7.3.0'
  gccosandvers=''
  intsize=4
  longsize=8
  ptrsize=8
  doublesize=8
  byteorder=12345678
  doublekind=3
  d_longlong=define
  longlongsize=8
  d_longdbl=define
  longdblsize=16
  longdblkind=3
  ivtype='long'
  ivsize=8
  nvtype='double'
  nvsize=8
  Off_t='off_t'
  lseeksize=8
  alignbytes=8
  prototype=define
  Linker and Libraries​:
  ld='g++'
  ldflags ='-pthread -Wl,-E -fstack-protector-strong -L/usr/local/lib'
  libpth=/usr/lib /usr/local/lib /usr/local/lib/gcc7/include/c++/ /usr/local/lib/gcc7/include/c++//x86_64-portbld-freebsd11.1 /usr/local/lib/gcc7/include/c++//backward /usr/local/lib /usr/lib
  libs=-lpthread -lgdbm -ldl -lm -lcrypt -lutil
  perllibs=-lpthread -ldl -lm -lcrypt -lutil
  libc=
  so=so
  useshrplib=false
  libperl=libperl.a
  gnulibc_version=''
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs
  dlext=so
  d_dlsymun=undef
  ccdlflags=' '
  cccdlflags='-DPIC -fPIC'
  lddlflags='-shared -L/usr/local/lib -fstack-protector-strong'

Characteristics of this binary (from libperl)​:
  Compile-time options​:
  HAS_TIMES
  MULTIPLICITY
  PERLIO_LAYERS
  PERL_COPY_ON_WRITE
  PERL_DONT_CREATE_GVSV
  PERL_IMPLICIT_CONTEXT
  PERL_MALLOC_WRAP
  PERL_OP_PARENT
  PERL_PRESERVE_IVUV
  PERL_USE_DEVEL
  USE_64_BIT_ALL
  USE_64_BIT_INT
  USE_ITHREADS
  USE_LARGE_FILES
  USE_LOCALE
  USE_LOCALE_COLLATE
  USE_LOCALE_CTYPE
  USE_LOCALE_NUMERIC
  USE_LOCALE_TIME
  USE_PERLIO
  USE_PERL_ATOF
  USE_REENTRANT_API
  Built under freebsd
  Compiled at Feb 6 2019 18​:49​:16
  %ENV​:
  PERL2DIR="/home/jkeenan/gitwork/perl2"
  PERL_WORKDIR="/home/jkeenan/gitwork/perl"
  @​INC​:
  lib
  /usr/local/lib/perl5/site_perl/5.29.8/amd64-freebsd-thread-multi
  /usr/local/lib/perl5/site_perl/5.29.8
  /usr/local/lib/perl5/5.29.8/amd64-freebsd-thread-multi
  /usr/local/lib/perl5/5.29.8

@p5pRT
Copy link
Author

p5pRT commented Feb 6, 2019

From [Unknown Contact. See original ticket]

On Wed, 06 Feb 2019 19​:23​:15 GMT, jkeenan@​pobox.com wrote​:

Since we're approaching code freeze, let's call attention to
a small number of build-time ('make') warnings.

On the FreeBSD-11.2 box where I regularly run smoke-tests of
blead, I built blead at commit 4a1f176 (Feb 05 2019) with
each of the three compilers I customarily use during
smoke-testing. Hence, I configured in these ways​:

#####
sh ./Configure -des -Dusedevel -Duseithreads -Doptimize="-O2 -pipe
-fstack-protector -fno-strict-aliasing" -Dcc="clang -Qunused-arguments"

sh ./Configure -des -Dusedevel -Duseithreads -Doptimize="-O2 -pipe
-fstack-protector -fno-strict-aliasing" -Dcc="gcc"

sh ./Configure -des -Dusedevel -Duseithreads -Doptimize="-O2 -pipe
-fstack-protector -fno-strict-aliasing" -Dcc=g++
#####

In each case I recorded the output of 'make test_prep',
grepped the file for warnings, then set aside warnings
generated during the build of modules distributed with core
but maintained upstream (e.g., Encode; IO​::Compress).

That left 0 build-time warnings directly attributable to p5p
when I built with clang, and only a handful when I built
with gcc or g++. See attached files. Indeed, I suspect
that these '-Wformat-overflow=' warnings recorded
differently in the gcc and g++ files probably express the
same underlying problem. If anyone wants to take a crack at
eliminating those warnings (without damaging anything else),
please do so.

Thank you very much.
Jim Keenan

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Feb 7, 2019

From @tonycoz

On Wed, 06 Feb 2019 11​:23​:15 -0800, jkeenan@​pobox.com wrote​:

Since we're approaching code freeze, let's call attention to
a small number of build-time ('make') warnings.

On the FreeBSD-11.2 box where I regularly run smoke-tests of
blead, I built blead at commit 4a1f176 (Feb 05 2019) with
each of the three compilers I customarily use during
smoke-testing. Hence, I configured in these ways​:

#####
sh ./Configure -des -Dusedevel -Duseithreads -Doptimize="-O2 -pipe
-fstack-protector -fno-strict-aliasing" -Dcc="clang -Qunused-arguments"

sh ./Configure -des -Dusedevel -Duseithreads -Doptimize="-O2 -pipe
-fstack-protector -fno-strict-aliasing" -Dcc="gcc"

sh ./Configure -des -Dusedevel -Duseithreads -Doptimize="-O2 -pipe
-fstack-protector -fno-strict-aliasing" -Dcc=g++
#####

In each case I recorded the output of 'make test_prep',
grepped the file for warnings, then set aside warnings
generated during the build of modules distributed with core
but maintained upstream (e.g., Encode; IO​::Compress).

That left 0 build-time warnings directly attributable to p5p
when I built with clang, and only a handful when I built
with gcc or g++. See attached files. Indeed, I suspect
that these '-Wformat-overflow=' warnings recorded
differently in the gcc and g++ files probably express the
same underlying problem. If anyone wants to take a crack at
eliminating those warnings (without damaging anything else),
please do so.

I think the warning is a false positive in that the code very carefully checks that the sprintf() result fits into the buffer before using it.

That said, using sprintf() (or gcvt(), which is what is used on Linux) is a bit of a code smell.

We could fix this pretty easily for this case by modifying the code block around line 43 in sv.c to fallback to using snprintf() if available and only using Gconvert() if it isn't.

It won't fix the more general use of Gconvert() though.

Tony

@p5pRT
Copy link
Author

p5pRT commented Feb 7, 2019

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Feb 7, 2019

From @jkeenan

On Thu, 07 Feb 2019 00​:54​:30 GMT, tonyc wrote​:

On Wed, 06 Feb 2019 11​:23​:15 -0800, jkeenan@​pobox.com wrote​:

Since we're approaching code freeze, let's call attention to
a small number of build-time ('make') warnings.

On the FreeBSD-11.2 box where I regularly run smoke-tests of
blead, I built blead at commit 4a1f176 (Feb 05 2019) with
each of the three compilers I customarily use during
smoke-testing. Hence, I configured in these ways​:

#####
sh ./Configure -des -Dusedevel -Duseithreads -Doptimize="-O2 -pipe
-fstack-protector -fno-strict-aliasing" -Dcc="clang -Qunused-
arguments"

sh ./Configure -des -Dusedevel -Duseithreads -Doptimize="-O2 -pipe
-fstack-protector -fno-strict-aliasing" -Dcc="gcc"

sh ./Configure -des -Dusedevel -Duseithreads -Doptimize="-O2 -pipe
-fstack-protector -fno-strict-aliasing" -Dcc=g++
#####

In each case I recorded the output of 'make test_prep',
grepped the file for warnings, then set aside warnings
generated during the build of modules distributed with core
but maintained upstream (e.g., Encode; IO​::Compress).

That left 0 build-time warnings directly attributable to p5p
when I built with clang, and only a handful when I built
with gcc or g++. See attached files. Indeed, I suspect
that these '-Wformat-overflow=' warnings recorded
differently in the gcc and g++ files probably express the
same underlying problem. If anyone wants to take a crack at
eliminating those warnings (without damaging anything else),
please do so.

I think the warning is a false positive in that the code very
carefully checks that the sprintf() result fits into the buffer before
using it.

That said, using sprintf() (or gcvt(), which is what is used on Linux)
is a bit of a code smell.

We could fix this pretty easily for this case by modifying the code
block around line 43 in sv.c to fallback to using snprintf() if
available and only using Gconvert() if it isn't.

It won't fix the more general use of Gconvert() though.

Tony

Whatever you think best. (Beyond my C-fu.)

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants