diff mbox

[FFmpeg-devel,1/2] checkasm: add sbrdsp tests

Message ID d077a126-0cc9-6871-e54e-43f0dbd08bcb@gmail.com
State Accepted
Commit ac8ad8d0981baee33bc5e9cd3b0a44643971f2e8
Headers show

Commit Message

James Almer June 30, 2017, 1:53 a.m. UTC 
On 6/29/2017 10:14 PM, Henrik Gramner wrote:
> On Fri, Jun 30, 2017 at 1:58 AM, Michael Niedermayer
> <michael@niedermayer.cc> wrote:
>> Program received signal SIGSEGV, Segmentation fault.
>> 0x0000000000684919 in ff_sbr_hf_gen_sse ()
> 
>>    0x0000000000684909 <ff_sbr_hf_gen_sse+25>:   sub    %r9,%r8
> 
>> => 0x0000000000684919 <ff_sbr_hf_gen_sse+41>:   movaps (%rsi,%r8,1),%xmm0
> 
>> r9             0xdeadbeef00000080       -2401053092612145024
> 
> Another case of a 32-bit int being used as part of a 64-bit operation.

I can't reproduce it on my ArchLinux x86_64 environment for some reason,
but based on what you said i assume the attached patch should fix it.
From f4646091b450b7c4c5479fbb4163ef89615a4a8d Mon Sep 17 00:00:00 2001
From: James Almer <jamrial@gmail.com>
Date: Thu, 29 Jun 2017 22:51:04 -0300
Subject: [PATCH] x86/sbrdsp: zero extend start and end gprs in
 ff_sbr_hf_gen_sse

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/x86/sbrdsp.asm | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

Comments

Michael Niedermayer June 30, 2017, 1:55 p.m. UTC | #1 
On Thu, Jun 29, 2017 at 10:53:06PM -0300, James Almer wrote:
> On 6/29/2017 10:14 PM, Henrik Gramner wrote:
> > On Fri, Jun 30, 2017 at 1:58 AM, Michael Niedermayer
> > <michael@niedermayer.cc> wrote:
> >> Program received signal SIGSEGV, Segmentation fault.
> >> 0x0000000000684919 in ff_sbr_hf_gen_sse ()
> > 
> >>    0x0000000000684909 <ff_sbr_hf_gen_sse+25>:   sub    %r9,%r8
> > 
> >> => 0x0000000000684919 <ff_sbr_hf_gen_sse+41>:   movaps (%rsi,%r8,1),%xmm0
> > 
> >> r9             0xdeadbeef00000080       -2401053092612145024
> > 
> > Another case of a 32-bit int being used as part of a 64-bit operation.
> 
> I can't reproduce it on my ArchLinux x86_64 environment for some reason,
> but based on what you said i assume the attached patch should fix it.

no crash occurs here with this, so it seems fixed

thx

[...]
James Almer June 30, 2017, 2:47 p.m. UTC | #2 
On 6/30/2017 10:55 AM, Michael Niedermayer wrote:
> On Thu, Jun 29, 2017 at 10:53:06PM -0300, James Almer wrote:
>> On 6/29/2017 10:14 PM, Henrik Gramner wrote:
>>> On Fri, Jun 30, 2017 at 1:58 AM, Michael Niedermayer
>>> <michael@niedermayer.cc> wrote:
>>>> Program received signal SIGSEGV, Segmentation fault.
>>>> 0x0000000000684919 in ff_sbr_hf_gen_sse ()
>>>
>>>>    0x0000000000684909 <ff_sbr_hf_gen_sse+25>:   sub    %r9,%r8
>>>
>>>> => 0x0000000000684919 <ff_sbr_hf_gen_sse+41>:   movaps (%rsi,%r8,1),%xmm0
>>>
>>>> r9             0xdeadbeef00000080       -2401053092612145024
>>>
>>> Another case of a 32-bit int being used as part of a 64-bit operation.
>>
>> I can't reproduce it on my ArchLinux x86_64 environment for some reason,
>> but based on what you said i assume the attached patch should fix it.
> 
> no crash occurs here with this, so it seems fixed
> 
> thx

