After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 316221 - G_LOCK warns about breaking strict-aliasing
G_LOCK warns about breaking strict-aliasing
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gthread
2.16.x
Other Windows
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
: 507134 527328 529742 531067 564935 584670 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2005-09-13 16:02 UTC by Michal Benes
Modified: 2011-06-04 04:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
glib-strict-aliasing-macros.patch (815 bytes, patch)
2005-11-11 13:51 UTC, Stanislav Brabec
none Details | Review
glib-strict-aliasing-macros.patch (936 bytes, patch)
2005-11-14 16:13 UTC, Stanislav Brabec
none Details | Review
glib-strict-aliasing-macros.patch (943 bytes, patch)
2005-11-14 16:15 UTC, Stanislav Brabec
none Details | Review
aliasing.diff (1.43 KB, patch)
2008-03-08 13:14 UTC, Sebastian Dröge (slomo)
rejected Details | Review

Description Michal Benes 2005-09-13 16:02:15 UTC
When using G_LOCK with -Wall -O2 on 
gcc version 3.4.4 (mingw special)
I get the following warning:
warning: dereferencing type-punned pointer will break strict-aliasing rules

the test program is
#include <glib.h>

G_LOCK_DEFINE_STATIC (mutex);

int main(void) {
  G_LOCK (mutex);
  G_UNLOCK (mutex);
  return 0;
}

I compile this with
gcc -Wall `pkg-config --cflags glib-2.0` -O3 -c test.c
in MinGW GCC 3.4.4.

My GLib version is 2.6.5 but I have checked that gthread.h has not changed in
GLib 2.8.1

Is this warning critical? I am trying to port GStreamer to Windows but this
project uses -Wall -Werror flags. Of cource GStreamer developers will not like
disabling -Werror flag.

See also bug #316079
Comment 1 Matthias Clasen 2005-09-13 16:36:12 UTC
If GSteamer wants to use -Werror, thats their problem.

We are certainly going to fix things that cause miscompilations, but
we cannot play catch-up with a changing set of compiler warnings.
Comment 2 Stanislav Brabec 2005-11-07 17:14:35 UTC
Strict aliasing warning means possible miscompilation (compiler will not reload
cached derefenrenced value in register after its change in memory) or need to
compile with decreased optimization level to get proper function.

There is an example of miscompiled code on x86_64:
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

void changeshort (short **p) {
    **p = -1;
}

int main(void)
{
        long *v = 0;
        v = malloc(sizeof(long));
        changeshort ((short**)&v);
        printf("%ld\n", *v);

        return 0;
}


sbrabec@hammer:~> gcc -Wall aliasing_example.c -o aliasing_example ;
./aliasing_example
65535
sbrabec@hammer:~> gcc -O3 -Wall aliasing_example.c -o aliasing_example ;
./aliasing_example
aliasing_example.c: In function ‘main’:
aliasing_example.c:13: warning: dereferencing type-punned pointer will break
strict-aliasing rules
0
Comment 3 Stanislav Brabec 2005-11-11 13:51:55 UTC
Created attachment 54636 [details] [review]
glib-strict-aliasing-macros.patch

Proposed patch.
Comment 4 Behdad Esfahbod 2005-11-11 16:52:29 UTC
An easier fix is to cast to (gpointer) before casting to something else.  So for
example, instead of (gpointer*)mutex, you write (gpointer*)(gpointer)mutex.  The
original cast is still needed, otherwise C++ compilerls won't like it.
Comment 5 Stanislav Brabec 2005-11-11 19:09:39 UTC
As far as I understand it, double cast hides warning, but not surely fixes the
problem. Strict alisasing rules allows dereferencing for type punned pointers
only for void and char pointers. Union fix should be cleaner. And because union
is already defined, it is also an easy fix.

I can consult the fix with our gcc experts.
Comment 6 Behdad Esfahbod 2005-11-11 19:13:34 UTC
AFAIK, double cast doesn't hide the problem.  Since you are casting foo* to
void*, the compiler doesn't make any aliasing assumptions about the foo* anymore.
Comment 7 Stanislav Brabec 2005-11-14 16:13:43 UTC
Created attachment 54739 [details] [review]
glib-strict-aliasing-macros.patch

My previous patch is bad and does not work.

I have just discussed this problem and possible fixes with Richard Guenther:
- Keep code as is - it breaks strict aliasing rules.
- usr dummy_pointer - it means (void**), not (void*), and that is why it breaks
strict aliasing rules.
- Declare dummy_gmutex inside union - impossible, cannot instantiate incomplete
type.
- Use double cast - not a fix, warning disappears, problem remains.
- Dereference pad[].

