fix compilation warnings #1731

flavorjones · 2018-03-04T22:18:41Z

Trying to make compilation produce no meaningful errors. Would love to have a second pair of eyes on this.

remove a few that generate lots of warnings on ruby.h: - -Wcast-qual - -Wconversion add -Wextra back (it was removed in 2008)

to fix compiler warnings. the node->type check is polymorphic, and so a macro makes more sense to me (so we get some compile-time checking) than force-typecasting.

xmlNewCDataBlock() takes ptr and len, so we can use RSTRING_LEN() instead of strlen(). strlen() was introduced in commit c766f6e to fix a segfault, but it's enough to execute StringValuePtr() and RSTRING_LEN() in order to avoid invalid memory access.

larskanis

Warning in rb_scan_args() is ruby intern, we can't control it in our source code. So removing -Wconversion is OK.

For -Wconst-qual I opened #1735 as an alternative approach.

IMHO the rest can't be done better than you did. Great job as usual!

…arnings Flavorjones fix compilation warnings

flavorjones · 2018-03-14T21:17:09Z

I've merged #1735 in here ... everyone OK to merge this into master?

knu · 2018-06-22T07:35:56Z

Just as reported in #1772, I believe this has broken the build where the following conditions are both met:

Using system libraries via NOKOGIRI_USE_SYSTEM_LIBRARIES or --use-system-libraries
pkg-config gem is not available (e.g. during bundle install --deployment with ruby and just the bundler gem installed)

My mkmf.log has the following lines:

| pkg-config --libs libxml-2.0
"gcc -o conftest -I/rbenv/versions/2.5.1/include/ruby-2.5.0/x86_64-linux -I/rbenv/versions/2.5.1/include/ruby-2.5.0/ruby/backward -I/rbenv/versions/2.5.1/include/ruby-2.5.0 -I. -D_FORTIFY_SOURCE=2   -g -O2 -fstack-protector-strong -Wformat -Werror=format-security  -g -DXP_UNIX -Wall -Wcast-qual -Wwrite-strings -Wextra -Wmissing-noreturn -Winline conftest.c  -L. -L/rbenv/versions/2.5.1/lib/x86_64-linux-gnu -Wl,-rpath,/rbenv/versions/2.5.1/lib/x86_64-linux-gnu -L. -Wl,-z,relro -fstack-protector -rdynamic -Wl,-export-dynamic     -Wl,-rpath,/rbenv/versions/2.5.1/lib/x86_64-linux-gnu -L/rbenv/versions/2.5.1/lib/x86_64-linux-gnu -lruby-static -lxml2 -lpthread -lgmp -ldl -lcrypt -lm   -lc "
conftest.c: In function 'main':
conftest.c:3:14: warning: unused parameter 'argc' [-Wunused-parameter]
 int main(int argc, char **argv)
              ^
conftest.c:3:27: warning: unused parameter 'argv' [-Wunused-parameter]
 int main(int argc, char **argv)
                           ^
checked program was:
/* begin */
1: #include "ruby.h"
2:
3: int main(int argc, char **argv)
4: {
5:   return 0;
6: }
/* end */

package configuration for libxml-2.0 is not found

The combination of -Wall -Wextra has led pkg_config() to fail because pkg_config() checks the availability of package libraries using try_ldflags() which invokes the linker with -Werror, and therefore the warnings above are considered errors.

flavorjones · 2018-06-24T11:18:18Z

@knu Can you help me reproduce this? I'm on Linux, and I've tried on Ruby 2.4.1 with and without the pkg-config gem, and cannot reproduce this issue. Can you describe your environment?

flavorjones · 2018-06-24T11:35:31Z

I've also tried to reproduce with Ruby 2.5.1 and I cannot. Here's my mkmf.log snippet:

| pkg-config --libs libxml-2.0
=> "-lxml2\n"
"gcc -o conftest -I/home/flavorjones/.rvm/rubies/ruby-2.5.1/include/ruby-2.5.0/x86_64-linux -I/home/flavorjones/.rvm/rubies/ruby-2.5.1/include/ruby-2.5.0/ruby/backward -I/home/flavorjones/.rvm/rubies/ruby-2.5.1/include/ruby-2.5.0 -I../../../../ext/nokogiri    -O3 -ggdb3 -Wall -Wextra -Wno-unused-parameter -Wno-parentheses -Wno-long-long -Wno-missing-field-initializers -Wno-tautological-compare -Wno-parentheses-equality -Wno-constant-logical-operand -Wno-self-assign -Wunused-variable -Wimplicit-int -Wpointer-arith -Wwrite-strings -Wdeclaration-after-statement -Wimplicit-function-declaration -Wdeprecated-declarations -Wno-packed-bitfield-compat -Wsuggest-attribute=noreturn -Wsuggest-attribute=format -Wno-maybe-uninitialized  -fPIC  -g -DXP_UNIX -O3 -Wall -Wcast-qual -Wwrite-strings -Wextra -Wmissing-noreturn -Winline conftest.c  -L. -L/home/flavorjones/.rvm/rubies/ruby-2.5.1/lib -Wl,-rpath,/home/flavorjones/.rvm/rubies/ruby-2.5.1/lib -L. -fstack-protector -rdynamic -Wl,-export-dynamic     -Wl,-rpath,/home/flavorjones/.rvm/rubies/ruby-2.5.1/lib -L/home/flavorjones/.rvm/rubies/ruby-2.5.1/lib -lruby -lxml2 -lpthread -lgmp -ldl -lcrypt -lm   -lc "
checked program was:
/* begin */
1: #include "ruby.h"
2: 
3: int main(int argc, char **argv)
4: {
5:   return 0;
6: }
/* end */

knu · 2018-06-24T13:22:15Z

Sure, I've minimized my case as uploaded here: https://gist.github.com/knu/2650e7d924e818ad4441dc06017162d1

Place the three files in the same directory and run docker build . for reproduction.

knu · 2018-06-24T13:42:00Z

There must be subtle differences somewhere. On environments where the build fails, it seems the compiler is invoked without warnflags added, which should include -Wno-unused-parameter.

voxik · 2018-07-10T07:53:40Z

Testing on Fedora Rawhide:

$ rpm -q gcc
gcc-8.1.1-4.fc29.1.x86_64

$ rpm -q pkgconf
pkgconf-1.5.1-2.fc29.x86_64

$ rpm -q ruby-devel
ruby-devel-2.5.1-93.fc29.x86_64

$ rpm -q libxml2-devel
libxml2-devel-2.9.8-2.fc29.x86_64

$ NOKOGIRI_USE_SYSTEM_LIBRARIES=yes ruby ext/nokogiri/extconf.extra.rb

... snip ...

$ cat mkmf.log 
"gcc -o conftest -I/usr/include -I/usr/include/ruby/backward -I/usr/include -Iext/nokogiri    -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fPIC  conftest.c  -L. -L/usr/lib64 -L. -Wl,-z,relro  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -fstack-protector -rdynamic -Wl,-export-dynamic  -m64   -lruby    -lc "
checked program was:
/* begin */
1: #include "ruby.h"
2: 
3: int main(int argc, char **argv)
4: {
5:   return 0;
6: }
/* end */

"gcc -I/usr/include -I/usr/include/ruby/backward -I/usr/include -Iext/nokogiri    -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fPIC  -m64  -Werror -c conftest.c"
checked program was:
/* begin */
1: #include "ruby.h"
2: 
3: int main() {return 0;}
/* end */

"pkg-config --exists libxml-2.0"
| pkg-config --libs libxml-2.0
=> "-lxml2 \n"
"gcc -o conftest -I/usr/include -I/usr/include/ruby/backward -I/usr/include -Iext/nokogiri    -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fPIC  -g -DXP_UNIX -Wall -Wcast-qual -Wwrite-strings -Wextra -Wmissing-noreturn -Winline conftest.c  -L. -L/usr/lib64 -L. -Wl,-z,relro  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -fstack-protector -rdynamic -Wl,-export-dynamic  -m64   -lruby -lxml2   -lc "
conftest.c: In function ‘main’:
conftest.c:3:14: warning: unused parameter ‘argc’ [-Wunused-parameter]
 int main(int argc, char **argv)
          ~~~~^~~~
conftest.c:3:27: warning: unused parameter ‘argv’ [-Wunused-parameter]
 int main(int argc, char **argv)
                    ~~~~~~~^~~~
checked program was:
/* begin */
1: #include "ruby.h"
2: 
3: int main(int argc, char **argv)
4: {
5:   return 0;
6: }
/* end */

package configuration for libxml-2.0 is not found
"pkg-config --exists libxslt"
| pkg-config --libs libxslt
=> "-lxslt -lm -lxml2 \n"
"gcc -o conftest -I/usr/include -I/usr/include/ruby/backward -I/usr/include -Iext/nokogiri    -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fPIC  -g -DXP_UNIX -Wall -Wcast-qual -Wwrite-strings -Wextra -Wmissing-noreturn -Winline conftest.c  -L. -L/usr/lib64 -L. -Wl,-z,relro  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -fstack-protector -rdynamic -Wl,-export-dynamic  -m64   -lruby -lxslt -lm -lxml2   -lc "
conftest.c: In function ‘main’:
conftest.c:3:14: warning: unused parameter ‘argc’ [-Wunused-parameter]
 int main(int argc, char **argv)
          ~~~~^~~~