Applied then.
Matthieu Bouron June 30, 2017, 3:16 p.m. UTC | #3 
On Fri, Jun 30, 2017 at 03:55:52PM +0200, Michael Niedermayer wrote:
> On Thu, Jun 29, 2017 at 10:53:06PM -0300, James Almer wrote:
> > On 6/29/2017 10:14 PM, Henrik Gramner wrote:
> > > On Fri, Jun 30, 2017 at 1:58 AM, Michael Niedermayer
> > > <michael@niedermayer.cc> wrote:
> > >> Program received signal SIGSEGV, Segmentation fault.
> > >> 0x0000000000684919 in ff_sbr_hf_gen_sse ()
> > > 
> > >>    0x0000000000684909 <ff_sbr_hf_gen_sse+25>:   sub    %r9,%r8
> > > 
> > >> => 0x0000000000684919 <ff_sbr_hf_gen_sse+41>:   movaps (%rsi,%r8,1),%xmm0
> > > 
> > >> r9             0xdeadbeef00000080       -2401053092612145024
> > > 
> > > Another case of a 32-bit int being used as part of a 64-bit operation.
> > 
> > I can't reproduce it on my ArchLinux x86_64 environment for some reason,
> > but based on what you said i assume the attached patch should fix it.
> 
> no crash occurs here with this, so it seems fixed

Should i push the patchset or wait a little bit longer ?

[...]
Matthieu Bouron July 3, 2017, 12:32 p.m. UTC | #4 
On Fri, Jun 30, 2017 at 05:16:37PM +0200, Matthieu Bouron wrote:
> On Fri, Jun 30, 2017 at 03:55:52PM +0200, Michael Niedermayer wrote:
> > On Thu, Jun 29, 2017 at 10:53:06PM -0300, James Almer wrote:
> > > On 6/29/2017 10:14 PM, Henrik Gramner wrote:
> > > > On Fri, Jun 30, 2017 at 1:58 AM, Michael Niedermayer
> > > > <michael@niedermayer.cc> wrote:
> > > >> Program received signal SIGSEGV, Segmentation fault.
> > > >> 0x0000000000684919 in ff_sbr_hf_gen_sse ()
> > > > 
> > > >>    0x0000000000684909 <ff_sbr_hf_gen_sse+25>:   sub    %r9,%r8
> > > > 
> > > >> => 0x0000000000684919 <ff_sbr_hf_gen_sse+41>:   movaps (%rsi,%r8,1),%xmm0
> > > > 
> > > >> r9             0xdeadbeef00000080       -2401053092612145024
> > > > 
> > > > Another case of a 32-bit int being used as part of a 64-bit operation.
> > > 
> > > I can't reproduce it on my ArchLinux x86_64 environment for some reason,
> > > but based on what you said i assume the attached patch should fix it.
> > 
> > no crash occurs here with this, so it seems fixed
> 
> Should i push the patchset or wait a little bit longer ?

Patchset applied.
Michael Niedermayer July 4, 2017, 5:31 p.m. UTC | #5 
On Mon, Jul 03, 2017 at 02:32:28PM +0200, Matthieu Bouron wrote:
> On Fri, Jun 30, 2017 at 05:16:37PM +0200, Matthieu Bouron wrote:
> > On Fri, Jun 30, 2017 at 03:55:52PM +0200, Michael Niedermayer wrote:
> > > On Thu, Jun 29, 2017 at 10:53:06PM -0300, James Almer wrote:
> > > > On 6/29/2017 10:14 PM, Henrik Gramner wrote:
> > > > > On Fri, Jun 30, 2017 at 1:58 AM, Michael Niedermayer
> > > > > <michael@niedermayer.cc> wrote:
> > > > >> Program received signal SIGSEGV, Segmentation fault.
> > > > >> 0x0000000000684919 in ff_sbr_hf_gen_sse ()
> > > > > 
> > > > >>    0x0000000000684909 <ff_sbr_hf_gen_sse+25>:   sub    %r9,%r8
> > > > > 
> > > > >> => 0x0000000000684919 <ff_sbr_hf_gen_sse+41>:   movaps (%rsi,%r8,1),%xmm0
> > > > > 
> > > > >> r9             0xdeadbeef00000080       -2401053092612145024
> > > > > 
> > > > > Another case of a 32-bit int being used as part of a 64-bit operation.
> > > > 
> > > > I can't reproduce it on my ArchLinux x86_64 environment for some reason,
> > > > but based on what you said i assume the attached patch should fix it.
> > > 
> > > no crash occurs here with this, so it seems fixed
> > 
> > Should i push the patchset or wait a little bit longer ?
> 
> Patchset applied.

it seems theres some issue still in this:

