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 758738 - Usage of GType properties causes crashes due to gulong/gpointer mismatch
Usage of GType properties causes crashes due to gulong/gpointer mismatch
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gobject
unspecified
Other Windows
: Normal critical
: ---
Assigned To: gtkdev
gtkdev
: 758739 763962 (view as bug list)
Depends on:
Blocks: 765659
 
 
Reported: 2015-11-27 14:14 UTC by Nicola
Modified: 2016-05-06 10:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Gstreamer logs (199.27 KB, application/x-bzip)
2015-11-27 14:14 UTC, Nicola
  Details
sample app that crash on windows (1.26 KB, text/x-c++src)
2015-11-29 20:18 UTC, Nicola
  Details
test app that crash on windows (1.30 KB, text/x-c++src)
2015-11-29 20:20 UTC, Nicola
  Details
gparamspecs: GTypes are stored in v_pointer, not v_long (1.85 KB, patch)
2016-04-14 11:41 UTC, Sebastian Dröge (slomo)
committed Details | Review

Description Nicola 2015-11-27 14:14:27 UTC
Created attachment 316385 [details]
Gstreamer logs

Here is the backtrace

Program received signal SIGSEGV, Segmentation fault.
0x0000000063a6318d in type_node_check_conformities_UorL (have_lock=0,
    support_prerequisites=1, support_interfaces=1, iface_node=0xf1c3ee0e98,
    node=0xffffffffc5c38500) at gtype.c:3455
3455    gtype.c: No such file or directory.
(gdb) bt
  • #0 type_node_check_conformities_UorL
    at gtype.c line 3455
  • #1 type_node_conforms_to_U
    at gtype.c line 3500
  • #2 g_type_is_a
    at gtype.c line 3527
  • #3 param_gtype_validate
    at gparamspecs.c line 1081
  • #4 g_param_value_validate
    at gparam.c line 667
  • #5 object_set_property
    at gobject.c line 1402
  • #6 g_object_new_with_custom_constructor
    at gobject.c line 1753
  • #7 g_object_new_internal
    at gobject.c line 1772
  • #8 g_object_new_valist
    at gobject.c line 2033
  • #9 soup_session_async_new_with_options
    at soup-session-async.c line 77
  • #10 gst_soup_http_src_session_open
    at gstsouphttpsrc.c line 932
  • #11 gst_soup_http_src_session_open
    at gstsouphttpsrc.c line 903
  • #12 gst_base_src_start
    at gstbasesrc.c line 3312
  • #13 gst_base_src_activate_push
    at gstbasesrc.c line 3699
  • #14 gst_base_src_activate_mode
    at gstbasesrc.c line 3776
  • #15 gst_pad_activate_mode
    at gstpad.c line 1187
  • #16 gst_pad_set_active
    at gstpad.c line 1056
  • #17 activate_pads
    at gstelement.c line 2690
  • #18 gst_iterator_fold
    at gstiterator.c line 614
  • #19 iterator_activate_fold_with_resync
    at gstelement.c line 2714
  • #20 gst_element_pads_activate
  • #21 gst_element_change_state_func
    at gstelement.c line 2822
  • #22 gst_base_src_change_state
    at gstbasesrc.c line 3813
  • #23 gst_element_change_state
    at gstelement.c line 2604
  • #24 gst_element_set_state_func
    at gstelement.c line 2560
  • #25 gst_bin_element_set_state
    at gstbin.c line 2341
  • #26 gst_bin_change_state_func
    at gstbin.c line 2699
  • #27 gst_element_change_state
    at gstelement.c line 2604
  • #28 gst_element_continue_state
    at gstelement.c line 2314
  • #29 gst_element_change_state
  • #30 gst_element_set_state_func
    at gstelement.c line 2560
  • #31 ??

the crash happen in an app that use gstreamer souphttpsrc element as soon as this code is called

http://cgit.freedesktop.org/gstreamer/gst-plugins-good/tree/ext/soup/gstsouphttpsrc.c?h=1.6#n932

ideas?
Comment 1 Tim-Philipp Müller 2015-11-27 14:42:55 UTC
Could you do

 (gdb) print *pspec

in frame 3 and

 (gdb) print *src

in frame 10?
Comment 2 Tim-Philipp Müller 2015-11-27 14:43:36 UTC
*** Bug 758739 has been marked as a duplicate of this bug. ***
Comment 3 Nicola 2015-11-27 14:58:19 UTC