conftest.c:3:27: warning: unused parameter ‘argv’ [-Wunused-parameter]
 int main(int argc, char **argv)
                    ~~~~~~~^~~~
checked program was:
/* begin */
1: #include "ruby.h"
2: 
3: int main(int argc, char **argv)
4: {
5:   return 0;
6: }
/* end */

package configuration for libxslt is not found
"pkg-config --exists libexslt"
| pkg-config --libs libexslt
=> "-lexslt -lxslt -lm -lgcrypt -ldl -lgpg-error -lxml2 \n"
"gcc -o conftest -I/usr/include -I/usr/include/ruby/backward -I/usr/include -Iext/nokogiri    -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fPIC  -g -DXP_UNIX -Wall -Wcast-qual -Wwrite-strings -Wextra -Wmissing-noreturn -Winline conftest.c  -L. -L/usr/lib64 -L. -Wl,-z,relro  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -fstack-protector -rdynamic -Wl,-export-dynamic  -m64   -lruby -lexslt -lxslt -lm -lgcrypt -ldl -lgpg-error -lxml2   -lc "
conftest.c: In function ‘main’:
conftest.c:3:14: warning: unused parameter ‘argc’ [-Wunused-parameter]
 int main(int argc, char **argv)
          ~~~~^~~~
conftest.c:3:27: warning: unused parameter ‘argv’ [-Wunused-parameter]
 int main(int argc, char **argv)
                    ~~~~~~~^~~~
checked program was:
/* begin */
1: #include "ruby.h"
2: 
3: int main(int argc, char **argv)
4: {
5:   return 0;
6: }
/* end */

package configuration for libexslt is not found
"gcc -E -I/usr/include -I/usr/include/ruby/backward -I/usr/include -Iext/nokogiri    -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fPIC  -g -DXP_UNIX -Wall -Wcast-qual -Wwrite-strings -Wextra -Wmissing-noreturn -Winline  conftest.c -o conftest.i"
conftest.c:3:10: fatal error: libxml/xmlversion.h: No such file or directory
 #include <libxml/xmlversion.h>
          ^~~~~~~~~~~~~~~~~~~~~
compilation terminated.
checked program was:
/* begin */
1: #include "ruby.h"
2: 
3: #include <libxml/xmlversion.h>
/* end */

$ NOKOGIRI_USE_SYSTEM_LIBRARIES=yes ruby ext/nokogiri/extconf.conversion.rb

... snip ...

$ cat mkmf.log 
"gcc -o conftest -I/usr/include -I/usr/include/ruby/backward -I/usr/include -Iext/nokogiri    -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fPIC  conftest.c  -L. -L/usr/lib64 -L. -Wl,-z,relro  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -fstack-protector -rdynamic -Wl,-export-dynamic  -m64   -lruby    -lc "
checked program was:
/* begin */
1: #include "ruby.h"
2: 
3: int main(int argc, char **argv)
4: {
5:   return 0;
6: }
/* end */

"gcc -I/usr/include -I/usr/include/ruby/backward -I/usr/include -Iext/nokogiri    -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fPIC  -m64  -Werror -c conftest.c"
checked program was:
/* begin */
1: #include "ruby.h"
2: 
3: int main() {return 0;}
/* end */

"pkg-config --exists libxml-2.0"
| pkg-config --libs libxml-2.0
=> "-lxml2 \n"
"gcc -o conftest -I/usr/include -I/usr/include/ruby/backward -I/usr/include -Iext/nokogiri    -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fPIC  -g -DXP_UNIX -Wall -Wcast-qual -Wwrite-strings -Wconversion -Wmissing-noreturn -Winline conftest.c  -L. -L/usr/lib64 -L. -Wl,-z,relro  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -fstack-protector -rdynamic -Wl,-export-dynamic  -m64   -lruby -lxml2   -lc "
checked program was:
/* begin */
1: #include "ruby.h"
2: 
3: int main(int argc, char **argv)
4: {
5:   return 0;
6: }
/* end */

| pkg-config --cflags-only-I libxml-2.0
=> "-I/usr/include/libxml2 \n"
| pkg-config --cflags-only-other libxml-2.0
=> "\n"
| pkg-config --libs-only-l libxml-2.0
=> "-lxml2 \n"
package configuration for libxml-2.0
cflags: 
ldflags: 
libs: -lxml2

... snip ...

voxik · 2018-07-10T07:55:57Z

The only difference in the check for "libxml-2.0" is the difference in -Wextra vs -Wconversion flags and the subsequent warnings.