Details:

_GMutex is an incomplete type, so
you cannot instantiate it inside a struct or union ;)  The only
correct fix that remains is using the address of the char pad[] array,
which of course needs a gcc patch to suppress the warning.

Double cast only removes the warning, the compiler is free to "see 
through" the cast and only consider the outermost and innermost types
for alias analysis.
Comment 8 Stanislav Brabec 2005-11-14 16:15:36 UTC
Created attachment 54740 [details] [review]
glib-strict-aliasing-macros.patch

Alternative patch adds also double cast to prevent possible gcc warning.
Comment 9 Matthias Clasen 2005-12-05 13:20:16 UTC
gcc 4.0.2 compiles the testcase without any warning here, with the current headers.
Comment 10 Matthias Clasen 2005-12-27 19:45:53 UTC
Hmm, I see the warnings now with gcc 4.1. But I don't quite understand your
patch. Aren't you missing an '&' there ? Comment #7 talks about dereferencing
pad, but I see no dereferencing in your patch.
Comment 11 Matthias Clasen 2005-12-27 19:49:25 UTC
2005-12-27  Matthias Clasen  <mclasen@redhat.com>

        Fix #316221, Michal Benes, Stanislav Brabec;

        * configure.in: Fix a strict aliasing problem in
        g_static_mutex_get_mutex().
        * glib/gthread.h: ...and in
        g_static_mutex_get_mutex_impl_shortcut().

Comment 12 Stanislav Brabec 2006-01-02 13:38:42 UTC
pad is a char array variable. Using of array name without [] performs the dereference. And because pad is part of union, strict aliasing is OK.
Comment 13 Murray Cumming 2006-01-16 17:39:06 UTC
This breaks the glibmm build, because the lack of a cast is an error in C++. I guess this needs to be reverted if there isn't some way to fix it. See bug #327022
Comment 14 Matthias Clasen 2006-01-16 18:36:31 UTC
Hmm, very nasty. 
If I re-add the (gpointer*) cast, does g++ spew strict aliasing warnings ?
Would it be possible to do this instead ? 

#ifdef __cplusplus
#define g_static_mutex_get_mutex_impl_shortcut(mutex) \
  (g_atomic_pointer_get ((gpointer*)mutex) ? *(mutex) :	      \
   g_static_mutex_get_mutex_impl (mutex))
#else
#define g_static_mutex_get_mutex_impl_shortcut(mutex) \
  (g_atomic_pointer_get (mutex) ? *(mutex) :	      \
   g_static_mutex_get_mutex_impl (mutex))
#endif

Very ugly...
Comment 15 Gustavo Carneiro 2007-10-03 18:24:07 UTC
I am seeing this warning right now in glib 2.14.1 (ubuntu gutsy).  It's not just G_LOCK.  I changed the code to GStaticMutex and g_static_mutex_* calls and the warning remains.
Comment 16 Gustavo Carneiro 2007-10-03 18:30:29 UTC
PS: the warning message I see is only slightly different: "warning: type-punning to incomplete type might break strict-aliasing rules".  Compiling with g++ 4.1.3.

I have no idea how to fix; it's strange because GStaticMutex appears to be defined in my glibconfig.h:

typedef struct _GStaticMutex GStaticMutex;
struct _GStaticMutex
{
  //...
};
Comment 17 Dan Williams 2008-02-08 00:06:34 UTC
*** Bug 507134 has been marked as a duplicate of this bug. ***
Comment 18 Sebastian Dröge (slomo) 2008-02-13 12:16:43 UTC
Ok, any news on this bug? :)
It definitely still happens with glib 2.15, see bug #507134 for an example.
Comment 19 Sebastian Dröge (slomo) 2008-03-08 13:14:37 UTC
Created attachment 106842 [details] [review]
aliasing.diff

What about this patch? Works fine for me with gcc 4.3 and building gstreamer... without it fails because of -Werror and the aliasing warning.
Comment 20 Sebastian Dröge (slomo) 2008-03-08 13:23:02 UTC
also glibmm builds fine with it. Would be nice if I could commit this one before 2.16.0 release ;)


For glibmm I get these warnings though, no matter if I patch gthread.h or not or if I guard it in an #ifdef __cplusplus or not...

