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 779319 - Register size hard-coded to zero in orc_x86_emit_*() functions causes SIGSEGV (x86)
Register size hard-coded to zero in orc_x86_emit_*() functions causes SIGSEGV...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: orc
0.4.26
Other other
: Normal major
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-02-27 15:24 UTC by Igor Rondarev
Modified: 2017-02-28 10:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (1.01 KB, patch)
2017-02-28 09:41 UTC, Igor Rondarev
committed Details | Review

Description Igor Rondarev 2017-02-27 15:24:22 UTC
This bug (behavior?) was detected under GStreamer 1.10.3 and Orc 0.4.26 built for QNX 6.5.0 SP1 operating system. Maybe in other OSes it's masked by sprintf() or strlen() fuction, dunno.

Steps to reproduce (at least at x86 platform with SSE support):
1) Build orc, GStreamer, gst-plugins-base with orc support.
2) Run gst-launch with videotestsrc (e.g. gst-launch videotestsrc ! fakesink):

igor@irpc:~/PROJECTS$ ntox86-gdb /opt/gstreamer/x86/bin/gst-launch-1.0
---cut---
(gdb) run videotestsrc ! fakesink
Starting program: /opt/gstreamer/x86/bin/gst-launch-1.0 videotestsrc ! fakesink
Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...
[New pid 733225 tid 2]

Program received signal SIGSEGV, Segmentation fault.
[Switching to pid 733225 tid 2]
0xb0361298 in strlen () from /opt/qnx650/target/qnx6/x86/lib/libc.so.3
(gdb) sharedlibrary 
(gdb) bt
  • #0 strlen
    from /opt/qnx650/target/qnx6/x86/lib/libc.so.3
  • #1 _Putfld
    from /opt/qnx650/target/qnx6/x86/lib/libc.so.3
  • #2 _Printf
    from /opt/qnx650/target/qnx6/x86/lib/libc.so.3
  • #3 sprintf
    from /opt/qnx650/target/qnx6/x86/lib/libc.so.3
  • #4 orc_x86_insn_output_asm
    at /home/src/orc-0.4.26/orc/orcx86insn.c line 483
  • #5 orc_x86_output_insns
    at /home/src/orc-0.4.26/orc/orcx86insn.c line 923
  • #6 orc_compiler_sse_assemble
    at /home/src/orc-0.4.26/orc/orcprogram-sse.c line 996
  • #7 orc_program_compile_full
    at /home/src/orc-0.4.26/orc/orccompiler.c line 352
  • #8 orc_program_compile_for_target
    at /home/src/orc-0.4.26/orc/orccompiler.c line 173
  • #9 orc_program_compile
    at /home/src/orc-0.4.26/orc/orccompiler.c line 150
  • #10 video_test_src_orc_splat_u32
    at tmp-orc.c line 200
  • #11 paint_tmpline_AYUV
    at /home/src/gst-plugins-base-1.10.3/gst/videotestsrc/videotestsrc.c line 1144
  • #12 gst_video_test_src_smpte
    at /home/src/gst-plugins-base-1.10.3/gst/videotestsrc/videotestsrc.c line 350
  • #13 gst_video_test_src_fill
    at /home/src/gst-plugins-base-1.10.3/gst/videotestsrc/gstvideotestsrc.c line 1022
  • #14 gst_push_src_fill
    at /home/src/gstreamer-1.10.3/libs/gst/base/gstpushsrc.c line 167
  • #15 gst_base_src_default_create
    at /home/src/gstreamer-1.10.3/libs/gst/base/gstbasesrc.c line 1486
  • #16 gst_push_src_create
    at /home/src/gstreamer-1.10.3/libs/gst/base/gstpushsrc.c line 132
  • #17 gst_base_src_get_range
    at /home/src/gstreamer-1.10.3/libs/gst/base/gstbasesrc.c line 2464
  • #18 gst_base_src_loop
    at /home/src/gstreamer-1.10.3/libs/gst/base/gstbasesrc.c line 2740
  • #19 gst_task_func
    at /home/src/gstreamer-1.10.3/gst/gsttask.c line 334
  • #20 default_func
    at /home/src/gstreamer-1.10.3/gst/gsttaskpool.c line 68
  • #21 g_thread_pool_thread_proxy
    at /home/igor/PROJECTS/extra_components/extra/src/glib-2.44.1/glib/gthreadpool.c line 307
  • #22 g_thread_proxy
    at /home/igor/PROJECTS/extra_components/extra/src/glib-2.44.1/glib/gthread.c line 764

