Message ID | CAF2FnvBig3eN83D-PhsbvH7oacxL48s4XU1g85==VWFCo5Cuuw@mail.gmail.com |
---|---|
State | New |
Headers | show |
Series | glibc code coverage | expand |
On Wed, 2 Dec 2020, Ashutosh Pandey via Libc-alpha wrote: > Enable building glibc with instrumentation for code coverage. For > + individual files '--coverage' can be used, but this fails > + when it is used as an option with '../configure' command to build > glibc. > + To successfully build glibc with coverage, you must first build and > install glibc > + with shared libraries enabled. This will allow some files such as > + 'libc-modules.h' to be generated which are needed for code coverage to > + work. > + The command '../configure --disable-shared --enable-gcov > --without-selinux > + --disable-nscd --prefix=/path/to/install' > + will build glibc with code coverage. Note that not including a prefix, > + or setting the prefix to usr/lib can affect the system glibc and can > + render the system unusable. The command to make is 'make CXX=', without > + which you will get a error 'cannot find -lgcc_s'. Much of this reads more like a list of caveats about a work-in-progress patch, than a description of a configure option that is ready for inclusion in glibc. I'd expect such issues to be fixed, so that the new option can be used by itself, before it should go in glibc. If there is some reason it's inherently hard to fix those issues, the proposed commit message should explain *why* those issues arise and are hard to fix. I know glibc has historically been hard to build and had many pitfalls in building it, but we should do better than that, which includes making new build-time features more robust. (If something about the build environment doesn't work for a feature, a clear error message at configure time is much better than an obscure error part way through the build - whether or not that error part way through the build is mentioned in the installation manual.) glibc should be configured using --prefix=/usr; a non-default glibc should be installed in a different sysroot, not configured with a different prefix. (usr/lib isn't a prefix, it's a directory relative to a sysroot.) Could you explain the design decisions made in more detail? This patch looks like the configure option changes how the existing libraries are built. Why is it doing things that way rather than something like --enable-profile: an option that enables an additional set of libraries built with coverage options, alongside the normal shared and static libraries? Something like --enable-profile, that can be used with any other glibc configure options rather than needing a series of other options, including poorly-tested ones such as --disable-shared, could be more convenient for users - even if in practice it's more of an option for glibc developers than one where distributions would be expected to provided packages of the coverage-enabled libraries. I suppose there's the question of whether coverage-enabled shared libraries make sense - if they do, it's harder to have them share a sysroot with normal shared libraries than it is to have a separate set of coverage-enabled static libraries. So supporting coverage-enabled shared libraries would be one reason to have the configure option work as it does in this patch. The documentation of the configure option in install.texi will need to make clear which choice is made (separate libraries versus changing how static / shared libraries are built). Note that install.texi, saying what the option does, is separate from the commit message, which should discuss more about the rationale and design choices - separate text will be needed for each place. A short NEWS entry will also be needed for a new configure option. At least one build with the new option should be added to build-many-glibcs.py and verified to work there. Note that configure should be regenerated with *unmodified* upstream autoconf, not with a version with distribution patches that introduce changes (--runstatedir) unrelated to the subject of the patch.
On Thu, 3 Dec 2020, 12:15 am Joseph Myers, <joseph@codesourcery.com> wrote: > On Wed, 2 Dec 2020, Ashutosh Pandey via Libc-alpha wrote: > > > Enable building glibc with instrumentation for code coverage. For > > + individual files '--coverage' can be used, but this fails > > + when it is used as an option with '../configure' command to build > > glibc. > > + To successfully build glibc with coverage, you must first build and > > install glibc > > + with shared libraries enabled. This will allow some files such as > > + 'libc-modules.h' to be generated which are needed for code coverage > to > > + work. > > + The command '../configure --disable-shared --enable-gcov > > --without-selinux > > + --disable-nscd --prefix=/path/to/install' > > + will build glibc with code coverage. Note that not including a > prefix, > > + or setting the prefix to usr/lib can affect the system glibc and can > > + render the system unusable. The command to make is 'make CXX=', > without > > + which you will get a error 'cannot find -lgcc_s'. > > Much of this reads more like a list of caveats about a work-in-progress > patch, than a description of a configure option that is ready for > inclusion in glibc. I'd expect such issues to be fixed, so that the new > option can be used by itself, before it should go in glibc. If there is > some reason it's inherently hard to fix those issues, the proposed commit > message should explain *why* those issues arise and are hard to fix. > > I know glibc has historically been hard to build and had many pitfalls in > building it, but we should do better than that, which includes making new > build-time features more robust. (If something about the build > environment doesn't work for a feature, a clear error message at configure > time is much better than an obscure error part way through the build - > whether or not that error part way through the build is mentioned in the > installation manual.) > > glibc should be configured using --prefix=/usr; a non-default glibc should > be installed in a different sysroot, not configured with a different > prefix. (usr/lib isn't a prefix, it's a directory relative to a sysroot.) > > Could you explain the design decisions made in more detail? This patch > looks like the configure option changes how the existing libraries are > built. Why is it doing things that way rather than something like > --enable-profile: an option that enables an additional set of libraries > built with coverage options, alongside the normal shared and static > libraries? Something like --enable-profile, that can be used with any > other glibc configure options rather than needing a series of other > options, including poorly-tested ones such as --disable-shared, could be > more convenient for users - even if in practice it's more of an option for > glibc developers than one where distributions would be expected to > provided packages of the coverage-enabled libraries. > > I suppose there's the question of whether coverage-enabled shared > libraries make sense - if they do, it's harder to have them share a > sysroot with normal shared libraries than it is to have a separate set of > coverage-enabled static libraries. So supporting coverage-enabled shared > libraries would be one reason to have the configure option work as it does > in this patch. > > The documentation of the configure option in install.texi will need to > make clear which choice is made (separate libraries versus changing how > static / shared libraries are built). Note that install.texi, saying what > the option does, is separate from the commit message, which should discuss > more about the rationale and design choices - separate text will be needed > for each place. A short NEWS entry will also be needed for a new > configure option. At least one build with the new option should be added > to build-many-glibcs.py and verified to work there. > > Note that configure should be regenerated with *unmodified* upstream > autoconf, not with a version with distribution patches that introduce > changes (--runstatedir) unrelated to the subject of the patch. > > -- > Joseph S. Myers > joseph@codesourcery.com Hi, Thank you for your feedback. I'm still working on improvements to the patch. Progress is slow because I'm a beginner, but I'm still at it! In the feedback you said: I suppose there's the question of whether coverage-enabled shared libraries make sense - if they do, it's harder to have them share a sysroot with normal shared libraries than it is to have a separate set of coverage-enabled static libraries. So supporting coverage-enabled shared libraries would be one reason to have the configure option work as it does in this patch. I actually gave a presentation at the recently concluded ELISA Workshop #6 where I discussed the patch, among other things. One of the attendees suggested that I link it here, so I am doing that. Here's the Google drive link: https://drive.google.com/file/d/1eW3rz5vvvCTeCoLKZh72P1wPL3RYSYDp/view I'm not sure about the other questions, but this should provide some insight as to why this patch would be needed (atleast for a specific subset of developers). Once again, thank you for your time. Regards, Ashutosh
From 3297a01fd688d35d5e242819688ba7e6150e77f2 Mon Sep 17 00:00:00 2001 From: Ashutosh Pandey <ashutoshpandey123456@gmail.com> Date: Tue, 24 Nov 2020 00:20:03 +0530 Subject: [PATCH] glibc code coverage --- INSTALL | 15 +++++++++++++++ Makeconfig | 7 +++++++ config.make.in | 1 + configure | 28 +++++++++++++++++++++++++++- configure.ac | 8 ++++++++ manual/install.texi | 15 +++++++++++++++ 6 files changed, 73 insertions(+), 1 deletion(-) diff --git a/INSTALL b/INSTALL index 2b00f80df5..56923fbc8b 100644 --- a/INSTALL +++ b/INSTALL @@ -111,6 +111,21 @@ if 'CFLAGS' is specified it must enable optimization. For example: systems support shared libraries; you need ELF support and (currently) the GNU linker. +'--enable-gcov' + Enable building glibc with instrumentation for code coverage. For + individual files '--coverage' can be used, but this fails + when it is used as an option with '../configure' command to build glibc. + To successfully build glibc with coverage, you must first build and install glibc + with shared libraries enabled. This will allow some files such as + 'libc-modules.h' to be generated which are needed for code coverage to + work. + The command '../configure --disable-shared --enable-gcov --without-selinux + --disable-nscd --prefix=/path/to/install' + will build glibc with code coverage. Note that not including a prefix, + or setting the prefix to usr/lib can affect the system glibc and can + render the system unusable. The command to make is 'make CXX=', without + which you will get a error 'cannot find -lgcc_s'. + '--enable-static-pie' Enable static position independent executable (static PIE) support. Static PIE is similar to static executable, but can be loaded at diff --git a/Makeconfig b/Makeconfig index 8074613b85..5956de9d44 100644 --- a/Makeconfig +++ b/Makeconfig @@ -610,6 +610,13 @@ endif link-libc-static = -Wl,--start-group $(common-objpfx)libc.a $(static-gnulib) -Wl,--end-group link-libc-static-tests = -Wl,--start-group $(common-objpfx)libc.a $(static-gnulib-tests) -Wl,--end-group +# support for code coverage with gcov +ifeq (yes,$(build-gcov)) ++cflags += -fprofile-arcs -ftest-coverage -O2 +link-libc-static = -Wl,--start-group -lgcov $(common-objpfx)libc.a $(static-gnulib) -Wl,--end-group +link-libc-static-tests = -Wl,--start-group -lgcov $(common-objpfx)libc.a $(static-gnulib-tests) -Wl,--end-group +endif + # How to link against libgcc. Some libgcc functions, such as those # for "long long" arithmetic or software floating point, can always be # built without use of C library headers and do not have any global diff --git a/config.make.in b/config.make.in index 1ac9417245..463e9d808c 100644 --- a/config.make.in +++ b/config.make.in @@ -92,6 +92,7 @@ build-shared = @shared@ build-pic-default= @libc_cv_pic_default@ build-pie-default= @libc_cv_pie_default@ cc-pie-default= @libc_cv_cc_pie_default@ +build-gcov = @gcov@ build-profile = @profile@ build-static-nss = @static_nss@ cross-compiling = @cross_compiling@ diff --git a/configure b/configure index 4795e721e5..21f8582e16 100755 --- a/configure +++ b/configure @@ -683,6 +683,7 @@ force_install bindnow hardcoded_path_in_tests enable_timezone_tools +gcov extra_nonshared_cflags use_default_link sysheaders @@ -731,6 +732,7 @@ infodir docdir oldincludedir includedir +runstatedir localstatedir sharedstatedir sysconfdir @@ -765,6 +767,7 @@ with_default_link with_nonshared_cflags enable_sanity_checks enable_shared +enable_gcov enable_profile enable_static_pie enable_timezone_tools @@ -842,6 +845,7 @@ datadir='${datarootdir}' sysconfdir='${prefix}/etc' sharedstatedir='${prefix}/com' localstatedir='${prefix}/var' +runstatedir='${localstatedir}/run' includedir='${prefix}/include' oldincludedir='/usr/include' docdir='${datarootdir}/doc/${PACKAGE_TARNAME}' @@ -1094,6 +1098,15 @@ do | -silent | --silent | --silen | --sile | --sil) silent=yes ;; + -runstatedir | --runstatedir | --runstatedi | --runstated \ + | --runstate | --runstat | --runsta | --runst | --runs \ + | --run | --ru | --r) + ac_prev=runstatedir ;; + -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \ + | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \ + | --run=* | --ru=* | --r=*) + runstatedir=$ac_optarg ;; + -sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb) ac_prev=sbindir ;; -sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \ @@ -1231,7 +1244,7 @@ fi for ac_var in exec_prefix prefix bindir sbindir libexecdir datarootdir \ datadir sysconfdir sharedstatedir localstatedir includedir \ oldincludedir docdir infodir htmldir dvidir pdfdir psdir \ - libdir localedir mandir + libdir localedir mandir runstatedir do eval ac_val=\$$ac_var # Remove trailing slashes. @@ -1384,6 +1397,7 @@ Fine tuning of the installation directories: --sysconfdir=DIR read-only single-machine data [PREFIX/etc] --sharedstatedir=DIR modifiable architecture-independent data [PREFIX/com] --localstatedir=DIR modifiable single-machine data [PREFIX/var] + --runstatedir=DIR modifiable per-process data [LOCALSTATEDIR/run] --libdir=DIR object code libraries [EPREFIX/lib] --includedir=DIR C header files [PREFIX/include] --oldincludedir=DIR C header files for non-gcc [/usr/include] @@ -1420,6 +1434,8 @@ Optional Features: --disable-sanity-checks really do not use threads (should not be used except in special situations) [default=yes] --enable-shared build shared library [default=yes if GNU ld] + --enable-gcov build with instrumentation for gcov code coverage + [default=no] --enable-profile build profiled library [default=no] --enable-static-pie enable static PIE support and use it in the testsuite [default=no] @@ -3361,6 +3377,16 @@ else shared=yes fi + +# Check whether --enable-gcov was given. +if test "${enable_gcov+set}" = set; then : + enableval=$enable_gcov; gcov=$enableval +else + gcov=no +fi + + + # Check whether --enable-profile was given. if test "${enable_profile+set}" = set; then : enableval=$enable_profile; profile=$enableval diff --git a/configure.ac b/configure.ac index 93e68fb696..d5b27a4909 100644 --- a/configure.ac +++ b/configure.ac @@ -174,6 +174,14 @@ AC_ARG_ENABLE([shared], [build shared library @<:@default=yes if GNU ld@:>@]), [shared=$enableval], [shared=yes]) + +AC_ARG_ENABLE([gcov], + AC_HELP_STRING([--enable-gcov], + [build with instrumentation for gcov code coverage library @<:@default=no@:>@]), + [gcov=$enableval], + [gcov=no]) +AC_SUBST(gcov) + AC_ARG_ENABLE([profile], AC_HELP_STRING([--enable-profile], [build profiled library @<:@default=no@:>@]), diff --git a/manual/install.texi b/manual/install.texi index 2e164476d5..b990118ec6 100644 --- a/manual/install.texi +++ b/manual/install.texi @@ -141,6 +141,21 @@ Don't build shared libraries even if it is possible. Not all systems support shared libraries; you need ELF support and (currently) the GNU linker. +@item --enable-gcov +Enable building glibc with instrumentation for code coverage. For +individual files @option{--coverage} can be used, but this fails +when it is used as an option with @code{configure} command to build glibc. +To successfully build glibc with coverage, you must first build and install glibc +with shared libraries enabled. This will allow some files such as +@file{libc-modules.h} to be generated which are needed for code coverage +to work. +The command @samp{../configure --disable-shared --enable-gcov +--without-selinux --disable-nscd --prefix=/path/to/install} +will build glibc with code coverage. Note that not including a prefix, +or setting the prefix to usr/lib can affect the system glibc and can +render the system unusable. The command to make is 'make CXX=', without +which you will get a error 'cannot find -lgcc_s'. + @item --enable-static-pie Enable static position independent executable (static PIE) support. Static PIE is similar to static executable, but can be loaded at any -- 2.25.1