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 698520 - audioconvert unit test failure with latest orc bytecode stuff
audioconvert unit test failure with latest orc bytecode stuff
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: orc
git master
Other Linux
: Normal blocker
: 1.1.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-04-21 16:37 UTC by Tim-Philipp Müller
Modified: 2013-08-20 12:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
bytecode: Fix 64bit value parsing (1.58 KB, patch)
2013-08-19 14:11 UTC, Edward Hervey
reviewed Details | Review

Description Tim-Philipp Müller 2013-04-21 16:37:58 UTC
This only happens for me with my checkout that is build against glib git (infrequently updated). Not sure it's related to that, or why it would be, but it's the main difference to my other checkouts and branches where the test passes without problems. All checkouts use the same orc version (0.4.17 in /usr/lib).

Running suite(s): audioconvert

Converted data:
00000000 (0x1cfa360): 00 80 00 80 00 80 00 80 00 80 00 80 00 80        ..............  

Expected data:
00000000 (0x7fff0c11f030): 00 00 ff 7f 00 80 00 40 00 c0 ff 7f 00 80        .......@......  

87%: Checks: 8, Failures: 1, Errors: 0
gstcheck.c:406:F:general:test_float_conversion:0: buffer contents not equal
Comment 1 Tim-Philipp Müller 2013-04-21 16:48:42 UTC
However, it works also fine with ORC_CODE=backup , which undermines the hypothesis that this is related to glib in any  way.
Comment 2 Sebastian Dröge (slomo) 2013-04-22 07:46:43 UTC
Also with glib 2.36.0
Comment 3 Sebastian Dröge (slomo) 2013-04-22 07:48:57 UTC
This looks like a rounding difference btw... but I'm not sure how a GLib change could cause problems here.
Comment 4 Sebastian Dröge (slomo) 2013-04-24 08:04:45 UTC
Doesn't fail for me with -DDISABLE_ORC or ORC_CODE=backup. Fails with ORC_CODE=emulate.
Comment 5 Sebastian Dröge (slomo) 2013-04-24 08:11:03 UTC
Works with orc 0.4.16 in all combinations
Comment 6 Sebastian Dröge (slomo) 2013-04-24 08:20:11 UTC
Also the unit tests fail with orc master but not with orc 0.4.17 (audioconvert test fails with 0.4.17 too), e.g. orc/audio:

Program received signal SIGSEGV, Segmentation fault.
_backup_audio_orc_unpack_u8 () at orc/audio.c:137
137	    var38.i = (orc_uint16)var37.i;
(gdb) bt
  • #0 _backup_audio_orc_unpack_u8
    at orc/audio.c line 137
  • #1 orc_test_compare_output_full
    at orctest.c line 662
  • #2 main
    at orc/audio.c line 1285

Comment 7 Sebastian Dröge (slomo) 2013-04-24 08:24:29 UTC
Also the unit tests fail with orc master but not with orc 0.4.17 (audioconvert test fails with 0.4.17 too), e.g. orc/audio:

Program received signal SIGSEGV, Segmentation fault.
_backup_audio_orc_unpack_u8 () at orc/audio.c:137
137	    var38.i = (orc_uint16)var37.i;
(gdb) bt
  • #0 _backup_audio_orc_unpack_u8
    at orc/audio.c line 137
  • #1 orc_test_compare_output_full
    at orctest.c line 662
  • #2 main
    at orc/audio.c line 1285

Comment 8 Sebastian Dröge (slomo) 2013-04-24 08:27:11 UTC
It's also not related to the new static bytecode mechanism, changing that #if 1 to an #if 0 in tmp-orc.c does not change anything at all
Comment 9 Sebastian Dröge (slomo) 2013-04-24 08:50:50 UTC
Actually it is, git bisect blames:

commit 34139bfbb286829b469825e01664c3d34058f63b
Author: David Schleef <ds@schleef.org>
Date:   Sat Jan 28 14:18:11 2012 -0800

    bytecode: Add bytecode parsing


Also generating code with --compat 0.4.16 makes it work.
Comment 10 Sebastian Dröge (slomo) 2013-04-24 08:58:36 UTC
And the problem is in the bytecode part of audio_convert_orc_unpack_double_s32 (and the swap variant too). Changing tmp-orc.c to not use bytecode for these two functions makes the test succeed.
Comment 11 Edward Hervey 2013-08-19 13:14:18 UTC
If it helps, here are the memdups before/after using audio_convert_orc_unpack_double_s32 