(In reply to Tim-Philipp Müller from comment #1)
> Could you do
> 
>  (gdb) print *pspec
> 
> in frame 3 and
> 
>  (gdb) print *src
> 
> in frame 10?

sure

(gdb) frame 3
  • #3 param_gtype_validate
    at gparamspecs.c line 1081
  • #10 gst_soup_http_src_session_open
    at gstsouphttpsrc.c line 932
$2 = {element = {parent = {element = {object = {object = {g_type_instance = {
              g_class = 0x94a2dec890}, ref_count = 5, qdata = 0x0}, lock = {
            p = 0x0, i = {0, 0}}, name = 0x94a2deda10 "nvr_rec_src",
          parent = 0x94a2f3eab0, flags = 16448, control_bindings = 0x0,
          control_rate = 100000000, last_sync = 18446744073709551615,
          _gst_reserved = 0x0}, state_lock = {p = 0x94a4b28510, i = {0, 0}},
        state_cond = {p = 0x0, i = {0, 0}}, state_cookie = 2,
        target_state = GST_STATE_PAUSED, current_state = GST_STATE_READY,
        next_state = GST_STATE_PAUSED, pending_state = GST_STATE_PAUSED,
        last_return = GST_STATE_CHANGE_ASYNC, bus = 0x94a4b25ca0,
        clock = 0x0, base_time = 0, start_time = 0, numpads = 1,
        pads = 0x94a4b27800, numsrcpads = 1, srcpads = 0x94a4b27a80,
        numsinkpads = 0, sinkpads = 0x0, pads_cookie = 1, _gst_reserved = {
          0x0, 0x0, 0x0, 0x0}}, srcpad = 0x94a2ded660, live_lock = {p = 0x0,
        i = {0, 0}}, live_cond = {p = 0x0, i = {0, 0}}, is_live = 1,
      live_running = 0, blocksize = 4096, can_activate_push = 1,
      random_access = 0, clock_id = 0x0, segment = {
        flags = GST_SEGMENT_FLAG_NONE, rate = 1, applied_rate = 1,
        format = GST_FORMAT_BYTES, base = 0, offset = 0, start = 0,
        stop = 18446744073709551615, time = 0, position = 0,
        duration = 18446744073709551615, _gst_reserved = {0x0, 0x0, 0x0,
          0x0}}, need_newsegment = 0, num_buffers = -1,
      num_buffers_left = -1, typefind = 0, running = 0, pending_seek = 0x0,
      priv = 0x94a2ded090, _gst_reserved = {0x0 <repeats 20 times>}},
---Type <return> to continue, or q <return> to quit---
    _gst_reserved = {0x0, 0x0, 0x0, 0x0}},
  location = 0x94a586a140 "http://192.168.2.251:55080/stream11?want_rec_quality=
1", redirection_uri = 0x0, redirection_permanent = 0,
  user_agent = 0x94a2ded890 "GStreamer souphttpsrc ", automatic_redirect = 1,
  proxy = 0x0, user_id = 0x94a586a0d0 "admin",
  user_pw = 0x94a586a1b0 "password", proxy_id = 0x0, proxy_pw = 0x0,
  cookies = 0x0, context = 0x94a586a8f0, loop = 0x94a586a710, session = 0x0,
  session_io_status = GST_SOUP_HTTP_SRC_SESSION_IO_STATUS_IDLE, msg = 0x0,
  ret = GST_FLOW_OK, outbuf = 0x0, interrupted = 0, retry = 0,
  retry_count = 0, max_retries = 3, method = 0x0, got_headers = 0,
  have_size = 0, content_size = 0, read_position = 0, seekable = 0,
  request_position = 0, stop_position = 18446744073709551615, have_body = 0,
  keep_alive = 0, ssl_strict = 0, ssl_ca_file = 0x0,
  ssl_use_system_ca_file = 1, tls_database = 0x0, iradio_mode = 1,
  src_caps = 0x0, iradio_name = 0x0, iradio_genre = 0x0, iradio_url = 0x0,
  extra_headers = 0x0, log_level = SOUP_LOGGER_LOG_HEADERS, compress = 0,
  timeout = 15, mutex = {p = 0x0, i = {0, 0}}, request_finished_cond = {
    p = 0x0, i = {0, 0}}, http_headers_event = 0x0}
Comment 4 Nicola 2015-11-27 15:07:58 UTC
this is what I do in my app on souphttpsrc element, probably not relevant

g_object_set(source,"location",this->mMediaUrl.constData(),"is-live",TRUE,"timeout",timeout,NULL);

if (g_object_class_find_property(G_OBJECT_GET_CLASS(source),"ssl-strict") != NULL){
    g_object_set(G_OBJECT(source),"ssl-strict",FALSE,NULL);
}
if (!this->mUsername.isEmpty()){
    g_object_set(source,"user-id",this->mUsername.constData(),NULL);
}

if (!this->mPassword.isEmpty()){
                g_object_set(source,"user-pw",this->mPassword.constData(),NULL);
}
gst_bin_add_many(GST_BIN(this->pipe),source,demuxer,NULL);
if (!gst_element_link_many(source,demuxer,NULL)){
....
Comment 5 Nicola 2015-11-27 15:11:04 UTC
my app "sometimes" works but most of the times crash with the above details
Comment 6 Nicola 2015-11-27 23:10:19 UTC
I wrote a really simple test app:

    gst_init(&argc,&argv);
    GstElement *source, *sink, *pipe;
    pipe = gst_pipeline_new("pipe");
    source = gst_element_factory_make("souphttpsrc","http_src");
    sink = gst_element_factory_make("fakesink","sink");
    if (!source || !sink || !pipe){
        fprintf(stderr,"Errore creazione elementi\n");
        return 1;
    }
    g_object_set(source,"location","http://admin:password@192.168.2.251:12011/stream?want_rec_quality=1","is-live",TRUE,"timeout",(guint)10,"ssl-strict",FALSE,NULL);
    gst_bin_add_many(GST_BIN(pipe),source,sink,NULL);
    if (!gst_element_link_many(source,sink,NULL)){
        fprintf(stderr,"Errore creazione elementi\n");
        return 1;
    }
    gst_element_set_state(pipe,GST_STATE_PLAYING);
    fprintf(stderr,"pipeline avviata\n");

when does not crash it print this warning:

GLib-GObject-WARNING **: value "((gpointer) 000000CB31E0ADB0)" of type 'GType' is invalid or out of range for property 'add-feature-by-type' of type 'GType'
Comment 7 Nicola 2015-11-27 23:24:34 UTC
my fix, probably workaround, was to remove this line from souphttpsrc gstreamer plugin (I don't need a proxy)

http://cgit.freedesktop.org/gstreamer/gst-plugins-good/tree/ext/soup/gstsouphttpsrc.c?h=1.6#n936
Comment 8 Tim-Philipp Müller 2015-11-27 23:26:49 UTC
How about a small test program that just uses libsoup (a stripped-down version of gst_soup_http_src_session_open() with the same values)?
Comment 9 Nicola 2015-11-27 23:31:31 UTC
I can try the next week. However seems quite clear that SOUP_SESSION_ADD_FEATURE_BY_TYPE lead to crash on *some* windows variants, for example it works fine on windows 7 but crash on windows 8 (both x64)
Comment 10 Dan Winship 2015-11-29 16:10:44 UTC
This looks like possibly a gobject-level problem with GType-valued properties.

Is everything (glib, libsoup, gstreamer, your app) compiled with the same toolchain? It looks like the definition of GType is environment-dependent:

  #if     GLIB_SIZEOF_SIZE_T != GLIB_SIZEOF_LONG || !defined __cplusplus
  typedef gsize                           GType;
  #else   /* for historic reasons, C++ links against gulong GTypes */
  typedef gulong                          GType;
  #endif

so if some parts got one definition and other parts got the other, then things would break.
Comment 11 Nicola 2015-11-29 16:24:29 UTC
(In reply to Dan Winship from comment #10)
> This looks like possibly a gobject-level problem with GType-valued
> properties.
> 
> Is everything (glib, libsoup, gstreamer, your app) compiled with the same
> toolchain? It looks like the definition of GType is environment-dependent:
> 
>   #if     GLIB_SIZEOF_SIZE_T != GLIB_SIZEOF_LONG || !defined __cplusplus
>   typedef gsize                           GType;
>   #else   /* for historic reasons, C++ links against gulong GTypes */
>   typedef gulong                          GType;
>   #endif
> 
> so if some parts got one definition and other parts got the other, then
> things would break.

glib, libsoup and gstreamer are compiled using cerbero. My app is compiled using visual studio, this should be supported by gstreamer distribution, Tim?
Comment 12 Tim-Philipp Müller 2015-11-29 16:39:45 UTC
(compiled using cerbero == mingw/gcc fwiw)

Yes, this should work, although there might be interesting issues if you pass file descriptors from your app to GStreamer and things like that, but this is not what's going on here as far as I can tell.

Since both libsoup and souphttpsrc have been compiled using cerbero/mingw, there should be no problem between those two. I'm not aware of any problems with GType and MSVC either, people do that quite a bit, that they build their own applications in Visual Studio and link against the pre-built binaries.

How about a little test app that just does:

  GType type = SOUP_TYPE_PROXY_RESOLVER_DEFAULT;

  g_print ("%d\n", (int) sizeof (GType));
  g_print ("%d\n", (int) sizeof (void *));
  g_print ("%s\n", g_type_name (type));
Comment 13 Nicola 2015-11-29 16:49:41 UTC
(In reply to Tim-Philipp Müller from comment #12)
> (compiled using cerbero == mingw/gcc fwiw)
> 
> Yes, this should work, although there might be interesting issues if you
> pass file descriptors from your app to GStreamer and things like that, but
> this is not what's going on here as far as I can tell.
> 
> Since both libsoup and souphttpsrc have been compiled using cerbero/mingw,
> there should be no problem between those two. I'm not aware of any problems
> with GType and MSVC either, people do that quite a bit, that they build
> their own applications in Visual Studio and link against the pre-built
> binaries.
> 
> How about a little test app that just does:
> 
>   GType type = SOUP_TYPE_PROXY_RESOLVER_DEFAULT;
> 
>   g_print ("%d\n", (int) sizeof (GType));
>   g_print ("%d\n", (int) sizeof (void *));
>   g_print ("%s\n", g_type_name (type));

ok, this app should be compiled with visual studio and not cerbero, right? I'll try later, thanks
Comment 14 Nicola 2015-11-29 20:17:03 UTC
(In reply to Tim-Philipp Müller from comment #12)
> (compiled using cerbero == mingw/gcc fwiw)
> 
> Yes, this should work, although there might be interesting issues if you
> pass file descriptors from your app to GStreamer and things like that, but
> this is not what's going on here as far as I can tell.
> 
> Since both libsoup and souphttpsrc have been compiled using cerbero/mingw,
> there should be no problem between those two. I'm not aware of any problems
> with GType and MSVC either, people do that quite a bit, that they build
> their own applications in Visual Studio and link against the pre-built
> binaries.
> 
> How about a little test app that just does:
> 
>   GType type = SOUP_TYPE_PROXY_RESOLVER_DEFAULT;
> 
>   g_print ("%d\n", (int) sizeof (GType));
>   g_print ("%d\n", (int) sizeof (void *));
>   g_print ("%s\n", g_type_name (type));

seems good:


C:\Users\Administrator>C:\Users\Administrator\Desktop\Condivisa\Deploy-x64\CrashTest.exe
WARNING: no real random source present!
8
8
SoupProxyResolverDefault

and then crash
Comment 15 Nicola 2015-11-29 20:18:08 UTC
Created attachment 316486 [details]
sample app that crash on windows

this is the app that produce the previous output
Comment 16 Nicola 2015-11-29 20:20:28 UTC
Created attachment 316487 [details]
test app that crash on windows
Comment 17 Nicola 2015-11-29 23:05:01 UTC
(In reply to Tim-Philipp Müller from comment #8)
> How about a small test program that just uses libsoup (a stripped-down
> version of gst_soup_http_src_session_open() with the same values)?

If I modify the test program to something like this:

    GMainContext *context = g_main_context_new();
    g_print("try to create soup session\n");
    SoupSession *session = soup_session_async_new_with_options (SOUP_SESSION_ASYNC_CONTEXT,context, SOUP_SESSION_USER_AGENT, "user_agent",                                                           SOUP_SESSION_TIMEOUT, (guint)10,                                                              SOUP_SESSION_SSL_STRICT, FALSE,                                                                SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_PROXY_RESOLVER_DEFAULT,NULL);
    g_print("soup session created %p\n",session);

I can reproduce the crash
Comment 18 Nicola 2015-11-29 23:11:08 UTC
even something more basic like this:

SoupSession *session = soup_session_new_with_options (SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_CONTENT_SNIFFER,NULL);

copied from here:

https://developer.gnome.org/libsoup/stable/libsoup-client-howto.html

will crash.

The crash seem related to SOUP_SESSION_ADD_FEATURE_BY_TYPE
Comment 19 Nicola 2015-11-29 23:25:58 UTC
and finally if I rename the same app from main.cpp to main.c and I cross compile with cerbero this way:

x86_64-w64-mingw32-gcc main.c -o CrashTestCross.exe $(pkg-config --cflags --libs gstreamer-1.0 libsoup-2.4)

no crash happen

C:\Users\Administrator>C:\Users\Administrator\Desktop\Condivisa\Deploy-x64\Crash
TestCross.exe
8
8
SoupProxyResolverDefault
try to create soup session
soup session created 0000000000439120
Comment 20 Nicola 2015-11-30 08:41:33 UTC
and yet more strange if I create a soup session like this:

SoupSession *session = soup_session_async_new_with_options   (SOUP_SESSION_ASYNC_CONTEXT,context, SOUP_SESSION_USER_AGENT,  "user_agent",SOUP_SESSION_TIMEOUT, (guint)10,                                                               SOUP_SESSION_SSL_STRICT, FALSE,                                                                                                                             NULL);

so the same code as in souphttpsrc but without 

SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_PROXY_RESOLVER_DEFAULT

then this code works without crash:

    g_print("session created %p\n",session);
    soup_session_add_feature_by_type(session, SOUP_TYPE_CONTENT_DECODER);
    g_print("added content decoder\n");
    soup_session_remove_feature_by_type(session, SOUP_TYPE_CONTENT_DECODER);
    g_print("removed content decoder\n");

so "add-feature-by-type" this time works

really no more ideas, what do you think about? 

P.S. after my modification in comment 7 my app is running since 54 hours with no crash
Comment 21 Dan Winship 2015-11-30 14:19:59 UTC
(In reply to Nicola from comment #18)
> The crash seem related to SOUP_SESSION_ADD_FEATURE_BY_TYPE

(In reply to Nicola from comment #20)
> so "add-feature-by-type" this time works

Right. The problem is specifically with demarshalling a GType passed to a varargs function. So soup_session_add_feature_by_type() works, but soup_session_new_with_options() doesn't.

(In reply to Nicola from comment #19)
> and finally if I rename the same app from main.cpp to main.c and I cross
> compile with cerbero this way:
> 
> x86_64-w64-mingw32-gcc main.c -o CrashTestCross.exe $(pkg-config --cflags
> --libs gstreamer-1.0 libsoup-2.4)
> 
> no crash happen

So this suggests something along the lines of what I said in comment 10.

Can you add printfs for sizeof(gulong) and the values of GLIB_SIZEOF_SIZE_T and GLIB_SIZEOF_LONG to your test? (It seems like the problem might be that GLIB_SIZEOF_LONG is somehow ending up as 8 but sizeof(gulong) is 4.)
Comment 22 Nicola 2015-11-30 14:33:09 UTC
(In reply to Dan Winship from comment #21)
> (In reply to Nicola from comment #18)
> > The crash seem related to SOUP_SESSION_ADD_FEATURE_BY_TYPE
> 
> (In reply to Nicola from comment #20)
> > so "add-feature-by-type" this time works
> 
> Right. The problem is specifically with demarshalling a GType passed to a
> varargs function. So soup_session_add_feature_by_type() works, but
> soup_session_new_with_options() doesn't.
> 
> (In reply to Nicola from comment #19)
> > and finally if I rename the same app from main.cpp to main.c and I cross
> > compile with cerbero this way:
> > 
> > x86_64-w64-mingw32-gcc main.c -o CrashTestCross.exe $(pkg-config --cflags
> > --libs gstreamer-1.0 libsoup-2.4)
> > 
> > no crash happen
> 
> So this suggests something along the lines of what I said in comment 10.
> 
> Can you add printfs for sizeof(gulong) and the values of GLIB_SIZEOF_SIZE_T
> and GLIB_SIZEOF_LONG to your test? (It seems like the problem might be that
> GLIB_SIZEOF_LONG is somehow ending up as 8 but sizeof(gulong) is 4.)


you are right, this code:

g_print("sizeof(gulong): %ld\n",sizeof(gulong));
g_print("GLIB_SIZEOF_SIZE_T: %d\n",GLIB_SIZEOF_SIZE_T);
g_print("GLIB_SIZEOF_LONG: %d\n",GLIB_SIZEOF_LONG);

print these results:

sizeof(gulong): 4
GLIB_SIZEOF_SIZE_T: 8
GLIB_SIZEOF_LONG: 4
Comment 23 Dan Winship 2015-11-30 15:00:37 UTC
(In reply to Nicola from comment #22)
> sizeof(gulong): 4
> GLIB_SIZEOF_SIZE_T: 8
> GLIB_SIZEOF_LONG: 4

OK, that's *not* what I was thinking before. That's correct and consistent. (I was thinking sizeof(gulong) and GLIB_SIZEOF_LONG might be different.) Given that GLIB_SIZEOF_SIZE_T != GLIB_SIZEOF_LONG, gtype.h will define GType as gsize regardless of C vs C++.

So I don't know exactly what's happening, but *somehow* you're ending up with some sort of mismatch in what different parts of your code think sizeof(GType) is.
Comment 24 Nicola 2015-11-30 15:09:30 UTC
on linux 64 the size is 8. If I cross compile for windows x64 I get exactly the same results posted above for the test app compiled with visual studio.
Comment 25 Nicola 2015-11-30 23:51:28 UTC
the really strange thing is that the same test app works with no crash on windows 7 64bit and crash very often on windows 8 64bit. Both machine have the same visual c++ runtime version. 

My workaround seems to be fine, but to be sure that my app works on all windows versions seems that I have to build everything from cerbero/mingw or use DLLImport

No more ideas?

thanks
Comment 26 Nicola 2015-12-01 08:16:31 UTC
the crash seeems solved using DYNAMICBASE:NO linker flags, this flag is active by default
Comment 27 Nicola 2015-12-01 15:29:16 UTC
I did several tests on different windows versions (7,8,10) and I can confirm that passing DYNAMICBASE:NO to visual studio linker solve the crash on all tested versions. With defaults linker flags, and so DYNAMICBASE enabled, the test app works only if it runs on the same windows version that is used for its compilation.

dynamicbase seems broken and/or not fully working with mingw:

http://sourceforge.net/p/mingw-w64/mailman/mingw-w64-public/thread/CAGFXeQKg0J+ax0Dj-cBXGGrwWT15K-aRsWWaBN6wWSDUtRHoLg@mail.gmail.com/


The crash seems to happen only with varargs.
Comment 28 Sebastian Dröge (slomo) 2016-04-13 16:18:07 UTC
*** Bug 763962 has been marked as a duplicate of this bug. ***
Comment 29 Sebastian Dröge (slomo) 2016-04-14 07:09:15 UTC
Nicola, can you test the patch in bug #765031? If all our guesses are correct, this should at least work around the problem until we really understand it.

Question now is what DYNAMICBASE is exactly doing and why/how it interferes with GType passing via varargs. It might also be fixed by a newer toolchain, we're considering updating the Windows toolchain for GStreamer soonish.
Comment 30 Sebastian Dröge (slomo) 2016-04-14 11:41:03 UTC
Created attachment 325991 [details] [review]
gparamspecs: GTypes are stored in v_pointer, not v_long

v_long is 32 bits on Win64, v_pointer is 64 bits. On most other platforms the
size of long and pointer is the same, so it's usually not a problem.
Comment 31 Sebastian Dröge (slomo) 2016-04-14 11:53:14 UTC
I confirmed that this patch fixes it for the GES case at least.
Comment 32 Nicolas Dufresne (ndufresne) 2016-04-14 14:11:42 UTC
Here the weird conditions around definition of GType. That does not seem right to me, as it means GType size may vary on the same platform dependind if it's C or C++.

#if     GLIB_SIZEOF_SIZE_T != GLIB_SIZEOF_LONG || !defined __cplusplus
typedef gsize                           GType;
#else   /* for historic reasons, C++ links against gulong GTypes */
typedef gulong                          GType;
#endif
Comment 33 Sebastian Dröge (slomo) 2016-04-14 17:00:48 UTC
Yes that looks very bad but is unrelated to this bug fortunately. But basically the GType definition means that it will fail interoperability between C and C++ code on Win64 as gsize and gulong have different sizes.

I assume the current definition has historical reasons. It was gulong in the beginning, then it was changed to gsize, then people noticed that this breaks the C++ ABI (having a gsize will result in different symbol mangling than a gulong I guess), and then this compromise was chosen. But that's just guessing.

However this also means that we can't possibly change anything here now without breaking the C++ ABI. OTOH if we don't change anything, GLib is basically unusable from C++ on Win64.
Comment 34 Sebastian Dröge (slomo) 2016-04-14 17:02:56 UTC
Also I'm not sure why the original problem only broke on Windows 10 with DYNAMICBASE:YES. Why would the GTypes have bits set in the upper 32 bits in that case, but not in the other?
Comment 35 Ignacio Casal Quinteiro (nacho) 2016-04-14 17:06:43 UTC
See that breaking the abi on windows is not such a big deal since you always endup building everything by yourself.
Comment 36 Nicola 2016-04-14 17:13:33 UTC
Hi Sebastian, 

thanks for the patch, tomorrow I'll try to find some time to test it. I'm sure it works, thanks! 

For info I'm using gstreamer from a c++ app (qt based), for me the problem happen on windows 8/10 64 bits and not on windows 7 64 bits. The application was compiled on windows 7 64bits using qtcreator with visual studio compiler, adding DYNAMICBASE:NO seems to solve the issue (not widely tested)
Comment 37 Sebastian Dröge (slomo) 2016-04-14 17:24:10 UTC
It should only break C++ on platforms where sizeof(long) == sizeof(size_t)... and where gsize and gulong are resulting in different symbol mangling although the have the same size.

That's not even Win64 (the sizes are different), and not with g++ in general (the mangling only considers the basic types, not any typedefs). So not sure what it would break... but the code probably has reasons. Someone needs to read the logs :)
Comment 38 Nicola 2016-04-15 08:36:42 UTC
I confirm that the patch fix the reported issue, thanks! 

Do you want any additional test? I have a c++ app that can generate the logs you want on win64 :)
Comment 39 Sebastian Dröge (slomo) 2016-04-18 11:45:01 UTC
What should happen with the commit now that fixes it for Win64? Getting that in would be a good start, then we can worry about the remaining problem (which is also unrelated to Windows).
Comment 40 Allison Karlitskaya (desrt) 2016-04-26 16:18:32 UTC
Review of attachment 325991 [details] [review]:

I'd prefer to introduce a v_uintptr or something to the union and use that.
Comment 41 Sebastian Dröge (slomo) 2016-04-26 16:32:02 UTC
(In reply to Allison Ryan Lortie (desrt) from comment #40)
> Review of attachment 325991 [details] [review] [review]:
> 
> I'd prefer to introduce a v_uintptr or something to the union and use that.

Me too, but that probably would have to wait for next major GLib release then and it would be good to have this crash fixed on Windows before that :)

The way how the patch does it now is the same as elsewhere already:
https://git.gnome.org/browse/glib/tree/gobject/gvaluetypes.c#n1188
https://git.gnome.org/browse/glib/tree/gobject/gvaluetypes.c#n1207


I can prepare an additional patch on top of the above one for master, but I would prefer to have it merged in two steps so that the above patch without new API can be cherry-picked for the next stable release.

Question is always which integer types you all want in GValue. uintptr? size_t? ssize_t? All of them, some subset of them, ...? They can all be different types in theory.
Comment 42 Allison Karlitskaya (desrt) 2016-04-27 07:12:09 UTC
As far as I am concerned, sizeof(gsize) == sizeof(gpointer).  A great number of bad things would happen if this were to stop being the case.  As for the exact type we use in the union, I doubt that it matters -- even for C++.  This union is also semi-private -- people aren't supposed to touch it outside of GLib... even in marshallers, accessing these fields is considered to be slightly evil.

I agree with your strategy, though.  Let's do it your way for now.  Thanks.
Comment 43 Allison Karlitskaya (desrt) 2016-04-27 07:12:48 UTC
Review of attachment 325991 [details] [review]:

Okay.  Go ahead with this one and we'll do the other one later.
Comment 44 Sebastian Dröge (slomo) 2016-04-27 07:46:16 UTC
Attachment 325991 [details] pushed as 4e3cd88 - gparamspecs: GTypes are stored in v_pointer, not v_long
Comment 45 Arnav Singh 2016-05-06 10:10:36 UTC
For everyone who's wondering why it would crash on Windows 8 and above and not on 7, the allocator on 8 and above tends to give 64-bit pointers that have non-zero upper 8 bits more often that the allocator on 7. So truncating a pointer has has a higher chance of breaking it on 8 and above.