checkasm: using random seed 3655967467
MMX:
 - audiodsp.audiodsp             [OK]
 - blockdsp.blockdsp             [OK]
 - h264dsp.idct                  [OK]
 - h264pred.pred4x4              [OK]
 - h264pred.pred8x8              [OK]
 - h264pred.pred16x16            [OK]
 - pixblockdsp.get_pixels        [OK]
 - pixblockdsp.diff_pixels       [OK]
 - vp8dsp.idct                   [OK]
 - vp8dsp.mc                     [OK]
 - vp9dsp.ipred                  [OK]
 - vp9dsp.itxfm                  [OK]
 - vp9dsp.mc                     [OK]
MMXEXT:
 - audiodsp.audiodsp             [OK]
 - h264dsp.idct                  [OK]
 - h264pred.pred4x4              [OK]
 - h264pred.pred8x8              [OK]
 - h264pred.pred16x16            [OK]
 - h264pred.pred8x8l             [OK]
 - h264qpel.put                  [OK]
 - h264qpel.avg                  [OK]
 - hevc_add_res.add_residual     [OK]
 - hevc_idct.idct_dc             [OK]
 - vp8dsp.mc                     [OK]
 - vp9dsp.ipred                  [OK]
 - vp9dsp.itxfm                  [OK]
 - vp9dsp.loopfilter             [OK]
 - vp9dsp.mc                     [OK]
SSE:
 - aacpsdsp.add_squares          [OK]
 - aacpsdsp.mul_pair_single      [OK]
 - aacpsdsp.hybrid_analysis      [OK]
 - sbrdsp.sum64x5                [OK]
 - sbrdsp.sum_square             [OK]
 - sbrdsp.neg_odd_64             [OK]
 - sbrdsp.qmf_post_shuffle       [OK]
 - sbrdsp.qmf_deint_neg          [OK]
 - sbrdsp.qmf_deint_bfly         [OK]
 - sbrdsp.autocorrelate          [OK]
 - sbrdsp.hf_gen                 [OK]
 - sbrdsp.hf_g_filt              [OK]
 - audiodsp.audiodsp             [OK]
 - blockdsp.blockdsp             [OK]
 - fmtconvert.fmtconvert         [OK]
 - h264pred.pred16x16            [OK]
 - vp8dsp.idct                   [OK]
 - vp8dsp.mc                     [OK]
 - vp9dsp.ipred                  [OK]
 - vp9dsp.mc                     [OK]
 - float_dsp.vector_fmul         [OK]
 - float_dsp.vector_fmac         [OK]
 - float_dsp.butterflies_float   [OK]
 - float_dsp.scalarproduct_float [OK]
SSE2:
 - sbrdsp.qmf_pre_shuffle        [OK]
 - sbrdsp.qmf_deint_bfly         [OK]

Program received signal SIGSEGV, Segmentation fault.
apply_noise_main.loop () at libavcodec/x86/sbrdsp.asm:418
418         movu       m7, [Yq + 2*count + mmsize]
(gdb) bt
Python Exception <type 'exceptions.ImportError'> No module named gdb.frames:
#0  apply_noise_main.loop () at libavcodec/x86/sbrdsp.asm:418
#1  0x000000000043659b in checkasm_checked_call () at tests/checkasm/x86/checkasm.asm:77
#2  0xdeadbeefdeadbeef in ?? ()
#3  0xdeadbeefdeadbeef in ?? ()
#4  0xdeadbeefdeadbeef in ?? ()
#5  0xdeadbeefdeadbeef in ?? ()
#6  0xdeadbeefdeadbeef in ?? ()
#7  0xdeadbeefdeadbeef in ?? ()
#8  0xdeadbeefdeadbeef in ?? ()
#9  0xdeadbeefdeadbeef in ?? ()
#10 0xdeadbeefdeadbeef in ?? ()
#11 0xdeadbeefdeadbeef in ?? ()
#12 0xdeadbeefdeadbeef in ?? ()
#13 0xdeadbeefdeadbeef in ?? ()
#14 0xdeadbeefdeadbeef in ?? ()
#15 0xdeadbeefdeadbeef in ?? ()
#16 0xdeadbeefdeadbeef in ?? ()
#17 0xdeadbeefdeadbeef in ?? ()
#18 0xdeadbeefdeadbeef in ?? ()
#19 0x00007fffffffd870 in ?? ()
#20 0x00007fffffffcc70 in ?? ()
#21 0x00007fffffffce70 in ?? ()
#22 0x0000000000000000 in ?? ()
(gdb) info all-registers
rax            0x0      0
rbx            0xed56bb2dcb3c7736       -1344681633365854410
rcx            0x8e8    2280
rdx            0x7ab77bbbffffd070       8842672440749314160
rsi            0x7ab77bbbffffce70       8842672440749313648
rdi            0xf56e7777ffffdc70       -761539929699263376
rbp            0x8bda43d3fd1a7e06       0x8bda43d3fd1a7e06
rsp            0x7fffffffcae8   0x7fffffffcae8
r8             0xdeadbeef00000000       -2401053092612145152
r9             0x85490444000009c0       -8842531703260968512
r10            0x684bf0 6835184
r11            0x1      1
r12            0x4a75479abd64e097       5365273261009854615
r13            0x249214109d5d1c88       2635190793557318792
r14            0xb64a9c9e5d318408       -5311260606547786744
r15            0xdf9a54b303f1d3a3       -2334460328996121693
rip            0x684cc9 0x684cc9 <apply_noise_main.loop+105>
eflags         0x10206  [ PF IF RF ]
cs             0x33     51
ss             0x2b     43
ds             0x0      0
es             0x0      0
fs             0x0      0
gs             0x0      0
st0            -nan(0x0fffb0005)        (raw 0xffff00000000fffb0005)
st1            -nan(0x334fe50ff28fc84)  (raw 0xffff0334fe50ff28fc84)
st2            -nan(0x0ff640150)        (raw 0xffff00000000ff640150)
st3            -nan(0x0005e005a)        (raw 0xffff00000000005e005a)
st4            -nan(0x0ff5bffe7)        (raw 0xffff00000000ff5bffe7)
st5            -nan(0xff63fc2cfe94fee5) (raw 0xffffff63fc2cfe94fee5)
st6            -nan(0x01c4df38a)        (raw 0xffff000000001c4df38a)
st7            -nan(0x06215436f)        (raw 0xffff000000006215436f)