before unpack
00000000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f0 3f  ...............?
00000010: 00 00 00 00 00 00 f0 bf 00 00 00 00 00 00 e0 3f  ...............?
00000020: 00 00 00 00 00 00 e0 bf 9a 99 99 99 99 99 f1 3f  ...............?
00000030: 9a 99 99 99 99 99 f1 bf                          ........

ORC_CODE=backup
after unpack
00000000: 00 00 00 00 ff ff ff 7f 02 00 00 80 00 00 00 40  ...............@
00000010: 01 00 00 c0 ff ff ff 7f 00 00 00 80              ............


ORC_CODE= (failing mode)
after unpack
00000000: 00 00 00 80 00 00 00 80 00 00 00 80 00 00 00 80  ................
00000010: 00 00 00 80 00 00 00 80 00 00 00 80              ............

I tried to find how to dump the compiled resulting ASM, but couldn't find any method for that.
Comment 12 Edward Hervey 2013-08-19 13:40:57 UTC
The first few instructions of the generated code show the difference:

Using bytecode (i.e. default and failling):
   0x7ffff7fe6000:	stmxcsr 0x254(%rdi)
   0x7ffff7fe6007:	mov    0x254(%rdi),%ecx
   0x7ffff7fe600d:	mov    %ecx,0x258(%rdi)
   0x7ffff7fe6013:	or     $0x8040,%ecx
   0x7ffff7fe6019:	mov    %ecx,0x254(%rdi)
   0x7ffff7fe601f:	ldmxcsr 0x254(%rdi)
   0x7ffff7fe6026:	mov    $0xffc00000,%ecx
   0x7ffff7fe602b:	mov    %ecx,0x118(%rdi)
   0x7ffff7fe6031:	mov    $0xffffffff,%ecx
   0x7ffff7fe6036:	mov    %ecx,0x11c(%rdi)
   0x7ffff7fe603c:	movq   0x118(%rdi),%xmm0
   0x7ffff7fe6044:	pshufd $0x44,%xmm0,%xmm0


Note using bytecode (successs)
   0x7ffff7fe6000:	stmxcsr 0x254(%rdi)
   0x7ffff7fe6007:	mov    0x254(%rdi),%ecx
   0x7ffff7fe600d:	mov    %ecx,0x258(%rdi)
   0x7ffff7fe6013:	or     $0x8040,%ecx
   0x7ffff7fe6019:	mov    %ecx,0x254(%rdi)
   0x7ffff7fe601f:	ldmxcsr 0x254(%rdi)
   0x7ffff7fe6026:	mov    $0xffc00000,%ecx
   0x7ffff7fe602b:	mov    %ecx,0x118(%rdi)
   0x7ffff7fe6031:	mov    $0x41dfffff,%ecx
   0x7ffff7fe6036:	mov    %ecx,0x11c(%rdi)
   0x7ffff7fe603c:	movq   0x118(%rdi),%xmm0
   0x7ffff7fe6044:	pshufd $0x44,%xmm0,%xmm0


The difference is at 0x7ffff7fe6031 (mov    $0x41dfffff,%ecx  got replaced somehow by mov    $0xffffffff,%ecx)

Looks like it's the constant value from "orc_program_add_constant_int64 (p, 8, 0x41dfffffffc00000ULL, "c1");" ... that got badly stored in the bytecode somehow.
Comment 13 Edward Hervey 2013-08-19 14:11:25 UTC
Created attachment 252231 [details] [review]
bytecode: Fix 64bit value parsing

So it's one of those "the code should have worked, but for some mystical
reason it doesn't" bugs where the output of _get_uint64() returned corrupted
values.

I propose an alternative fix that just parses two 32 bit values and returns
the concatenated result... which works.
Comment 14 Edward Hervey 2013-08-19 14:44:10 UTC
An alternative fix is to always cast (and not just for the last 4 shifts)
Comment 15 Sebastian Dröge (slomo) 2013-08-19 15:04:35 UTC
commit 3f98bc77a8a6056dc7674aa48c683eea2bd0c5b8
Author: Sebastian Dröge <slomo@circular-chaos.org>
Date:   Mon Aug 19 17:02:24 2013 +0200

    orcbytecode: Fix parsing of 64 bit values from bytecode
    
    The shift by 24 bits has to be casted already, otherwise we shift
    into the sign bit which causes undefined behaviour.
    
    Thanks to Edward Hervey for debugging this.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=698520