Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.

Close

#294 GCC complains of strict-aliasing breakage

open
nobody
5
2012-11-24
2012-11-24
Steve Snyder
No

At the standard -O2 optimization GCC v4.4.6 complains of broken strict-aliasing (see below) when building libevent v2.0.21 on a CentOS5 / i686 system.

The warnings can be silenced by compiling with switch -fno-strict-aliasing, but at the cost of disabling this optimization for the entire libevent code base. That seems pretty extreme just for a couple instances of complaint.

Best case would be to fix the code being complained about. Second-best case would be to apply the -fno-strict-aliasing switch to the compilation of the only 2 source files being complained about.

This is the source of the complaints:

common declaration: struct sockaddr_storage ss;
evdns.c: if (evutil_parse_sockaddr_port(addr, (struct sockaddr*)&ss, &socklen)<0)
regress_testutils.c: if (getsockname(fd, (struct sockaddr*)&ss, &socklen) != 0)

Basically, GCC doesn't like passing a pointer to 1 struct as a pointer of a different struct.

-----------------------

libtool: compile: gcc44 -DHAVE_CONFIG_H -I. -I./compat -I./include -I./include -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=native -mno-avx -fomit-frame-pointer -fPIC -MT evdns.lo -MD -MP -MF .deps/evdns.Tpo -c evdns.c -o evdns.o
evdns.c: In function 'evdns_base_parse_hosts_line':
evdns.c:2567: warning: dereferencing pointer 'ss.461' does break strict-aliasing rules
evdns.c:2565: warning: dereferencing pointer 'ss.461' does break strict-aliasing rules
evdns.c:4037: note: initialized from here
evdns.c:2566: warning: dereferencing pointer 'sa.301' does break strict-aliasing rules
evdns.c:2566: note: initialized from here
evdns.c:2568: warning: dereferencing pointer 'sa.302' does break strict-aliasing rules
evdns.c:2568: note: initialized from here
evdns.c: In function 'evdns_base_nameserver_ip_add':
evdns.c:2557: warning: dereferencing pointer 'sa' does break strict-aliasing rules
evdns.c:2555: warning: dereferencing pointer 'sa' does break strict-aliasing rules
evdns.c:2567: warning: dereferencing pointer 'sa' does break strict-aliasing rules
evdns.c:2565: warning: dereferencing pointer 'sa' does break strict-aliasing rules
evdns.c:2587: note: initialized from here
evdns.c:2566: warning: dereferencing pointer 'sa.301' does break strict-aliasing rules
evdns.c:2566: note: initialized from here
evdns.c:2568: warning: dereferencing pointer 'sa.302' does break strict-aliasing rules
evdns.c:2568: note: initialized from here
evdns.c:2556: warning: dereferencing pointer 'sa.299' does break strict-aliasing rules
evdns.c:2556: note: initialized from here
evdns.c:2558: warning: dereferencing pointer 'sa.300' does break strict-aliasing rules
evdns.c:2558: note: initialized from here

gcc44 -DHAVE_CONFIG_H -I. -I.. -I.. -I../compat -I../include -I../include -DTINYTEST_LOCAL -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=native -mno-avx -fomit-frame-pointer -fPIC -MT regress-regress_testutils.o -MD -MP -MF .deps/regress-regress_testutils.Tpo -c -o regress-regress_testutils.o `test -f 'regress_testutils.c' || echo './'`regress_testutils.c
regress_testutils.c: In function 'regress_get_socket_port':
regress_testutils.c:86: warning: dereferencing pointer 'ss.22' does break strict-aliasing rules
regress_testutils.c:86: note: initialized from here
regress_testutils.c:88: warning: dereferencing pointer 'ss.23' does break strict-aliasing rules
regress_testutils.c:88: note: initialized from here

gcc44 -DHAVE_CONFIG_H -I. -I.. -I.. -I../compat -I../include -I../include -DTINYTEST_LOCAL -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=native -mno-avx -fomit-frame-pointer -fPIC -MT regress-regress_listener.o -MD -MP -MF .deps/regress-regress_listener.Tpo -c -o regress-regress_listener.o `test -f 'regress_listener.c' || echo './'`regress_listener.c
regress_listener.c: In function 'regress_pick_a_port':
regress_listener.c:112: warning: dereferencing pointer 'sin1' does break strict-aliasing rules
regress_listener.c:110: warning: dereferencing pointer 'sin1' does break strict-aliasing rules
regress_listener.c:108: note: initialized from here
regress_listener.c:112: warning: dereferencing pointer 'sin2' does break strict-aliasing rules
regress_listener.c:111: warning: dereferencing pointer 'sin2' does break strict-aliasing rules
regress_listener.c:109: note: initialized from here