[...]
diff mbox

Patch

diff --git a/libavcodec/x86/sbrdsp.asm b/libavcodec/x86/sbrdsp.asm
index d0f774b277..c716184b14 100644
--- a/libavcodec/x86/sbrdsp.asm
+++ b/libavcodec/x86/sbrdsp.asm
@@ -149,19 +149,19 @@  cglobal sbr_hf_gen, 4,4,8, X_high, X_low, alpha0, alpha1, BW, S, E
     ; start and end 6th and 7th args on stack
     mov        r2d, Sm
     mov        r3d, Em
-%define  start r2q
-%define  end   r3q
+    DEFINE_ARGS X_high, X_low, start, end
 %else
 ; BW does not actually occupy a register, so shift by 1
-%define  start BWq
-%define  end   Sq
+    DEFINE_ARGS X_high, X_low, alpha0, alpha1, start, end
+    movsxd  startq, startd
+    movsxd    endq, endd
 %endif
-    sub      start, end          ; neg num of loops
-    lea    X_highq, [X_highq + end*2*4]
-    lea     X_lowq, [X_lowq  + end*2*4 - 2*2*4]
-    shl      start, 3            ; offset from num loops
+    sub     startq, endq         ; neg num of loops
+    lea    X_highq, [X_highq + endq*2*4]
+    lea     X_lowq, [X_lowq  + endq*2*4 - 2*2*4]
+    shl     startq, 3            ; offset from num loops
 
-    mova        m0, [X_lowq + start]
+    mova        m0, [X_lowq + startq]
     shufps      m3, m3, q1111
     shufps      m4, m4, q1111
     xorps       m3, [ps_mask]
@@ -169,7 +169,7 @@  cglobal sbr_hf_gen, 4,4,8, X_high, X_low, alpha0, alpha1, BW, S, E
     shufps      m2, m2, q0000
     xorps       m4, [ps_mask]
 .loop2:
-    movu        m7, [X_lowq + start + 8]        ; BbCc
+    movu        m7, [X_lowq + startq + 8]       ; BbCc
     mova        m6, m0
     mova        m5, m7
     shufps      m0, m0, q2301                   ; aAbB
@@ -179,12 +179,12 @@  cglobal sbr_hf_gen, 4,4,8, X_high, X_low, alpha0, alpha1, BW, S, E
     mulps       m6, m2
     mulps       m5, m1
     addps       m7, m0
-    mova        m0, [X_lowq + start +16]        ; CcDd
+    mova        m0, [X_lowq + startq + 16]      ; CcDd
     addps       m7, m0
     addps       m6, m5
     addps       m7, m6
-    mova  [X_highq + start], m7
-    add     start, 16
+    mova  [X_highq + startq], m7
+    add     startq, 16
     jnz         .loop2
     RET