Bug 41423

Looks like a system bug to me. This function was defined in a header (windows.h / winnt.h) and never implemented anywhere. The MS documentation[1] says it's inline, no DLL is necessary for this.

I'd say NOTOURBUG, the bug is in your toolchain. MinGW should not provide declarations for functions it does not implement. Or if it wants to be 100% compatible with the Win32 API, it must implement them.

[1] http://msdn.microsoft.com/en-us/library/ms684208(VS.85).aspx

(In reply to comment #1)
> Looks like a system bug to me. This function was defined in a header (windows.h
> / winnt.h) and never implemented anywhere. The MS documentation[1] says it's
> inline, no DLL is necessary for this.
> 
> I'd say NOTOURBUG, the bug is in your toolchain. MinGW should not provide
> declarations for functions it does not implement. Or if it wants to be 100%
> compatible with the Win32 API, it must implement them.
> 
> [1] http://msdn.microsoft.com/en-us/library/ms684208(VS.85).aspx

I got it. Thanks!

Turns out that the MemoryBarrier function/macro is only available on Windows Server 2003 or Vista or up. It's in the link I posted but I failed to take notice.

So the D-Bus code should properly avoid it.

If this is the patch you mean:

28 From d7bc1aff67132c09d154999c282e1b391bbe7a39 Mon Sep 17 00:00:00 2001
29 From: mingw-cross-env
30 Date: Fri, 23 Sep 2011 16:24:46 +0200
31 Subject: [PATCH 2/2] add missing MemoryBarrier (mingw-cross-env specific)
32
33 Windows API has this, but it's not in MinGW.
34 See http://lists-archives.org/mingw-users/15935-missing-definition-of-memorybarrier.html
35
36 diff --git a/dbus/dbus-sysdeps-win.c b/dbus/dbus-sysdeps-win.c
37 index b492b09..082225f 100644
38 --- a/dbus/dbus-sysdeps-win.c
39 +++ b/dbus/dbus-sysdeps-win.c
40 @@ -55,6 +55,13 @@
41 #include <ws2tcpip.h>
42 #include <wincrypt.h>
43
44 +__CRT_INLINE VOID MemoryBarrier(VOID)
45 +{
46 + LONG Barrier = 0;
47 + __asm__ __volatile__("xchgl %%eax,%0 "
48 + :"=r" (Barrier));
49 +}
50 +
51 /* Declarations missing in mingw's headers */
52 extern BOOL WINAPI ConvertStringSidToSidA (LPCSTR StringSid, PSID *Sid);
53 extern BOOL WINAPI ConvertSidToStringSidA (PSID Sid, LPSTR *StringSid);
54 -- 

will it break compilation in non-mingw32 and/or Vista-or-newer environments?

Is this portable to compilers other than gcc? (My guess is "no".)

If this is approximately how memory barriers are implemented in the Win32 SDK or whatever it is that provides MemoryBarrier (as an always-inline thing), then surely the distinction isn't between *running* D-Bus on (<=) XP or (>=) Vista, but between compiling D-Bus on an older or newer SDK/compiler/C-runtime?

(In reply to comment #3)
> Turns out that the MemoryBarrier function/macro is only available on Windows
> Server 2003 or Vista or up.

How would that even work, if it's a macro that expands to a compiler intrinsic? Is "only available on Vista+" secret Microsoft code for "only available on the Win32 headers supplied by the Vista+ SDKs" or something?

It occurs to me that since InterlockedIncrement is claimed to be a full memory barrier, this could work:

#ifdef MemoryBarrier
  MemoryBarrier ();
#else
  /* do something else that provides a read/write memory barrier,
   * but exists in pre-Vista Win32 headers and old mingw versions */
  volatile long *dummy;
  InterlockedIncrement (&dummy);
#endif

desrt, any thoughts on this? Have you had people complain about the corresponding code in GLib, or has nobody tried building GLib 2.31 against Windows XP headers yet?

(In reply to comment #4)

> will it break compilation in non-mingw32 and/or Vista-or-newer environments?

in short - yes 

c:\kde\trunk\git\dbus-src-git\dbus\dbus-sysdeps-win.c(59) : error C2084: Funktion 'void MemoryBarrier(void)' hat bereits einen Funktionsrumpf
        C:\Program Files\Microsoft SDKs\Windows\v7.0A\include\winnt.h(3939): Siehe vorherige Definition von 'MemoryBarrier'

(In reply to comment #6)
> in short - yes 

We can't take this patch, then.

I'm still somewhat inclined to say NOTOURBUG, and the mingw32 maintainers should add MemoryBarrier to their winnt.h. mingw-w64, which is a confusingly-named fork of mingw32 that supports both 32- and 64-bit Windows, does have MemoryBarrier.

Since MemoryBarrier is an inlined compiler intrinsic, I still can't see how it can possibly affect runtime requirements, only build-time requirements? If it means you can't *compile* D-Bus unless you have either an up-to-date Windows SDK or a mingw-w64 toolchain, to be honest I'm tempted to say "get a newer Windows SDK or a mingw-w64 toolchain, then" (both appear to be free-to-download).

(In reply to comment #4)
> Is this portable to compilers other than gcc? (My guess is "no".)

msvc 2010 defines this function at C:\Program Files\Microsoft
SDKs\Windows\v7.0A\Include\winnt.h(3939) as: 

FORCEINLINE
VOID
MemoryBarrier (
    VOID
    )
{
    LONG Barrier;
    __asm {
        xchg Barrier, eax
    }
}


> If this is approximately how memory barriers are implemented in the Win32 SDK
> or whatever it is that provides MemoryBarrier (as an always-inline thing), then
> surely the distinction isn't between *running* D-Bus on (<=) XP or (>=) Vista,
> but between compiling D-Bus on an older or newer SDK/compiler/C-runtime?

it looks so

http://msdn.microsoft.com/en-us/library/aa383745%28v=vs.85%29.aspx shows some
plattform related defines. One can set for example _WIN32_WINNT to enable the
api set for a specific os version, but there is no setting for detecting the
sdk version. 

Also there is the _MSC_VER macro which defines the compiler/c-runtime version 

In the mentioned header from msvc2010 (_MSC_VER=1400), which uses windows sdk
7.0a, MemoryBarrier is defined unconditional. On msvc2008, which uses windows
sdk 6.0a the inline function is also there  (_MSC_VER=1600).

For msvc I would say wrapping the MemoryBarrier inline definition with
_MSC_VER<1400 should work like shown below: 

#if defined(_MSC_VER) && _MSC_VER < 1400 
inline VOID MemoryBarrier (VOID)
{
    LONG Barrier;
    __asm {
        xchg Barrier, eax
    }
}
#elif ....  // gcc part 
__inline VOID MemoryBarrier(VOID)
{
  LONG Barrier = 0;
  __asm__ __volatile__("xchgl %%eax,%0 "
  :"=r" (Barrier));
}
#endif

there is still the gcc part open.

(In reply to comment #7)
> (In reply to comment #6)
> > in short - yes 
> 
> We can't take this patch, then.
> 
> I'm still somewhat inclined to say NOTOURBUG, and the mingw32 maintainers
> should add MemoryBarrier to their winnt.h. mingw-w64, which is a
> confusingly-named fork of mingw32 that supports both 32- and 64-bit Windows,
> does have MemoryBarrier.
> 
> Since MemoryBarrier is an inlined compiler intrinsic, I still can't see how it
> can possibly affect runtime requirements, only build-time requirements? If it
> means you can't *compile* D-Bus unless you have either an up-to-date Windows
> SDK or a mingw-w64 toolchain, to be honest I'm tempted to say "get a newer
> Windows SDK or a mingw-w64 toolchain, then" (both appear to be
> free-to-download).

This lead to the question - for which os and/or compiler should be dbus support available ? 

A dedicated compiler support needs people having a test environment for that  compiler and there seems to be several compiler/sdk combinations possible to compile dbus on windows. 

At least we have vc and gcc based compilers. 

msvc 2005 is very old, msvc 2008 could not be downloaded from internet since one or two years and msvc 2010 is the recent compiler. 

How is the situation with mingw ? 

Next, what kind of cross compile options are there for dbus ?

(In reply to comment #9)
> This lead to the question - for which os and/or compiler should
> be dbus support available ? 

Well, we have active or semi-active maintainers for modern gcc + autotools on modern Linux (me, Lennart, Colin) and GNU/kFreeBSD (me + Debian kFreeBSD fans), and for whatever versions of MSVC and Windows you use (you).

Everything else is best-effort - if people want their OS supported they'll have to help us. We do get contributions for Mac OS and Solaris fairly often; I believe the BSDs work fine too. Most of the time, making things work in a recent version of any popular Unix isn't that hard, so we might as well.

I do tend to draw the line at Unix OSs that aren't (at least mostly) POSIX-compliant, because they've had literally a decade to get there. In particular, Interix seems to be substandard enough that supporting it would require treating it as a third major platform (Unix, Windows, Interix).

As far as I'm concerned, the only purpose of having cmake in D-Bus is to be nice to MSVC++ users - everyone else can use autotools. I do think it's worth keeping cmake working on Linux, because that means I can test cmake changes that aren't Windows-specific (for instance to keep up with restructuring that happened in autotools-land), but I'm not really interested in cmake on niche Unix platforms.

Regarding Windows, it'd be nice if D-Bus worked with some variant of mingw (which has the advantage of combining a Free compiler with widely-available MS runtime libraries), Cygwin (which has a significant Unix-portability layer, but can be treated as a mixture of Unix and Windows), and MSVC++ if you insist ;-) I believe all of those currently work fine.

mingw and Cygwin both use gcc in a Unix-ish environment (you can expect to have enough Unix shell stuff to make autotools work), and neither needs a Windows SDK (mingw is basically a Free, reverse-engineered Windows SDK, using msvcrt.dll as its runtime library). So, it seems fine to require use of autotools, and to require a reasonably recent gcc/mingw if necessary.

As far as MSVC++ is concerned, I think you get to decide what you support - choose a compiler and a Windows SDK version (preferably one that's downloadable at no cost) and declare anything older to be unsupported. For runtime requirements, the usual minimum version these days seems to be XP, or 2000 if it's easy to do?

> How is the situation with mingw ?

There are two major versions, mingw32 (the original, apparently rather outdated and not very well maintained) and mingw-w64 (a fork which also supports 64-bit Windows). After some recent discussion on the debian-devel mailing list, Debian is in the process of moving cross-compiled packages from mingw32 to mingw-w64. I believe Fedora has already done the same.

> Next, what kind of cross compile options are there for dbus ?

In principle gcc can cross-compile for any supported host architecture on any supported build architecture. I'm not sure whether dbus' cmake build system knows how to cross-compile, but to be honest I don't really care - the autotools build system does, and that's sufficient. If you're cross-compiling, then in practice you're almost certainly running gcc on Linux anyway.

I run x86-64 Linux, and occasionally cross-compile dbus for 32-bit Windows using mingw-w64. Some of the tests pass under Wine, some don't (I suspect some of the tests have hidden Unix assumptions, though - do you run them on real Windows?)

(In reply to comment #10)
> (In reply to comment #9)
> > This lead to the question - for which os and/or compiler should
> > be dbus support available ? 
> 
> Well, we have active or semi-active maintainers for modern gcc + autotools on
> modern Linux (me, Lennart, Colin) and GNU/kFreeBSD (me + Debian kFreeBSD fans),
> and for whatever versions of MSVC and Windows you use (you).
> 
> Everything else is best-effort - if people want their OS supported they'll have
> to help us. We do get contributions for Mac OS and Solaris fairly often; I
> believe the BSDs work fine too. Most of the time, making things work in a
> recent version of any popular Unix isn't that hard, so we might as well.
> 
> I do tend to draw the line at Unix OSs that aren't (at least mostly)
> POSIX-compliant, because they've had literally a decade to get there. In
> particular, Interix seems to be substandard enough that supporting it would
> require treating it as a third major platform (Unix, Windows, Interix).
> 
> As far as I'm concerned, the only purpose of having cmake in D-Bus is to be
> nice to MSVC++ users - everyone else can use autotools. I do think it's worth
> keeping cmake working on Linux, because that means I can test cmake changes
> that aren't Windows-specific (for instance to keep up with restructuring that
> happened in autotools-land), but I'm not really interested in cmake on niche
> Unix platforms.

Just a few side note: 

- dbus cmake build system is recommended by the kde on windows project for msvc 2010 and mingw-w32/w64 compiler. 
- also using cmake on windows reduces the amount of required dependencies 

autotools: msys, autoconf, automake, m4,... expat, gcc, binutils, make
cmake:    expat, gcc, binutils, make, cmake
   
 
> Regarding Windows, it'd be nice if D-Bus worked with some variant of mingw
> (which has the advantage of combining a Free compiler with widely-available MS
> runtime libraries), Cygwin (which has a significant Unix-portability layer, but
> can be treated as a mixture of Unix and Windows), and MSVC++ if you insist ;-)
> I believe all of those currently work fine.
> 
> mingw and Cygwin both use gcc in a Unix-ish environment (you can expect to have
> enough Unix shell stuff to make autotools work), and neither needs a Windows
> SDK (mingw is basically a Free, reverse-engineered Windows SDK, using
> msvcrt.dll as its runtime library). So, it seems fine to require use of
> autotools, and to require a reasonably recent gcc/mingw if necessary.
> 
> As far as MSVC++ is concerned, I think you get to decide what you support -
> choose a compiler and a Windows SDK version (preferably one that's downloadable
> at no cost) and declare anything older to be unsupported. For runtime
> requirements, the usual minimum version these days seems to be XP, or 2000 if
> it's easy to do?

> 
> > How is the situation with mingw ?
> 
> There are two major versions, mingw32 (the original, apparently rather outdated
> and not very well maintained) and mingw-w64 (a fork which also supports 64-bit
> Windows). After some recent discussion on the debian-devel mailing list, Debian
> is in the process of moving cross-compiled packages from mingw32 to mingw-w64.
> I believe Fedora has already done the same.

kde on windows migrated from mingw4 to mingw32/63 this year. 

Okay, then we should announce the following: 

dbus supports Windows XP, Windows Vista and Windows 7
supported compiler/sdk are msvc 2010, mingw-w32/w64, cygwin


> 
> > Next, what kind of cross compile options are there for dbus ?
> 
> In principle gcc can cross-compile for any supported host architecture on any
> supported build architecture. I'm not sure whether dbus' cmake build system
> knows how to cross-compile, but to be honest I don't really care - the
> autotools build system does, and that's sufficient. If you're cross-compiling,
> then in practice you're almost certainly running gcc on Linux anyway.
> 
> I run x86-64 Linux, and occasionally cross-compile dbus for 32-bit Windows
> using mingw-w64. Some of the tests pass under Wine, some don't (I suspect some
> of the tests have hidden Unix assumptions, though  - do you run them on real Windows?)
yes 

c:\>cd <build-root>
c:\>set DBUS_TEST_DATA=%CD%\test\data
c:\>bin\dbus-test.exe 
Test data in C:\kde\trunk\build\win32libs-sources\dbus-src-1.4.10-20110302\work\msvc2010-Debug-gitHEAD\test\data
dbus-test: running string tests
dbus-test: checking for memleaks
dbus-test: running sysdeps tests
dbus-test: checking for memleaks
dbus-test: running data-slot tests
dbus-test: checking for memleaks
dbus-test: running misc tests
dbus-test: checking for memleaks
dbus-test: running address tests
dbus-test: checking for memleaks
dbus-test: running server tests
dbus-test: checking for memleaks
dbus-test: running object-tree tests
dbus-test: checking for memleaks
dbus-test: running signature tests
dbus-test: checking for memleaks
dbus-test: running marshalling tests
dbus-test: checking for memleaks
SKIP: recursive marshal tests disabled
dbus-test: running byteswap tests
  120 blocks swapped from order 'l' to 'B'
  120 blocks swapped from order 'B' to 'l'
dbus-test: checking for memleaks
dbus-test: running memory tests
dbus-test: checking for memleaks
dbus-test: running mem-pool tests
dbus-test: checking for memleaks
dbus-test: running list tests
dbus-test: checking for memleaks
dbus-test: running marshal-validate tests
dbus-test: checking for memleaks
dbus-test: running message tests
 testing message loading: trivial example of each message type 4 test loads cumulative
 testing message loading: assorted arguments 124 test loads cumulative
 testing message loading: assorted special cases 146 test loads cumulative
 testing message loading: each uint32 modified 35368 test loads cumulative
 testing message loading: wrong body lengths 38368 test loads cumulative
 testing message loading: each byte modified 51547 test loads cumulative
51547 sample messages tested
validity   0 seen 381 times
validity   1 seen 3471 times
validity   2 seen 177 times
validity   4 seen 3 times
validity   5 seen 3 times
validity   6 seen 3 times
validity   7 seen 273 times
validity   8 seen 3 times
validity   9 seen 15780 times
validity  10 seen 189 times
validity  11 seen 2727 times
validity  12 seen 5718 times
validity  13 seen 3960 times
validity  14 seen 360 times
validity  15 seen 3 times
validity  16 seen 1080 times
validity  17 seen 1440 times
validity  18 seen 1107 times
validity  19 seen 3 times
validity  20 seen 1080 times
validity  21 seen 7200 times
validity  22 seen 3 times
validity  23 seen 3 times
validity  24 seen 720 times
validity  25 seen 360 times
validity  26 seen 1440 times
validity  27 seen 720 times
validity  28 seen 360 times
validity  29 seen 3 times
validity  30 seen 720 times
validity  31 seen 3 times
validity  32 seen 360 times
validity  33 seen 3 times
validity  34 seen 3 times
validity  35 seen 14625 times
validity  37 seen 6666 times
validity  38 seen 2013 times
validity  39 seen 15447 times
validity  40 seen 1788 times
validity  41 seen 8325 times
validity  42 seen 3408 times
validity  43 seen 4875 times
validity  45 seen 3327 times
validity  46 seen 7485 times
validity  47 seen 1125 times
validity  48 seen 3 times
validity  49 seen 3 times
validity  50 seen 144 times
validity  51 seen 3 times
validity  54 seen 12 times
validity  57 seen 3 times
validity  -4 seen 0 times
validity  -2 seen 0 times
validity   3 seen 0 times
validity  36 seen 0 times
validity  44 seen 0 times
validity  52 seen 0 times
validity  53 seen 0 times
validity  55 seen 0 times
validity  56 seen 0 times
Testing valid-messages:
SKIP: Could not load array-of-array-of-uint32.message, message builder language no longer supported
SKIP: Could not load dict-simple.message, message builder language no longer supported
SKIP: Could not load dict.message, message builder language no longer supported
SKIP: Could not load emptiness.message, message builder language no longer supported
SKIP: Could not load lots-of-arguments.message, message builder language no longer supported
SKIP: Could not load no-padding.message, message builder language no longer supported
SKIP: Could not load opposite-endian.message, message builder language no longer supported
SKIP: Could not load recursive-types.message, message builder language no longer supported
SKIP: Could not load simplest-manual.message, message builder language no longer supported
SKIP: Could not load simplest.message, message builder language no longer supported
SKIP: Could not load standard-acquire-service.message, message builder language no longer supported
SKIP: Could not load standard-hello.message, message builder language no longer supported
SKIP: Could not load standard-list-services.message, message builder language no longer supported
SKIP: Could not load standard-service-exists.message, message builder language no longer supported
SKIP: Could not load unknown-header-field.message, message builder language no longer supported
Testing invalid-messages:
SKIP: Could not load array-of-nil.message, message builder language no longer supported
SKIP: Could not load array-with-mixed-types.message, message builder language no longer supported
SKIP: Could not load bad-boolean-array.message, message builder language no longer supported
SKIP: Could not load bad-boolean.message, message builder language no longer supported
SKIP: Could not load bad-endian.message, message builder language no longer supported
SKIP: Could not load bad-header-field-alignment.message, message builder language no longer supported
    boolean-has-no-value.message-raw
SKIP: Could not load local-namespace.message, message builder language no longer supported
SKIP: Could not load no-dot-in-name.message, message builder language no longer supported
SKIP: Could not load not-nul-header-padding.message, message builder language no longer supported
SKIP: Could not load overlong-name.message, message builder language no longer supported
SKIP: Could not load too-little-header-padding.message, message builder language no longer supported
SKIP: Could not load too-much-header-padding-by-far.message, message builder language no longer supported
SKIP: Could not load too-much-header-padding.message, message builder language no longer supported
SKIP: Could not load too-short-dict.message, message builder language no longer supported
Testing incomplete-messages:
SKIP: Could not load missing-body.message, message builder language no longer supported
dbus-test: checking for memleaks
dbus-test: running hash tests
Computing test hash keys...
... done.
dbus-test: checking for memleaks
dbus-test: running spawn tests
dbus-test: checking for memleaks
dbus-test: running credentials tests
dbus-test: checking for memleaks
dbus-test: running keyring tests
Using your real home directory for testing, set DBUS_TEST_HOMEDIR to avoid
 1 keys in test
dbus-test: checking for memleaks
dbus-test: running sha tests
SHA-1: H>SHS Type 1 Strings<H
SHA-1: H>SHS Type 2 Strings<H
SHA-1: H>SHS Type 3 Strings<H
 (ending tests due to Type 3 tests seen - this is normal)
Passed the 229 SHA-1 tests in the test file
dbus-test: checking for memleaks
dbus-test: running auth tests
Testing auth:
    anonymous-client-successful.auth-script
    anonymous-server-successful.auth-script
    cancel.auth-script
    client-out-of-mechanisms.auth-script
    external-failed.auth-script
    external-root.auth-script
skipping unix only auth script
    external-silly.auth-script
    external-successful.auth-script
    extra-bytes.auth-script
    fail-after-n-attempts.auth-script
    fallback.auth-script
    invalid-command-client.auth-script
    invalid-command.auth-script
    invalid-hex-encoding.auth-script
    mechanisms.auth-script
dbus-test: checking for memleaks
dbus-test: completed successfully


There is still a problem with session.d directory 
Failed to load valid file but still had memory: Failed to read directory "C:\kde\trunk\build\win32libs-sources\dbus-src-1.4.10-20110302\work\msvc2010-Debug-gitHEAD\test\data\valid-config-files\session.d": Das System kann den angegebenen Pfad nicht finden.

which is fixed temporary by the following command 
C:\kde\trunk\build\win32libs-sources\dbus-src-1.4.10-20110302\work\msvc2010-Debug-gitHEAD>mkdir %CD%\test\data\valid-config-files\session.d

C:\kde\trunk\build\win32libs-sources\dbus-src-1.4.10-20110302\work\msvc2010-Debug-gitHEAD>bin\bus-test.exe
bin\bus-test.exe: Running expire list test
bin\bus-test.exe: checking for memleaks
bin\bus-test.exe: Running config file parser test
Testing retrieving the default session service directories
    default service dir: C:\kde\trunk\build\win32libs-sources\dbus-src-1.4.10-20110302\work\msvc2010-Debug-gitHEAD\share/dbus-1/services
    default service dir: C:\Program Files\Common Files/dbus-1/services
    test service dir: C:\kde\trunk\build\win32libs-sources\dbus-src-1.4.10-20110302\work\msvc2010-Debug-gitHEAD\share/dbus-1/services
    test service dir: C:\Program Files\Common Files/dbus-1/services
default system service dir skipped
Testing valid files:
    basic.conf
    debug-allow-all-sha1.conf
    debug-allow-all.conf
    entities.conf
    incoming-limit.conf
    many-rules.conf
Unknown username "root" on element <deny>
Unknown group "bin" on element <deny>
Unknown username "root" on element <deny>
Unknown group "bin" on element <deny>
    session.conf
    system.conf
Unknown username "root" in message bus configuration file
Testing invalid files:
    badselinux-1.conf
    badselinux-2.conf
    circular-1.conf
    circular-2.conf
    circular-3.conf
    not-well-formed.conf
    truncated-file.conf
Comparing equivalent files:
    basic-1.conf
    basic-2.conf
Comparing equivalent files:
    entities-1.conf
    entities-2.conf
bin\bus-test.exe: checking for memleaks
bin\bus-test.exe: Running signals test
bin\bus-test.exe: checking for memleaks
bin\bus-test.exe: Running SHA1 connection test
Using your real home directory for testing, set DBUS_TEST_HOMEDIR to avoid
bin\bus-test.exe: checking for memleaks
bin\bus-test.exe: Running message dispatch test
TODO: testing of GetConnectionUnixUser message skipped for now
TODO: testing of GetConnectionUnixProcessID message skipped for now
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
TODO: dispatch.c segfault_service_no_auto_start test
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
TODO: dispatch.c segfault_service_auto_start test
Activating service name='org.freedesktop.DBus.TestSuiteEchoService'
Successfully activated service 'org.freedesktop.DBus.TestSuiteEchoService'
Echo service echoed string: "Test echo message"
Sending HelloFromSelf
Sent HelloFromSelf
Received the HelloFromSelf message
Reply from HelloFromSelf received
Activating service name='org.freedesktop.DBus.TestSuiteShellEchoServiceSuccess'
Successfully activated service 'org.freedesktop.DBus.TestSuiteShellEchoServiceSuccess'
Shell echo service echoed the command line
Info: Launch helper activation tests skipped because launch-helper is not supported yet
bin\bus-test.exe: checking for memleaks
bin\bus-test.exe: Running service files reloading test
bin\bus-test.exe: checking for memleaks
bin\bus-test.exe: Success

mingw4 isn't a supported platform on windows. 

Compilers are supported because people use them in their daily work and are able and willing to fix bugs and provide patches. 

If people need to have support for a dedicated compiler they have to care about this compiler in term of bug fixing and providing working patches.

*** Bug 44431 has been marked as a duplicate of this bug. ***

I'm reopening this because as much as it's not really our problem, mingw.org doesn't show any signs of fixing this, and everyone[1][2][3][4] keeps hacking around it using cargo-culted patches that will break MSVC and mingw-w64.

A note on terminology to be completely clear, since it's ridiculously confusing:

MinGW (mingw.org, current version "MinGW 4", formerly MinGW32, GNU tuple i586-pc-mingw32), is a build environment and toolchain targeting 32-bit Windows, either compiling under Windows or cross-compiling from Unix. It used to be shipped in Debian as gcc-mingw32.

MinGW-w64 (mingw-w64.sourceforge.net, GNU tuples i686-w64-mingw32 for 32-bit and x86_64-w64-mingw32 for 64-bit) is a fork of MinGW targeting both 32- and 64-bit Windows. Its GNU tuples seem to have been carefully chosen to cause the maximum possible amount of confusion, but it also seems to work better than MinGW. It's what is now shipped in Debian, e.g. to compile Wine.

Ralf is D-Bus' Windows maintainer. I believe the configuration he supports is CMake + MSVC. As a result, we will not knowingly apply patches that break MSVC, and if we accidentally break this configuration, Ralf will hopefully catch it eventually.

I occasionally test that D-Bus cross-compiles with Autotools and MinGW-w64 on Debian - some of the tests even work under Wine in this configuration - so I have an interest in making this configuration work. So, we will not knowingly apply patches that break Autotools + MinGW-w64, and if we accidentally break this configuration, we'll hopefully catch it eventually.

As far as I know, no D-Bus maintainer does systematic testing with MinGW from mingw.org, but it seems to be rather popular with people who cross-compile D-Bus anyway. If you (for any value of "you") would like MinGW to be a supported platform for D-Bus, please volunteer to be its maintainer, test it on a regular basis, and provide patches that aren't going to break our other platforms (i.e. respond to review feedback and suggest/test alternatives, rather than advocating use of patches that have already been rejected).

[1] https://lists.gnu.org/archive/html/mingw-cross-env-list/2011-09/msg00033.html
[2] http://lists.freedesktop.org/archives/dbus/2012-June/015158.html
[3] http://poquitopicante.blogspot.co.uk/2012/07/building-dbus-python-on-windows-with.html
[4] https://bugs.freedesktop.org/show_bug.cgi?id=44431

Created attachment 65342 [details] [review]
Use InterlockedIncrement to get a full memory barrier on  Windows

(In reply to comment #15)
> Created attachment 65342 [details] [review]
> Use InterlockedIncrement to get a full memory barrier on  Windows

There's my proposal. Note that I have not run it... but it seems better than having a patch floating around that's known to break our supported targets and hence will not be accepted, but everyone who likes mingw passes it around on the grapevine and applies it anyway.

Code review would be much appreciated.

When I compile with mingw-w64 and disassemble the resulting binary (objdump -d dbus/.libs/libdbus-1-3.dll) I get this implementation of _dbus_atomic_get:

66092210 <__dbus_atomic_get>:
66092210:	83 ec 10             	sub    $0x10,%esp
66092213:	b8 01 00 00 00       	mov    $0x1,%eax
66092218:	c7 44 24 0c 00 00 00 	movl   $0x0,0xc(%esp)
6609221f:	00 
66092220:	f0 0f c1 44 24 0c    	lock xadd %eax,0xc(%esp)
66092226:	8b 44 24 14          	mov    0x14(%esp),%eax
6609222a:	8b 00                	mov    (%eax),%eax
6609222c:	83 c4 10             	add    $0x10,%esp
6609222f:	c3                   	ret    

I don't speak fluent x86 assembler, but I believe the "lock xadd" is the memory barrier that we wanted, and the two mov instructions immediately following it are the "get" operation (the first one gets the DBusAtomic * from the stack, the second dereferences the pointer and puts the result in register EAX to be returned).

Someone who cares about MSVC (Ralf?), please compile and test this with MSVC, disassemble its version of _dbus_atomic_get and check that it does roughly the right thing.

Someone who cares about mingw.org's mingw (Mat? Siraj? Mark?), please compile and test this with mingw, disassemble *its* version of _dbus_atomic_get and check that *it* does the right thing.

Comment on attachment 65342 [details] [review]
Use InterlockedIncrement to get a full memory barrier on  Windows

See no reason why this should not work

A cosmetic optimization may be to use InterlockedExchange http://msdn.microsoft.com/en-us/library/windows/desktop/ms683590%28v=vs.85%29.aspx instead of InterlockedIncrement because
the original MemoryBarrier() implementation uses an exchange statement (mentioned in this thread). 
In this case the patch need to be updated with 
...  
-  InterlockedIncrement (&dummy);
+  InterlockedExchange (&dummy, 1);
...

(In reply to comment #17)
> A cosmetic optimization may be to use InterlockedExchange
> instead of InterlockedIncrement because
> the original MemoryBarrier() implementation uses an exchange statement

Sure, why not. On mingw-w64 that gets compiled to

66092210 <__dbus_atomic_get>:
66092210:	83 ec 10             	sub    $0x10,%esp
66092213:	b8 01 00 00 00       	mov    $0x1,%eax
66092218:	c7 44 24 0c 00 00 00 	movl   $0x0,0xc(%esp)
6609221f:	00 
66092220:	f0 87 44 24 0c       	lock xchg %eax,0xc(%esp)
66092225:	8b 44 24 14          	mov    0x14(%esp),%eax
66092229:	8b 00                	mov    (%eax),%eax
6609222b:	83 c4 10             	add    $0x10,%esp
6609222e:	c3                   	ret    
6609222f:	90                   	nop

which, again, looks to me like a memory barrier followed by a "get", as desired.

I'll attach the alternative patch.

Neither patch can be applied without review and testing. Please compile, disassemble, test, review?

Created attachment 65504 [details] [review]
[alternative] Use InterlockedExchange to get a full memory barrier on  Windows

The same as the other patch, but with InterlockedExchange instead, since both mingw-w64 and MSVC appear to implement MemoryBarrier() with an xchg instruction.

(In reply to comment #18)
> (In reply to comment #17)
> > A cosmetic optimization may be to use InterlockedExchange
> > instead of InterlockedIncrement because
> > the original MemoryBarrier() implementation uses an exchange statement
> 
> Sure, why not. On mingw-w64 that gets compiled to
> 
> 66092210 <__dbus_atomic_get>:
> 66092210:    83 ec 10                 sub    $0x10,%esp
> 66092213:    b8 01 00 00 00           mov    $0x1,%eax
> 66092218:    c7 44 24 0c 00 00 00     movl   $0x0,0xc(%esp)
> 6609221f:    00 
> 66092220:    f0 87 44 24 0c           lock xchg %eax,0xc(%esp)
> 66092225:    8b 44 24 14              mov    0x14(%esp),%eax
> 66092229:    8b 00                    mov    (%eax),%eax
> 6609222b:    83 c4 10                 add    $0x10,%esp
> 6609222e:    c3                       ret    
> 6609222f:    90                       nop
> 
> which, again, looks to me like a memory barrier followed by a "get", as
> desired.
> 
> I'll attach the alternative patch.
> 
> Neither patch can be applied without review and testing. Please compile,
> disassemble, test, review?

here is a disassemble of the related msvc compile. Interlocked... functions are located in kernel32.dll and are not inlined.

__dbus_atomic_inc:
  00000000: 8B 44 24 04        mov         eax,dword ptr [esp+4]
  00000004: 50                 push        eax
  00000005: FF 15 00 00 00 00  call        dword ptr [__imp__InterlockedIncrement@4]
  0000000B: 48                 dec         eax
  0000000C: C3                 ret

__dbus_atomic_dec:
  00000000: 8B 44 24 04        mov         eax,dword ptr [esp+4]
  00000004: 50                 push        eax
  00000005: FF 15 00 00 00 00  call        dword ptr [__imp__InterlockedDecrement@4]
  0000000B: 40                 inc         eax
  0000000C: C3                 ret

__dbus_atomic_get:
  00000000: 51                 push        ecx
  00000001: 6A 01              push        1
  00000003: 8D 44 24 04        lea         eax,[esp+4]
  00000007: 50                 push        eax
  00000008: C7 44 24 08 00 00  mov         dword ptr [esp+8],0
            00 00
  00000010: FF 15 00 00 00 00  call        dword ptr [__imp__InterlockedExchange@8]
  00000016: 8B 4C 24 08        mov         ecx,dword ptr [esp+8]
  0000001A: 8B 01              mov         eax,dword ptr [ecx]
  0000001C: 59                 pop         ecx
  0000001D: C3                 ret

The call to  
  00000010: FF 15 00 00 00 00  call        dword ptr [__imp__InterlockedExchange@8]

is the memory barrier and 

  0000001A: 8B 01              mov         eax,dword ptr [ecx]

is the get statement. 


According to http://www.installsetupconfig.com/win32programming/threadprocesssynchronizationapis11_40.html there is 

"... The InterlockedExchange() function generates a full memory barrier (or fence) and performs the exchange operation. This ensures the strict memory access ordering that is necessary,..." 

so this implementation looks good to me.

Fixed in git for 1.7.0.

I'm not going to go applying patches to the stable branch in code I don't compile to support toolchains none of the dbus maintainers use; but if anyone else wants to cherry-pick this onto dbus-1.6 after appropriate testing, go for it.