Bug 10599 - maybe see whether strict aliasing would be a performance improvement
Summary: maybe see whether strict aliasing would be a performance improvement
Status: RESOLVED MOVED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: 1.5
Hardware: All All
: lowest enhancement
Assignee: D-Bus Maintainers
QA Contact: D-Bus Maintainers
URL:
Whiteboard:
Keywords: love
Depends on:
Blocks:
 
Reported: 2007-04-10 12:25 UTC by Pacho Ramos
Modified: 2018-10-12 21:04 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Pacho Ramos 2007-04-10 12:25:59 UTC
When I compile dbus I get:

dbus-launch-x11.c:313: warning: dereferencing type-punned pointer will break strict-aliasing rules
dbus-launch-x11.c:326: warning: dereferencing type-punned pointer will break strict-aliasing rules


Thanks a lot
Comment 1 Havoc Pennington 2007-04-10 12:38:28 UTC
The best fix is probably -fno-strict-aliasing, I don't see a need for silly tricks with union{}.
Comment 2 Stian Skjelstad 2009-02-03 13:52:39 UTC
Hmm, I just tried to compile dbus-launch-x11.c with -Wstrict-aliasing, and I got no warnings:

dbus-1.2.12
$ gcc --version
gcc (Gentoo 4.3.2 p1.0) 4.3.2

Comment 3 Colin Walters 2009-02-03 14:06:07 UTC
I should have followed up to this bug; we use -fno-strict-aliasing by default as of the following commit.  It's still worth investigating at some point whether there are noticeable performance gains in certain parts of the code from making it strict-aliasing compatible, but for now we don't do it.

commit 98495b896dc77493db21b1847f508f64dc6ee7d3
Author: Colin Walters <walters@verbum.org>
Date:   Fri Dec 19 20:02:14 2008 -0500

    Enable -Werror by default with --enable-maintainer-mode, and change warnings
    
    Important compiler warnings were being lost in the noise from warnings
    we know about but aren't problems, and moreover made using -Werror
    difficult.  Now we expect *all* developers and testers to be using
    -Werror.
Comment 4 Stian Skjelstad 2009-02-03 14:10:22 UTC
n@laptop /usr/portage/distfiles/dbus-1.2.12/tools $ i686-pc-linux-gnu-gcc -DHAVE_CONFIG_H -I. -I.. -I..   -DDBUS_LOCALEDIR=\"/usr/share/locale\" -DDBUS_COMPILATION -DDBUS_DAEMONDIR=\"/usr/bin\" -DDBUS_MACHINE_UUID_FILE=\""/var/lib/dbus/machine-id"\"    -ffunction-sections -fdata-sections -g -O2 -Wall -MT dbus-launch-x11.o -MD -MP -MF .deps/dbus-launch-x11.Tpo -c -o dbus-launch-x11.o dbus-launch-x11.c -Wextra -Wstrict-aliasing -fstrict-aliasing
dbus-launch-x11.c: In function 'x_io_error_handler':
dbus-launch-x11.c:44: warning: unused parameter 'xdisplay'
stian@laptop /usr/portage/distfiles/dbus-1.2.12/tools $ 

But I guess there might be aliasing issues in other source-files
Comment 5 Simon McVittie 2015-02-24 11:38:23 UTC
Under clang, we also have to compile with -Wno-error=cast-align to suppress (fatal) warnings about inappropriate casts (Bug #89243).

This is a very long-standing issue: the DBusString code has always used "aliasing" that is not, strictly speaking, Standard C, and compensates for it by second-guessing the compiler and doing the alignment "by hand". I've wanted to replace this with the correct C way to do it (via a pointer to a union) for a while, but have never got round to it.

Alternatively, I might try just replacing the aliased accesses with a memcpy() into a correctly-aligned local variable, and seeing whether it makes any practical difference on benchmarks. My guess is that it won't actually produce a measurable difference.

Under gcc these warnings masked by the fact that the x86 family have essentially no mandatory alignment requirements (unaligned accesses are just slow); building for ARM, where unaligned access silently does the wrong thing, has raised these warnings for years. clang treats the multi-byte integers' preferred alignment as if it was mandatory even on x86, which is a stricter interpretation.
Comment 6 Thiago Macieira 2015-04-03 17:55:39 UTC
The union trick is not standard C either. The only correct way is memcpy.
Comment 7 GitLab Migration User 2018-10-12 21:04:00 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/dbus/dbus/issues/4.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.