What's inside OrcX86Insn structure (see backtrace line #4):

(gdb) print *(OrcX86Insn*)0x812d2d0
$1 = {opcode_index = ORC_X86_push, opcode = 0xb88049b8, imm = 0, src = 37, dest = 37, size = 0, label = 0, type = 0, offset = 0, index_reg = 0, shift = 0, code_offset = 0}

(NOTICE: size == 0)

So, seems that the main cause of this bug is that OrcX86Insn structure for ORC_X86_push opcode has its 'size' field value set to zero.

These fuctions (for some reason - i don't know if it's right or wrong because i'm not an expert in Orc at all) ignore 'size' argument and explicitly set opcode's size to 0. So they emit 'push' opcode to OrcCompiler's list with zero size:

(orc/orcx86.c:127)
void
orc_x86_emit_push (OrcCompiler *compiler, int size, int reg)
{
  orc_x86_emit_cpuinsn_size (compiler, ORC_X86_push, 0, reg, reg);
}

void
orc_x86_emit_pop (OrcCompiler *compiler, int size, int reg)
{
  orc_x86_emit_cpuinsn_size (compiler, ORC_X86_pop, 0, reg, reg); 
}

But later, it's used here as an argument of orc_x86_get_regname_size() function (orc/orcx86insn.c:483):

-------- cut ----------
    case ORC_X86_INSN_TYPE_REGM_REG:
    case ORC_X86_INSN_TYPE_STACK:
      sprintf(op2_str, "%%%s", orc_x86_get_regname_size (xinsn->dest,
      xinsn->size));
      break;  
-------- cut ----------

In this case, function orc_x86_get_regname_size() returns NULL and causes sprintf (at lease in QNX's libc) to cause SIGSEGV.

My fix for this issue (no deep testing yet, but it seems reasonable and at least it works for me):
void

orc_x86_emit_push (OrcCompiler *compiler, int size, int reg)
{
/*  orc_x86_emit_cpuinsn_size (compiler, ORC_X86_push, 0, reg, reg); */
  orc_x86_emit_cpuinsn_size (compiler, ORC_X86_push, size, reg, reg);
}

void
orc_x86_emit_pop (OrcCompiler *compiler, int size, int reg)
{
/*  orc_x86_emit_cpuinsn_size (compiler, ORC_X86_pop, 0, reg, reg); */
  orc_x86_emit_cpuinsn_size (compiler, ORC_X86_pop, size, reg, reg);
}

Regards, Igor.
Comment 1 Sebastian Dröge (slomo) 2017-02-27 15:43:51 UTC
Thanks for finding this. Does the testsuite work for you after this? Also would you mind providing a "git format-patch" style patch with an appropriate commit message and your name/mail address? :)
Comment 2 Igor Rondarev 2017-02-27 16:20:23 UTC
Sure, i'll check (right moment to start using testsuites :) If it'll pass, i'll prepare an appropriate patch. But, again, it's rather strange that this issue was dormant during such a long time and hasn't been detected earlier on other platforms (far more popular than QNX).
Comment 3 Igor Rondarev 2017-02-28 09:41:52 UTC
Created attachment 346892 [details] [review]
proposed patch
Comment 4 Igor Rondarev 2017-02-28 09:44:39 UTC
I've checked it with 'make check' both in QNX 6.5.0 SP1 and Linux, no errors (10 of 10 passed).
Comment 5 Sebastian Dröge (slomo) 2017-02-28 10:43:10 UTC
Thanks for the patch! :)

commit 249c709e90458ecef7a068df553ea41a83eedf0f
Author: Igor Rondarev <igor.rondarev@gmail.com>
Date:   Tue Feb 28 12:23:55 2017 +0300

    orcx86: Don't hard-code register size to zero in orc_x86_emit_*() functions
    
    Instead use the size passed as argument. Fixes segmentation fault on QNX.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=779319