GNOME Bugzilla – Bug 797092
opusenc: segmentation fault
Last modified: 2018-09-24 14:27:21 UTC
When i run "gst-launch-1.0.exe audiotestsrc ! audioconvert ! opusenc ! fakesink" results in segmentation fault. Output: Setting pipeline to PAUSED ... Pipeline is PREROLLING ... Redistribute latency... Segmentation fault This command works fine with other encoders, such as avenc_g722 and wavenc.
Works for me, with the binary shipped from the GStreamer website and Win 10.
Thanks for taking the time to report this. This bug report isn't very useful because it doesn't describe the bug well. If you have time and can still reproduce the bug, please read https://bugzilla.gnome.org/page.cgi?id=bug-writing.html and add a description of how to reproduce this bug. You'll also need to add a stack trace; please see https://wiki.gnome.org/Community/GettingInTouch/Bugzilla/GettingTraces for more information about how to do so. When providing a better description and pasting a stack trace, please reset the status of this bug report from NEEDINFO to its previous status. Thanks in advance!
Created attachment 373592 [details] Last lines of trace file I have it tested on 2 computers running Windows 10 and got Segfault error on both PCs. I've added the last few lines from the trace file (GST_DEBUG=8) as an attachment. It seems the error always occurs after that allocation query.
Comment on attachment 373592 [details] Last lines of trace file Thanks for that, but I think we were hoping for a stack trace from a debugger such as gdb or the visual studio debugger (if you're using the GStreamer-made binaries you may have to use gdb).
Or anything that may help reproducing the issue. - Where did you get your build from ? - Did you build it yourself, if so how ? - Which Windows flavour are you running ? (32 vs 64 bit, version, etc) For my comment, I have spawned a Windows 10 64bit VM, downloaded and installed: https://gstreamer.freedesktop.org/data/pkg/windows/1.14.2/gstreamer-1.0-x86_64-1.14.2.msi Open a command line, move to the bin directory of GStreamer and ran the exact command you provided, and that worked.
I've got the build from gstreamer download page (X86, didn't test the X86_64 version). I'm running Windows 10 64 bit. I didn't manage to debug the libs using gdb on Windows so I have built gstreamer using the gst-build. I also built the libopus from these sources: https://github.com/xiph/opus. Now I'm having an error when linking audioconvert to opusenc. This is the trace of the error: * gstopus.dll!gstopusenc.c(879): gst_opus_enc_get_sink_template_caps() ... 878 s = gst_structure_copy (s2); 879 gst_structure_set (s, "channels", G_TYPE_INT, i, "channel-mask", 880 GST_TYPE_BITMASK, 0x0, NULL); * gstreamer-1.0-0.dll!gststructure.c(638): gst_structure_set(_GstStructure * structure, const char * field, ...) ... 637 va_start (varargs, field); 638 gst_structure_set_valist_internal (structure, field, varargs); 639 va_end (varargs); * gstreamer-1.0-0.dll!gststructure.c(607): gst_structure_set_valist_internal(_GstStructure * structure, const char * fieldname, char * varargs) ... 607 G_VALUE_COLLECT_INIT (&field.value, type, varargs, 0, &err); * gvaluecollector.h(92): #define G_VALUE_COLLECT_INIT(value, _value_type, var_args, flags, __error) ... 92 GTypeValueTable *g_vci_vtab = g_type_value_table_peek (_value_type); * gobject-2.0.dll!gtype.c(4235): g_type_value_table_peek (GType type) ... 4234 as_refed_data = node && node->data && NODE_REFCOUNT (node) > 0; 4235 has_table = has_refed_data && node->data->common.value_table->value_init; 4236 if (has_refed_data) The error is: Exception thrown: read access violation. node->data->common.value_table was 0x53EC8B55. Now I'm not sure if this error is the same that I got before from the gstreamer release binaries or if it's a new error caused by my build.
Ok, I only tested the 64bit version.
I think the error is happening in the gst_structure_set_valist_internal function of gststructure.c. There's a while loop that reads the args from varargs until it finds "NULL". while (fieldname) { ... type = va_arg (varargs, GType); // gets type ... G_VALUE_COLLECT_INIT (&field.value, type, varargs, 0, &err); // gets value ... fieldname = va_arg (varargs, gchar *); // gets next field from vargs } The varargs is: va_list varargs = G_TYPE_INT, 2, "channel-mask", GST_TYPE_BITMASK, 0x0, NULL He should read: type = G_TYPE_INT value = 2 fieldname = "channel-mask" type = GST_TYPE_BITMASK value = 0 fielname = NULL //should end here, but it's not what's happening Here is the memory dump from Visual Studio. va_list varargs = G_TYPE_INT, 2, "channel-mask", GST_TYPE_BITMASK, 0x0, NULL address of varargs = 0x00EFF924 Memory: 0x00EFF924 18 00 00 00 //ok: G_TYPE_INT == 0x18 0x00EFF928 02 00 00 00 //ok: value == (int)2 0x00EFF92C 20 d9 02 53 //ok: address of string with "channel-mask" 0x00EFF930 e0 00 00 00 //ok: GST_TYPE_BITMASK == 0xe0 0x00EFF934 00 00 00 00 //ok: 0x0 is supposed to have 4 bytes 0x00EFF938 00 00 00 00 //not ok: this should be the "NULL" field, but is the continuation of the last value (0x0), it's being read as a gint64(8 bytes) 0x00EFF93C b0 90 1e 00 //not ok: he skipped the NULL then caused va_list "overflow", get random string at this address, then crash at next iteration ... 0x5302D920 63 68 61 6e 6e 65 6c 2d 6d 61 73 6b 00 //"channel-mask" 0x001E90B0 ... // random stuff So either the GST_TYPE_BITMASK value should have 8 bytes and the va_list is not putting the NULL value at the end, or the va_list is putting the NULL value at the end and GST_TYPE_BITMASK should have 4 bytes.
Oh yeah, I'm dumb. All you have to do is casting to assign the 0x0 value to a guint64 variable... and it's already being done in the same function: guint64 channel_mask = 0; ... 867 gst_structure_set (s, "channels", G_TYPE_INT, i, "channel-mask", 868 GST_TYPE_BITMASK, channel_mask, NULL); ... 872 gst_structure_set (s, "channels", G_TYPE_INT, i, "channel-mask", 873 GST_TYPE_BITMASK, channel_mask, NULL); ... // change 0x0 to channel_mask so it won't be interpreted as 4 bytes int by the compiler 879 gst_structure_set (s, "channels", G_TYPE_INT, i, "channel-mask", 880 GST_TYPE_BITMASK, 0x0, NULL); ... // same here 884 gst_structure_set (s, "channels", G_TYPE_INT, i, "channel-mask", 885 GST_TYPE_BITMASK, 0x0, NULL);
This is in the gst_opus_enc_get_sink_template_caps function at gstopusenc.c
(In reply to Marcos Kintschner from comment #8) > I think the error is happening in the gst_structure_set_valist_internal > function of gststructure.c. > > There's a while loop that reads the args from varargs until it finds "NULL". > > while (fieldname) { > ... > type = va_arg (varargs, GType); // gets type > ... > G_VALUE_COLLECT_INIT (&field.value, type, varargs, 0, &err); // gets value > ... > fieldname = va_arg (varargs, gchar *); // gets next field from vargs > } > > The varargs is: > va_list varargs = G_TYPE_INT, 2, "channel-mask", GST_TYPE_BITMASK, 0x0, NULL > > > He should read: > type = G_TYPE_INT > value = 2 > fieldname = "channel-mask" > type = GST_TYPE_BITMASK > value = 0 > fielname = NULL //should end here, but it's not what's happening Good catch, GST_TYPE_BITMASK is a 64bit type, 0x0 is 32bit, so we'll be off by 32bit, reading after the NULL sentinel. Can be fixed with "G_GUINT64_CONSTANT (0)", do you wan to provide a patch ?
Created attachment 373650 [details] [review] x86 segfault fix Sure, no problem :) Feel free to change anything if necessary.
Review of attachment 373650 [details] [review]: Looks good, thanks.
Thanks a lot, I'll backport to 1.14 in a minute.
Tim, do you think we could have Windows32/64 in Hardware section ?
> do you think we could have Windows32/64 in Hardware section ? I can add it if you like, but that field is pretty much unused and not very useful anyway IMHO (it's often wrong because it defaults to whatever OS was used by the user filing the bug, or set to Linux for bugs that affect All), and we don't really differentiate between that for the other architectures, like ARM. This bug is also not really Win32-specific, but will likely be a problem on all 32-bit architectures in the right circumstances (depending on compiler, compiler version, compiler flags, potluck), no?
True. Better to just take the habit to ask the.
It's going to happen on all 32 bit architectures where the ABI says that varargs arguments can be smaller than 64 bits (which is probably all of them).
Yes, but the reporter does not know that before end. But he knows he's running Windows 32bit binary. Again, I guess we just need to ask to save a little time.
The being said, I downloaded 1.14.3 (both 32bit and 64bit), and ran it over wine. The 64bit version works, while the 32bit version still crash. Unhandled exception: page fault on read access to 0xffffffff in 32-bit code (0x62513c6b). Register dump: CS:0023 SS:002b DS:002b ES:002b FS:0063 GS:006b EIP:62513c6b ESP:00a16c84 EBP:00a19f5c EFLAGS:00010206( R- -- I - -P- ) EAX:00a17ee0 EBX:00000348 ECX:00000004 EDX:00a18fd0 ESI:0000006c EDI:00000078 Stack dump: 0x00a16c84: 0079aa24 000001e0 a1f53300 0079a938 0x00a16c94: 0079aa24 0079a938 0079aa24 62506478 0x00a16ca4: 00a18fd0 00a17ee0 0000006c 00000348 0x00a16cb4: be30a000 bdfa0800 bd955800 00000000 0x00a16cc4: 00000000 62543bc0 00000078 00000004 0x00a16cd4: 436390ce 4381f330 43906439 439cd43f Backtrace: =>0 0x62513c6b comb_filter_const_sse+0x1b() [d:\projects\cerbero\autotools\1.14\build\sources\windows_x86\opus-1.2.1/d:/projects/cerbero/autotools/1.14/build/mingw/w32/bin/../lib/gcc/i686-w64-mingw32/4.7.3/include/xmmintrin.h:866] in libopus-0 (0x00a19f5c) 1 0x62506478 celt_encode_with_ec+0x987(frame_size=<is not available>, nbCompressedBytes=0x9f, enc=0xa1b48c) [d:\projects\cerbero\autotools\1.14\build\sources\windows_x86\opus-1.2.1/celt/celt_encoder.c:1217] in libopus-0 (0x00a19f5c) 2 0x62535fab opus_encode_native+0x222a(data="", lsb_depth=0x100) [d:\projects\cerbero\autotools\1.14\build\sources\windows_x86\opus-1.2.1/src/opus_encoder.c:2054] in libopus-0 (0x00a1b69c) 3 0x625395af opus_multistream_encode_native+0x76e(st=<internal error>, copy_channel_in=0x62538120, pcm=0x790080, data="", max_data_bytes=0xa0) [d:\projects\cerbero\autotools\1.14\build\sources\windows_x86\opus-1.2.1/src/opus_multistream_encoder.c:1073] in libopus-0 (0x00a1f99c) 4 0x62539fe4 opus_multistream_encode+0x43() [d:\projects\cerbero\autotools\1.14\build\sources\windows_x86\opus-1.2.1/src/opus_multistream_encoder.c:1194] in libopus-0 (0x000003c0) 5 0x64445c28 gst_opus_enc_handle_frame+0x567() [d:\projects\cerbero\autotools\1.14\build\sources\windows_x86\gst-plugins-base-1.0-1.14.3\ext\opus/gstopusenc.c:1041] in libgstopus (0x000003c0) 6 0x64bae689 gst_audio_encoder_push_buffers+0x378(enc=0x783268, force=0) [d:\projects\cerbero\autotools\1.14\build\sources\windows_x86\gst-plugins-base-1.0-1.14.3\gst-libs\gst\audio/gstaudioencoder.c:1119] in libgstaudio-1.0-0 (0x00000000) 7 0x64baf76c gst_audio_encoder_chain+0x87b(buffer=0x772030) [d:\projects\cerbero\autotools\1.14\build\sources\windows_x86\gst-plugins-base-1.0-1.14.3\gst-libs\gst\audio/gstaudioencoder.c:1349] in libgstaudio-1.0-0 (0x00772030) 8 0x61488849 gst_pad_chain_data_unchecked+0x248(pad=0x76a200, type=4112, data=0x772030) [d:\projects\cerbero\autotools\1.14\build\sources\windows_x86\gstreamer-1.0-1.14.3\gst/gstpad.c:4322] in libgstreamer-1.0-0 (0x00783268) 9 0x6148c65b gst_pad_push_data+0x5aa(pad=0x76a0a8, type=4112, data=0x772030) [d:\projects\cerbero\autotools\1.14\build\sources\windows_x86\gstreamer-1.0-1.14.3\gst/gstpad.c:4578] in libgstreamer-1.0-0 (0x0076a0b4) 10 0x61492dbb gst_pad_push+0xea() [d:\projects\cerbero\autotools\1.14\build\sources\windows_x86\gstreamer-1.0-1.14.3\gst/gstpad.c:4697] in libgstreamer-1.0-0 (0x00772030) 11 0x6b5f56e0 gst_base_src_loop+0xd3f() [d:\projects\cerbero\autotools\1.14\build\sources\windows_x86\gstreamer-1.0-1.14.3\libs\gst\base/gstbasesrc.c:2957] in libgstbase-1.0-0 (0x0077f1dc) 12 0x614c0eae gst_task_func+0x10d() [d:\projects\cerbero\autotools\1.14\build\sources\windows_x86\gstreamer-1.0-1.14.3\gst/gsttask.c:332] in libgstreamer-1.0-0 (0x0076a0f4) 13 0x68618a98 g_thread_pool_thread_proxy+0x177() [d:\projects\cerbero\autotools\1.14\build\sources\windows_x86\glib-2.54.3\glib/gthreadpool.c:307] in libglib-2.0-0 (0x0025ee10) 14 0x686181bd in libglib-2.0-0 (+0x581bc) (0x00a1feec) 15 0x6863581d g_thread_win32_proxy+0xc() [d:\projects\cerbero\autotools\1.14\build\sources\windows_x86\glib-2.54.3\glib/gthread-win32.c:450] in libglib-2.0-0 (0x00a1feec) 16 0x7bcac188 call_thread_func_wrapper+0xb() in ntdll (0x00a1feec) 17 0x7bcafb21 in ntdll (+0x6fb20) (0x00a1ffdc) 18 0x7bcac17a call_thread_exit_func+0x31() in ntdll (0x00a1ffec) 0x62513c6b comb_filter_const_sse+0x1b [d:\projects\cerbero\autotools\1.14\build\sources\windows_x86\opus-1.2.1/d:/projects/cerbero/autotools/1.14/build/mingw/w32/bin/../lib/gcc/i686-w64-mingw32/4.7.3/include/xmmintrin.h:866] in libopus-0:
GDB backtrace, I'll need to rebuild using cerbero to get the source: (gdb) bt
+ Trace 238692
Created attachment 373739 [details] [review] opus: Force stack alignment on sse optimized functions This fixes a crash on Win32. Our old GCC 4.7 isn't smart enough to do it by itself. On Win32, the stack alignment is 4 bytes, while GCC assumes 16 bytes when using __m128 vectors.
This was tested on 1.14 branch. For master, we should just move forward and get rid of this old GCC 4.7 mingw.
Attachment 373739 [details] pushed as bd99c49 - opus: Force stack alignment on sse optimized functions
I'll try and get #775777 working in master, worst case it's easy to add this to master too.
I was going to report that the crash still occurs for me. I've built the sources again with gst-build and the crash was happening on libopus-0.dll (but I could not debug it because it was built with gcc). So I've used the libopus.dll that I have built the last time and it worked fine (because I was using this new compiled libopus.dll, I didn't catch the error when I submitted the patch, sorry).
For the record, when you need to debug, these pre-built GDB executable worked great for me: http://www.equation.com/servlet/equation.cmd?fa=gdb
Nice, thanks! I'll take a look.
The patch is for libopus but gst-build would only rebuild the GStreamer plugin, and the patch is also not merged into master (yet).
(In reply to Sebastian Dröge (slomo) from comment #29) > The patch is for libopus but gst-build would only rebuild the GStreamer > plugin, and the patch is also not merged into master (yet). And what's your request ? The reporter followed our instructions and showed that our delivered build was broken too (we are no responsible for other builds of libopus that may have the same issue). For the master part, read comment #26 please.
I was just wondering if Marcos rebuilt only things with gst-build (and assumed that contains the fix), and because of that he still sees the problem.
Ok, I think Marcos meant that he didn't report because he has replaced libopus locally and forgot about it. There was of course two issue, the generic 32bit bug we fixed in 1.14.3, but then the second is the bad build generated by us, which is worked around now. The master fix will take more time, but moving to from the shelf mingw really cleans up the mess in cerbero, so I'm looking forward this way.