johnl · 2018-07-25T13:06:37Z

Broken for me on Ubuntu 16.04 (xenial) and 18.04 (bionic), both long term supported release of Ubuntu.

NOKOGIRI_USE_SYSTEM_LIBRARIES=true bundle exec rake compile:nokogiri
cd tmp/x86_64-linux-gnu/nokogiri/2.3.7
/usr/bin/ruby2.3 -I. ../../../../ext/nokogiri/extconf.rb 
checking if the C compiler accepts ... yes
Building nokogiri using system libraries.
pkg-config could not be used to find libxml-2.0
Please install either `pkg-config` or the pkg-config gem per

    gem install pkg-config -v "~> 1.1"

pkg-config could not be used to find libxslt
Please install either `pkg-config` or the pkg-config gem per

    gem install pkg-config -v "~> 1.1"

pkg-config could not be used to find libexslt
Please install either `pkg-config` or the pkg-config gem per

    gem install pkg-config -v "~> 1.1"

ERROR: cannot discover where libxml2 is located on your system. please make sure `pkg-config` is installed.
rake aborted!

git bisect found 77be60c to be the culprit.

xenial is ruby 2.3 and bionic is ruby 2.5.

This Dockerfile reproduces it, just put it in an empty dir and run docker build .

FROM ubuntu:xenial

RUN apt-get update -q && apt-get install ruby ruby-dev bundler libxslt-dev libxml2-dev pkg-config git -qy
RUN git clone https://github.com/sparklemotion/nokogiri.git
WORKDIR /nokogiri
RUN git checkout v1.8.3
RUN bundler install
ENV NOKOGIRI_USE_SYSTEM_LIBRARIES=true
RUN bundle exec rake compile

due to conflicting gcc CFLAGS, details in #1772 and #1731. added to both master and PR pipelines.

flavorjones · 2018-08-15T14:01:56Z

Please see #1722 for details, but this will be fixed in 1.8.5, due out soon.

flavorjones added 3 commits March 3, 2018 19:45

tweak compiler flags to reduce noise

77be60c

remove a few that generate lots of warnings on ruby.h: - -Wcast-qual - -Wconversion add -Wextra back (it was removed in 2008)

fix some compiler warnings

b0a12b9

replace namespace_eh function with a macro

0b9c828

to fix compiler warnings. the node->type check is polymorphic, and so a macro makes more sense to me (so we get some compile-time checking) than force-typecasting.

flavorjones requested review from tenderlove and larskanis March 4, 2018 22:18

larskanis added 2 commits March 14, 2018 21:52

Re-add gcc option -Wconst-qual

ca22ccb

larskanis approved these changes Mar 14, 2018

View reviewed changes

Merge pull request #1735 from larskanis/flavorjones-fix-compilation-w…

c93fa43

…arnings Flavorjones fix compilation warnings

flavorjones merged commit 31babad into master Mar 14, 2018

flavorjones deleted the flavorjones-fix-compilation-warnings branch March 22, 2018 17:52

mild123 mentioned this pull request Jun 21, 2018

Wunused-parameter introduced in #1731 breaks the "make config" phase on Mingw64/Msys64 #1772

Closed

flavorjones mentioned this pull request Aug 15, 2018

Commit 77be60c3 breaks installation with homebrew system libraries and pkg_config #1780

Closed

flavorjones added a commit that referenced this pull request Aug 15, 2018

add tests for pkg-config failure scenario

91a63d5

due to conflicting gcc CFLAGS, details in #1772 and #1731. added to both master and PR pipelines.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix compilation warnings #1731

fix compilation warnings #1731

flavorjones commented Mar 4, 2018

larskanis left a comment

flavorjones commented Mar 14, 2018

knu commented Jun 22, 2018

flavorjones commented Jun 24, 2018

flavorjones commented Jun 24, 2018

knu commented Jun 24, 2018

knu commented Jun 24, 2018

voxik commented Jul 10, 2018

voxik commented Jul 10, 2018 •

edited

johnl commented Jul 25, 2018

flavorjones commented Aug 15, 2018

fix compilation warnings #1731

fix compilation warnings #1731

Conversation

flavorjones commented Mar 4, 2018

larskanis left a comment

Choose a reason for hiding this comment

flavorjones commented Mar 14, 2018

knu commented Jun 22, 2018

flavorjones commented Jun 24, 2018

flavorjones commented Jun 24, 2018

knu commented Jun 24, 2018

knu commented Jun 24, 2018

voxik commented Jul 10, 2018

voxik commented Jul 10, 2018 • edited

johnl commented Jul 25, 2018

flavorjones commented Aug 15, 2018

voxik commented Jul 10, 2018 •

edited