Discussion

  • Nick Mathewson
    Nick Mathewson
    2012-11-24

    I wouldn't mind a patch to fix these warnings, if you feel up to writing one. For now, libevent's build process ought to be adding "-fno-strict-aliasing" by default; please let me know if that isn't true for you. I'm not sure how one is supposed to use the sockaddr types correctly in the presence of strict aliasing rules, but I'd be glad to see an example.

    Do you have data to show that disabling "-fno-strict-aliasing" will improve libevent performance, or is this more of an "every optimization helps" kind of thing?

     
  • Steve Snyder
    Steve Snyder
    2012-11-24

    I wouldn't be comfortable submitting a patch because I couldn't test it adequately. The only source file that I was really concerned about is evdns.c, as that is runtime code rather than a sample application. After submitting this bug report I discovered that the library containing the evdns object module, libevent_exatras, is not actually used by my intended application - Tor. So if (and I write "if" because I don't know this to be true) there is bad code generation I would never see it.

    And, yes, my reluctance to apply -fno-strict-aliasing universally stems from a more-optimization-is-better mindset than any measured reduction in performance.

    Thanks for the quick response.

     
  • Nick Mathewson
    Nick Mathewson
    2012-11-24

    Look more closely: Tor links -levent, which includes both libevent_core and libevent_extras. Tor will indeed use evdns.

     
  • Steve Snyder
    Steve Snyder
    2012-11-24

    Output of libevent build with GCC 4.4.6 / i686

     
    Attachments
  • Steve Snyder
    Steve Snyder
    2012-11-24

    Ah, I see. The libevent_extras lib doesn't appear on the Tor link command line, but symbols from evdns are in the Tor executable. I guess libevent_extras is pulled in to reconcile symbols from the libevent and/or libevent_openssl libraries.

    > For now, libevent's build process ought to be adding "-fno-strict-aliasing" by default; please let me know if that isn't true for you.

    It is not true. See the attached file "libevent.out" for a record of the libevent build process. Maybe you're thinking of Tor, which does use the "-fno-strict-aliasing" by default? (A comment in the Tor configure script suggests that somebody's been burned by this optimization before.)

     
  • Nick Mathewson
    Nick Mathewson
    2012-11-28

    > It is not true. See the attached file "libevent.out" for a record of the
    libevent build process

    Strange. It does include -fno-strict-aliasing for me. e.g.:

    libtool: compile: gcc -DHAVE_CONFIG_H -I. -I./compat -I./include -I./include -g -O2 -Wall -fno-strict-aliasing -Wno-deprecated-declarations -D_THREAD_SAFE -W -Wfloat-equal -Wundef -Wpointer-arith -Wstrict-prototypes -Wmissing-prototypes -Wwrite-strings -Wredundant-decls -Wchar-subscripts -Wcomment -Wformat -Wwrite-strings -Wmissing-declarations -Wredundant-decls -Wnested-externs -Wbad-function-cast -Wswitch-enum -Wno-unused-parameter -Wstrict-aliasing -Winit-self -Wmissing-field-initializers -Wdeclaration-after-statement -Waddress -Wnormalized=id -Woverride-init -MT buffer.lo -MD -MP -MF .deps/buffer.Tpo -c buffer.c -fno-common -DPIC -o .libs/buffer.o

    The code to add it is in configure.ac (configure.in for Libevent 2.0):

    AC_COMPILE_IFELSE([AC_LANG_PROGRAM([], [
    #if !defined(__GNUC__) || (__GNUC__ < 2) || (__GNUC__ == 2 && __GNUC_MINOR__ < 95)
    #error
    #endif])], have_gcc295=yes, have_gcc295=no)

    if test "$GCC" = "yes" ; then
    # Enable many gcc warnings by default...
    CFLAGS="$CFLAGS -Wall"
    # And disable the strict-aliasing optimization, since it breaks
    # our sockaddr-handling code in strange ways.
    if test x$have_gcc295 = xyes; then
    CFLAGS="$CFLAGS -fno-strict-aliasing"
    fi
    fi

    I wonder why it isn't working for you. Any insight there?

     
  • Steve Snyder
    Steve Snyder
    2012-11-28

    > I wonder why it isn't working for you. Any insight there?

    I looked at it and found that the configure script actually does add -fno-strict-aliasing to the configured CFLAGS. But then CFLAGS is redefined on the make command line in the spec file (I'm building a Red Hat RPM) to the standard RPM build flags.

    I've added -fno-strict-aliasing to those RPM flags to conform to what the configure script thinks is required.