thread.cc: In member function 'void Glib::StaticMutex::lock()':
thread.cc:166: warning: type-punning to incomplete type might break strict-aliasing rules
thread.cc: In member function 'bool Glib::StaticMutex::trylock()':
thread.cc:171: warning: type-punning to incomplete type might break strict-aliasing rules
thread.cc: In member function 'void Glib::StaticMutex::unlock()':
thread.cc:176: warning: type-punning to incomplete type might break strict-aliasing rules
thread.cc: In member function 'Glib::StaticMutex::operator Glib::Mutex&()':
thread.cc:198: warning: type-punning to incomplete type might break strict-aliasing rules
thread.cc: In constructor 'Glib::RecMutex::RecMutex()':
thread.cc:273: warning: type-punning to incomplete type might break strict-aliasing rules
thread.cc: In constructor 'Glib::RWLock::RWLock()':
thread.cc:330: warning: type-punning to incomplete type might break strict-aliasing rules
Comment 21 Sebastian Dröge (slomo) 2008-03-12 15:36:33 UTC
2008-03-12  Sebastian Dröge  <slomo@circular-chaos.org>

	Bug 316221 - G_LOCK warns about breaking strict-aliasing rules

	* configure.in:
	* glib/gthread.h: Prevent the compiler from warning about breaking
	strict-aliasing rules when using gcc 4.3 and G_LOCK on C sources.

Comment 22 Dan Williams 2008-04-08 20:02:42 UTC
This commit breaks PPC:

nm-utils.c:129: error: passing argument 1 of 'g_atomic_pointer_get' from incompatible pointer type
nm-utils.c:153: error: passing argument 1 of 'g_atomic_pointer_get' from incompatible pointer type

which maps to:

127:	static GStaticMutex mutex = G_STATIC_MUTEX_INIT;
128:
129:	g_static_mutex_lock (&mutex);
.... do stuff
153:	g_static_mutex_unlock (&mutex);

Comment 23 Sebastian Dröge (slomo) 2008-04-08 20:11:25 UTC
Confirmed, also on other architectures (ia64, ...). Any idea how to fix both warnings at the same time? :)
Comment 24 Dan Williams 2008-04-13 15:03:37 UTC
Well, the patch from March 12 should probably be reverted so there aren't compile failures any more; the previous code just emitted warnings (and when using -Werror you of course needed to -fno-strict-aliasing) but didn't fail like the current bits do.  From there, we could figure out how to work around the new gcc strictness.
Comment 25 Dan Williams 2008-04-13 15:06:01 UTC
*** Bug 527328 has been marked as a duplicate of this bug. ***
Comment 26 Sebastian Dröge (slomo) 2008-04-13 16:31:17 UTC
Hm, "error: passing argument 1 of 'g_atomic_pointer_get' from
incompatible pointer type" is a warning in the build logs where I saw it. See http://buildd.debian.org/fetch.cgi?pkg=glib2.0;ver=2.16.3-2;arch=powerpc;stamp=1207953299 for example. Which compiler makes an error out of this without -Werror?
Comment 27 Sebastian Dröge (slomo) 2008-04-24 17:18:31 UTC
*** Bug 529742 has been marked as a duplicate of this bug. ***
Comment 28 Tim-Philipp Müller 2008-05-02 14:45:45 UTC
*** Bug 531067 has been marked as a duplicate of this bug. ***
Comment 29 Sebastian Dröge (slomo) 2008-06-20 11:29:40 UTC
Ok, after talking about it with Matthias on IRC again the committed patch is reverted in 2.16 and trunk as it doesn't improve the situation.
Comment 30 alexandre.nunes 2008-08-12 19:56:55 UTC
A bit out of topic and out of date, but recent gcc has -Wno-error=strict-aliasing , which at least allows people to keep (to some extent) strict build rules.

Of course, this annoying warnings from gtk should vanish eventually, but I couldn't find a cast that would allow them to get away yet (glib 2.16.14, linux x86 32 bits).
Comment 31 Behdad Esfahbod 2008-12-17 23:34:35 UTC
*** Bug 564935 has been marked as a duplicate of this bug. ***
Comment 32 Martin Walch 2009-02-15 13:32:14 UTC
This is still present in glib-2.18.4 and causes in Gentoo (amd64, gcc-4.3.3) a QA Notice, repeating 474 occurrences of this warning.
Comment 33 Thomas Andersen 2009-02-18 16:06:53 UTC
This seems to have been fixed in 2.19.8 by:
http://svn.gnome.org/viewvc/glib?view=revision&sortby=date&revision=7875

The strict alias warnings at least went away when compiling gnome-games with glib 2.19.8 while they were present with glib 2.19.7

Fixed?
Comment 34 Dan Winship 2009-06-03 12:55:18 UTC
*** Bug 584670 has been marked as a duplicate of this bug. ***
Comment 35 Matthias Clasen 2011-06-04 04:14:02 UTC
Ok, assuming fixed then