r-hub / rhub-linux-builders

Docker configuration for the R-hub Linux builders
GNU General Public License v2.0
23 stars 13 forks source link

debian-gcc-devel should have CFLAGS = -O2 #38

Open wch opened 5 years ago

wch commented 5 years ago

On CRAN's r-devel-linux-x86_64-debian-gcc build machine, they use -O2, which results in some significant compiler warnings that don't occur without -O2. However, r-hub's debian-gcc-devel image doesn't use -O2. This was an issue when we submitted the sass package at commit rstudio/sass@609944079951ea757d82c047ad9941746f758cfe.

It received the significant warning (which resulted in the package getting rejected):

* installing *source* package 'sass' ...
** using staged installation
** libs
/home/hornik/tmp/R/share/make/shlib.mk:14: warning: overriding recipe for target 'shlib-clean'
Makevars:12: warning: ignoring old recipe for target 'shlib-clean'
gcc-8  -I"/home/hornik/tmp/R/include" -DNDEBUG -I./libsass/include  -I/usr/local/include  -fpic  -g -O2 -Wall -pedantic -mtune=native -c compile.c -o compile.o
gcc-8  -I"/home/hornik/tmp/R/include" -DNDEBUG -I./libsass/include  -I/usr/local/include  -fpic  -g -O2 -Wall -pedantic -mtune=native -c create_string.c -o create_string.o
create_string.c: In function 'create_string':
create_string.c:9:3: warning: 'strncpy' specified bound depends on the length of the source argument [-Wstringop-overflow=]
   strncpy(str, value, n);
   ^~~~~~~~~~~~~~~~~~~~~~
create_string.c:7:11: note: length computed here
   int n = strlen(value) + 1;
           ^~~~~~~~~~~~~
gcc-8  -I"/home/hornik/tmp/R/include" -DNDEBUG -I./libsass/include  -I/usr/local/include  -fpic  -g -O2 -Wall -pedantic -mtune=native -c init.c -o init.o
MAKEFLAGS= CC="gcc-8 " CFLAGS="-g -O2 -Wall -pedantic -mtune=native" CXX="g++-8  -std=gnu++11" AR="ar" LDFLAGS="-Wl,-O1" make -C libsass
make[1]: Entering directory '/srv/hornik/tmp/CRAN/sass.Rcheck/00_pkg_src/sass/src/libsass'

The doesn't occur on rhub's debian-gcc-devel Docker image. However, when I modified /opt/R-devel/lib/R/etc/Makeconf so that the CFLAGS also included -O2, I did see the warning.

You can see the difference with -O2 more easily by running the following from the command line:

docker run --rm -ti rhub/debian-gcc-devel

echo -e "
#include <stdlib.h>
#include <string.h>

char* create_string(const char* value) {
  // strlen doesn't count null terminator
  int n = strlen(value) + 1;
  char* str = (char *) malloc(n);
  strncpy(str, value, n);
  return str;
}" > create_string.c

gcc -O2 -Wall -c create_string.c -o create_string.o

This will result in:

root@45c98a6c18dc:/# gcc -O2 -Wall -c create_string.c -o create_string.o
create_string.c: In function ‘create_string’:
create_string.c:9:3: warning: ‘strncpy’ specified bound depends on the length of the source argument [-Wstringop-overflow=]
   strncpy(str, value, n);
   ^~~~~~~~~~~~~~~~~~~~~~
create_string.c:7:11: note: length computed here
   int n = strlen(value) + 1;
           ^~~~~~~~~~~~~

However, if you run the same compiler command without -O2, there's no warning.

root@45c98a6c18dc:/# gcc  -Wall -c create_string.c -o create_string.o
root@45c98a6c18dc:/# 

(Note that I was unable to reproduce this warning on ubuntu:disco, which has gcc (Ubuntu 8.3.0-3ubuntu1) 8.3.0; debian:testing has gcc (Debian 8.3.0-2) 8.3.0.

So I think this means that r-hub's Docker images should have -O2 in CFLAGS, and probably in CXXFLAGS. There are other options which are different between the rhub image and CRAN. It's probably a good idea to make them the same as CRAN. For example:

cc: @rich-iannone, @jcheng5