Fail to build with GCC-10

[mizuno-as]

I installed GCC-10 from experimental in Debian sid environment. In this environment, the build has failed with linker error. The cause is that the -fno-common option is the default in GCC-10. The workaround is to add CFLAGS='-fcommon' to configure.

FYI: http://gcc.gnu.org/gcc-10/porting_to.html

How to reproduce.

$ sudo apt install -t=experimental g++
$ gcc --version
gcc (Debian 10-20200418-1) 10.0.1 20200418 (experimental) [master revision 27c171775ab:4c277008be0:c5bac7d127f288fd2f8a1f15c3f30da5903141c6]

$ git clone https://github.com/ggreer/the_silver_searcher.git
$ cd the_silver_searcher/

$ ./build.sh
configure.ac:10: installing './compile'
configure.ac:22: installing './config.guess'
configure.ac:22: installing './config.sub'
configure.ac:8: installing './install-sh'
configure.ac:8: installing './missing'
Makefile.am: installing './depcomp'
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... no
checking for mawk... mawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables... 
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking whether gcc understands -c and -o together... yes
checking whether make supports the include directive... yes (GNU style)
checking dependency style of gcc... gcc3
checking for grep that handles long lines and -e... /bin/grep
checking whether make supports nested variables... (cached) yes
checking for pkg-config... /usr/bin/pkg-config
checking pkg-config is at least version 0.9.0... yes
checking for libpcre... yes
checking build system type... x86_64-pc-linux-gnu
checking host system type... x86_64-pc-linux-gnu
checking how to run the C preprocessor... gcc -E
checking for egrep... /bin/grep -E
checking for ANSI C header files... yes
checking for sys/types.h... yes
checking for sys/stat.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for memory.h... yes
checking for strings.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for unistd.h... yes
checking if compiler needs -Werror to reject unknown flags... no
checking for the pthreads library -lpthreads... no
checking whether pthreads work without any flags... no
checking whether pthreads work with -Kthread... no
checking whether pthreads work with -kthread... no
checking for the pthreads library -llthread... no
checking whether pthreads work with -pthread... yes
checking for joinable pthread attribute... PTHREAD_CREATE_JOINABLE
checking if more special flags are required for pthreads... no
checking for PTHREAD_PRIO_INHERIT... yes
checking pthread.h usability... yes
checking pthread.h presence... yes
checking for pthread.h... yes
checking zlib.h usability... yes
checking zlib.h presence... yes
checking for zlib.h... yes
checking for library containing inflate... -lz
checking lzma.h usability... yes
checking lzma.h presence... yes
checking for lzma.h... yes
checking for liblzma... yes
checking whether PCRE_CONFIG_JIT is declared... yes
checking whether CPU_ZERO, CPU_SET is declared... yes
checking sys/cpuset.h usability... no
checking sys/cpuset.h presence... no
checking for sys/cpuset.h... no
checking err.h usability... yes
checking err.h presence... yes
checking for err.h... yes
checking for struct dirent.d_type... yes
checking for struct dirent.d_namlen... no
checking for fgetln... no
checking for fopencookie... yes
checking for getline... yes
checking for realpath... yes
checking for strlcpy... no
checking for strndup... yes
checking for vasprintf... yes
checking for madvise... yes
checking for posix_fadvise... yes
checking for pthread_setaffinity_np... yes
checking for pledge... no
checking for clang-format-3.8... no
checking for clang-format-3.7... no
checking for clang-format-3.6... no
checking for clang-format... no
configure: WARNING: clang-format not found. 'make test' will not detect improperly-formatted files.
checking that generated files are newer than configure... done
configure: creating ./config.status
config.status: creating Makefile
config.status: creating the_silver_searcher.spec
config.status: creating src/config.h
config.status: executing depfiles commands
  CC       src/ignore.o
  CC       src/log.o
  CC       src/options.o
  CC       src/print.o
  CC       src/print_w32.o
  CC       src/scandir.o
  CC       src/search.o
  CC       src/lang.o
  CC       src/util.o
src/lang.c: In function 'combine_file_extensions':
src/lang.c:198:13: warning: 'strncpy' output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]
  198 |             strncpy(pos, ext, strlen(ext));
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  CC       src/decompress.o
src/decompress.c: In function 'decompress_zlib':
src/decompress.c:52:22: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
   52 |     stream.next_in = (Bytef *)buf;
      |                      ^
  CC       src/main.o
  CC       src/zfile.o
  CCLD     ag
/usr/bin/ld: src/log.o:/the_silver_searcher/src/log.h:12: multiple definition of `print_mtx'; src/ignore.o:/the_silver_searcher/src/log.h:12: first defined here
/usr/bin/ld: src/log.o:/the_silver_searcher/src/util.h:15: multiple definition of `out_fd'; src/ignore.o:/the_silver_searcher/src/util.h:15: first defined here
/usr/bin/ld: src/log.o:/the_silver_searcher/src/util.h:54: multiple definition of `stats'; src/ignore.o:/the_silver_searcher/src/util.h:54: first defined here
/usr/bin/ld: src/log.o:/the_silver_searcher/src/options.h:95: multiple definition of `opts'; src/ignore.o:/the_silver_searcher/src/options.h:95: first defined here
/usr/bin/ld: src/options.o:/the_silver_searcher/src/options.h:95: multiple definition of `opts'; src/ignore.o:/the_silver_searcher/src/options.h:95: first defined here
/usr/bin/ld: src/options.o:/the_silver_searcher/src/ignore.h:32: multiple definition of `root_ignores'; src/ignore.o:/the_silver_searcher/src/ignore.h:32: first defined here
/usr/bin/ld: src/options.o:/the_silver_searcher/src/util.h:15: multiple definition of `out_fd'; src/ignore.o:/the_silver_searcher/src/util.h:15: first defined here
/usr/bin/ld: src/options.o:/the_silver_searcher/src/util.h:54: multiple definition of `stats'; src/ignore.o:/the_silver_searcher/src/util.h:54: first defined here
/usr/bin/ld: src/options.o:/the_silver_searcher/src/log.h:12: multiple definition of `print_mtx'; src/ignore.o:/the_silver_searcher/src/log.h:12: first defined here
/usr/bin/ld: src/print.o:/the_silver_searcher/src/options.h:95: multiple definition of `opts'; src/ignore.o:/the_silver_searcher/src/options.h:95: first defined here
/usr/bin/ld: src/print.o:/the_silver_searcher/src/util.h:15: multiple definition of `out_fd'; src/ignore.o:/the_silver_searcher/src/util.h:15: first defined here
/usr/bin/ld: src/print.o:/the_silver_searcher/src/util.h:54: multiple definition of `stats'; src/ignore.o:/the_silver_searcher/src/util.h:54: first defined here
/usr/bin/ld: src/print.o:/the_silver_searcher/src/log.h:12: multiple definition of `print_mtx'; src/ignore.o:/the_silver_searcher/src/log.h:12: first defined here
/usr/bin/ld: src/print.o:/the_silver_searcher/src/ignore.h:32: multiple definition of `root_ignores'; src/ignore.o:/the_silver_searcher/src/ignore.h:32: first defined here
/usr/bin/ld: src/scandir.o:/the_silver_searcher/src/util.h:54: multiple definition of `stats'; src/ignore.o:/the_silver_searcher/src/util.h:54: first defined here
/usr/bin/ld: src/scandir.o:/the_silver_searcher/src/util.h:15: multiple definition of `out_fd'; src/ignore.o:/the_silver_searcher/src/util.h:15: first defined here
/usr/bin/ld: src/scandir.o:/the_silver_searcher/src/options.h:95: multiple definition of `opts'; src/ignore.o:/the_silver_searcher/src/options.h:95: first defined here
/usr/bin/ld: src/scandir.o:/the_silver_searcher/src/log.h:12: multiple definition of `print_mtx'; src/ignore.o:/the_silver_searcher/src/log.h:12: first defined here
/usr/bin/ld: src/scandir.o:/the_silver_searcher/src/ignore.h:32: multiple definition of `root_ignores'; src/ignore.o:/the_silver_searcher/src/ignore.h:32: first defined here
/usr/bin/ld: src/search.o:/the_silver_searcher/src/options.h:95: multiple definition of `opts'; src/ignore.o:/the_silver_searcher/src/options.h:95: first defined here
/usr/bin/ld: src/search.o:/the_silver_searcher/src/search.h:36: multiple definition of `h_table'; src/print.o:/the_silver_searcher/src/search.h:36: first defined here
/usr/bin/ld: src/search.o:/the_silver_searcher/src/search.h:35: multiple definition of `find_skip_lookup'; src/print.o:/the_silver_searcher/src/search.h:35: first defined here
/usr/bin/ld: src/search.o:/the_silver_searcher/src/search.h:34: multiple definition of `alpha_skip_lookup'; src/print.o:/the_silver_searcher/src/search.h:34: first defined here
/usr/bin/ld: src/search.o:/the_silver_searcher/src/log.h:12: multiple definition of `print_mtx'; src/ignore.o:/the_silver_searcher/src/log.h:12: first defined here
/usr/bin/ld: src/search.o:/the_silver_searcher/src/search.h:48: multiple definition of `stats_mtx'; src/print.o:/the_silver_searcher/src/search.h:48: first defined here
/usr/bin/ld: src/search.o:/the_silver_searcher/src/util.h:54: multiple definition of `stats'; src/ignore.o:/the_silver_searcher/src/util.h:54: first defined here
/usr/bin/ld: src/search.o:/the_silver_searcher/src/util.h:15: multiple definition of `out_fd'; src/ignore.o:/the_silver_searcher/src/util.h:15: first defined here
/usr/bin/ld: src/search.o:/the_silver_searcher/src/search.h:49: multiple definition of `work_queue_mtx'; src/print.o:/the_silver_searcher/src/search.h:49: first defined here
/usr/bin/ld: src/search.o:/the_silver_searcher/src/search.h:44: multiple definition of `work_queue'; src/print.o:/the_silver_searcher/src/search.h:44: first defined here
/usr/bin/ld: src/search.o:/the_silver_searcher/src/search.h:47: multiple definition of `files_ready'; src/print.o:/the_silver_searcher/src/search.h:47: first defined here
/usr/bin/ld: src/search.o:/the_silver_searcher/src/search.h:46: multiple definition of `done_adding_files'; src/print.o:/the_silver_searcher/src/search.h:46: first defined here
/usr/bin/ld: src/search.o:/the_silver_searcher/src/search.h:45: multiple definition of `work_queue_tail'; src/print.o:/the_silver_searcher/src/search.h:45: first defined here
/usr/bin/ld: src/search.o:/the_silver_searcher/src/search.h:67: multiple definition of `symhash'; src/print.o:/the_silver_searcher/src/search.h:67: first defined here
/usr/bin/ld: src/search.o:/the_silver_searcher/src/ignore.h:32: multiple definition of `root_ignores'; src/ignore.o:/the_silver_searcher/src/ignore.h:32: first defined here
/usr/bin/ld: src/lang.o:/the_silver_searcher/src/util.h:54: multiple definition of `stats'; src/ignore.o:/the_silver_searcher/src/util.h:54: first defined here
/usr/bin/ld: src/lang.o:/the_silver_searcher/src/util.h:15: multiple definition of `out_fd'; src/ignore.o:/the_silver_searcher/src/util.h:15: first defined here
/usr/bin/ld: src/lang.o:/the_silver_searcher/src/options.h:95: multiple definition of `opts'; src/ignore.o:/the_silver_searcher/src/options.h:95: first defined here
/usr/bin/ld: src/lang.o:/the_silver_searcher/src/log.h:12: multiple definition of `print_mtx'; src/ignore.o:/the_silver_searcher/src/log.h:12: first defined here
/usr/bin/ld: src/util.o:/the_silver_searcher/src/util.h:54: multiple definition of `stats'; src/ignore.o:/the_silver_searcher/src/util.h:54: first defined here
/usr/bin/ld: src/util.o:/the_silver_searcher/src/util.h:15: multiple definition of `out_fd'; src/ignore.o:/the_silver_searcher/src/util.h:15: first defined here
/usr/bin/ld: src/util.o:/the_silver_searcher/src/options.h:95: multiple definition of `opts'; src/ignore.o:/the_silver_searcher/src/options.h:95: first defined here
/usr/bin/ld: src/util.o:/the_silver_searcher/src/log.h:12: multiple definition of `print_mtx'; src/ignore.o:/the_silver_searcher/src/log.h:12: first defined here
/usr/bin/ld: src/decompress.o:/the_silver_searcher/src/options.h:95: multiple definition of `opts'; src/ignore.o:/the_silver_searcher/src/options.h:95: first defined here
/usr/bin/ld: src/decompress.o:/the_silver_searcher/src/log.h:12: multiple definition of `print_mtx'; src/ignore.o:/the_silver_searcher/src/log.h:12: first defined here
/usr/bin/ld: src/main.o:/the_silver_searcher/src/search.h:44: multiple definition of `work_queue'; src/print.o:/the_silver_searcher/src/search.h:44: first defined here
/usr/bin/ld: src/main.o:/the_silver_searcher/src/search.h:45: multiple definition of `work_queue_tail'; src/print.o:/the_silver_searcher/src/search.h:45: first defined here
/usr/bin/ld: src/main.o:/the_silver_searcher/src/ignore.h:32: multiple definition of `root_ignores'; src/ignore.o:/the_silver_searcher/src/ignore.h:32: first defined here
/usr/bin/ld: src/main.o:/the_silver_searcher/src/util.h:15: multiple definition of `out_fd'; src/ignore.o:/the_silver_searcher/src/util.h:15: first defined here
/usr/bin/ld: src/main.o:/the_silver_searcher/src/options.h:95: multiple definition of `opts'; src/ignore.o:/the_silver_searcher/src/options.h:95: first defined here
/usr/bin/ld: src/main.o:/the_silver_searcher/src/search.h:46: multiple definition of `done_adding_files'; src/print.o:/the_silver_searcher/src/search.h:46: first defined here
/usr/bin/ld: src/main.o:/the_silver_searcher/src/search.h:47: multiple definition of `files_ready'; src/print.o:/the_silver_searcher/src/search.h:47: first defined here
/usr/bin/ld: src/main.o:/the_silver_searcher/src/log.h:12: multiple definition of `print_mtx'; src/ignore.o:/the_silver_searcher/src/log.h:12: first defined here
/usr/bin/ld: src/main.o:/the_silver_searcher/src/search.h:49: multiple definition of `work_queue_mtx'; src/print.o:/the_silver_searcher/src/search.h:49: first defined here
/usr/bin/ld: src/main.o:/the_silver_searcher/src/search.h:67: multiple definition of `symhash'; src/print.o:/the_silver_searcher/src/search.h:67: first defined here
/usr/bin/ld: src/main.o:/the_silver_searcher/src/search.h:34: multiple definition of `alpha_skip_lookup'; src/print.o:/the_silver_searcher/src/search.h:34: first defined here
/usr/bin/ld: src/main.o:/the_silver_searcher/src/search.h:35: multiple definition of `find_skip_lookup'; src/print.o:/the_silver_searcher/src/search.h:35: first defined here
/usr/bin/ld: src/main.o:/the_silver_searcher/src/search.h:36: multiple definition of `h_table'; src/print.o:/the_silver_searcher/src/search.h:36: first defined here
/usr/bin/ld: src/main.o:/the_silver_searcher/src/util.h:54: multiple definition of `stats'; src/ignore.o:/the_silver_searcher/src/util.h:54: first defined here
/usr/bin/ld: src/main.o:/the_silver_searcher/src/search.h:48: multiple definition of `stats_mtx'; src/print.o:/the_silver_searcher/src/search.h:48: first defined here
/usr/bin/ld: src/zfile.o:/the_silver_searcher/src/options.h:95: multiple definition of `opts'; src/ignore.o:/the_silver_searcher/src/options.h:95: first defined here
/usr/bin/ld: src/zfile.o:/the_silver_searcher/src/log.h:12: multiple definition of `print_mtx'; src/ignore.o:/the_silver_searcher/src/log.h:12: first defined here
collect2: error: ld returned 1 exit status
make: *** [Makefile:478: ag] Error 1

[dev-0x7C6]

I've same issue on my Gentoo box after switching to GCC 10.1.

[rishitc]

Can I try working on the first 2 warnings shown in the build logs? The rest of the errors/warnings regarding the multiple definitions seem to be due to the inclusion of the header file(s) multiple times, in different files, this could be fixed by using preprocessor directives.

Looking forward to working on the issue.

Thanks

[aswild]

The multiple symbol definition issues are fixed by https://github.com/ggreer/the_silver_searcher/pull/1377 (21eaa1c4160b868b0c5bbf59da17974429f30055)

[sten0]

@aswild, thank you for cross-referencing these issues :-) @rishitc, how is your work progressing? The Silver Searcher has been scheduled for autoremoval (Bug #957798) in Debian as a result of this issue. Is it still the case that these need to be fixed?:

src/lang.c: In function 'combine_file_extensions':
src/lang.c:198:13: warning: 'strncpy' output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]
  198 |             strncpy(pos, ext, strlen(ext));
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  CC       src/decompress.o
src/decompress.c: In function 'decompress_zlib':
src/decompress.c:52:22: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
   52 |     stream.next_in = (Bytef *)buf;

(some of my packages depend on silversearcher-ag and will also be removed). CCing @mizuno-as

[rishitc]

Hi, I have been working on the issue in the background. @sten0 The warning you mentioned is still present while running the ./build.sh command. I tried building it on GCC version 9.3.0 (Ubuntu 9.3.0-10ubuntu2) and it still gives the same warnings, it doesn't stop the compilation though as -Werror flag does not seem to be set.

I am still working on fixing the warning as a clean build output is a priority.

[mizuno-as]

@sten0 I'm going to add a CFLAGS to the silversearcher-ag package to get around it. but I'd like to see a correction on upstream, if possible.

[aswild]

I opened https://github.com/ggreer/the_silver_searcher/pull/1398 to all compile warnings seen on Arch Linux with GCC 10 (-Wstringop-truncation in lang.c and -Wcast-qual in decompress.c)

I don't have a Debian box with GCC10, but the warnings on Arch were the same as in https://github.com/ggreer/the_silver_searcher/issues/1378#issuecomment-665253277

[rishitc]

I think the fix made by @aswild in #1398, is correct for the warning:

src/decompress.c: In function 'decompress_zlib':
src/decompress.c:52:22: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
   52 |     stream.next_in = (Bytef *)buf;

At first, I thought that const needed to be removed for the variable buf as the link that the original author had referred to in a comment, in the decompress.c file, uses a variable exactly like buf called in, which has the type: unsigned char (unsigned is by default anyway) but it is not marked const in that code.

Here is original comment form the decompress.c file, I am referring to:

/* Code in decompress_zlib from
 *
 * https://raw.github.com/madler/zlib/master/examples/zpipe.c
 *
 * zpipe.c: example of proper use of zlib's inflate() and deflate()
 *    Not copyrighted -- provided to the public domain
 *    Version 1.4  11 December 2005  Mark Adler 
 */

---

Not sure if the correction made by @aswild is correct for the other warning:

src/lang.c: In function 'combine_file_extensions':
src/lang.c:198:13: warning: 'strncpy' output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]
  198 |             strncpy(pos, ext, strlen(ext));
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  CC       src/decompress.o

As replacing strncpy with memcpy is not a recommended practice as strncpy checks for the NULL terminating character while copying, so it is safer as compared to using memcpy. Also, strncpy pads the remaining/extra memory block in the destination with NULL characters. Also, as you're using memcpy, buffer overflows are still possible and now might be hidden from the compile-time checks. I would recommend you try running Valgrind to verify if there are any memory errors/buffer overflows.

[aswild]

FWIW, the strncpy->memcpy change makes the code behave identically as it does now (since the *exts array is zero-initialized), but I agree it isn't ideal. strncpy where n==strlen(src) is just as susceptible to buffer overflows in dest as memcpy. I ran the test suite against AddressSanitizer (compile with -fsanitize=address) and didn't get any overflows flagged, just a handful of inconsequential leaks.

In my personal fork I refactored this whole area to add features (like matching full filenames e.g. Makefile or CMakeLists.txt), but that deep of refactoring is probably beyond the scope of fixing compile warnings here.

I had thought that the SINGLE_EXT_LEN math would work out such that there couldn't be overflow, but I see now that's wrong - there's no actual checking that the size of any extension in langs is within SINGLE_EXT_LEN.

What we really need to do is some math to calculate the remaining space in exts and use that as the max length of the copy. I'll update my PR accordingly.

[aswild]

I got inspired to write some nontrivial C tonight, so I refactored the entirety of make_lang_regex and combine_file_extensions (removing the latter). This should remove the potential overflow vulnerabilities, and IMO makes the code easier to follow, but the UX remains the same.

@rishitc @ggreer I'd appreciate any review comments if/when you get a chance.

[sten0]

Thanks @mizuno-as :-) I'm relieved to hear you have a contingency plan in place. 27 days to autoremoval of about nine packages.

Will anyone have time to review & merge aswild's work before then?

[cassioneri]

I'm trying to build with GCC 11 and, not surprisingly, I'm getting the issues above and many more like

warning: implicit declaration of function 'vasprintf'
error: unknown type name 'cookie_read_function_t'
undefined reference to symbol 'pthread_create'

I'd love to see these issues being fixed and released. In the mean time, I found is this workaround:

make CFLAGS="-fcommon -D_GNU_SOURCE -lpthread"

[chr0n1x]

@cassioneri - those CFLAGS worked for me on alpine 3.13. Thank you!

[thebaptiste]

Do you plan a new release fixing all this ?