Ticket #12584 (closed Bugs: Fixed)

Opened 3 years ago

Last modified 21 months ago

Crash in zoom_filter_mmx /usr/lib/xbmc/addons/visualization.goom/Goom.vis

Reported by: rgom Owned by:
Priority: 4 - Normal Milestone: 12.1 "Frodo"
Component: Audio Visualization Version: 11.0 "Eden" Beta 2
Severity: Normal Keywords:
Cc: jmarshall, spiff, Blocked By:
Blocking: Platform: Linux
Revision:

Description

After some playing with Xbmc and probably setting up the Goom visualization, it crashes.

Reproducible: always

Steps to reproduce:

  1. (Probably) Set up goom visualization plugin
  2. Try to play music

Observed behaviour: crash Expected behaviour: no crash, music plays, visualization works

$ apt-cache policy xbmc xbmc:

Installed: 11.0~beta2-0.3 Candidate: 11.0~beta2-0.3 Version table:

* 11.0~beta2-0.3 0

500  http://www.debian-multimedia.org/ sid/main i386 Packages 100 /var/lib/dpkg/status

Attachments

xbmc_crashlog-20120204_154302.log Download (20.7 KB) - added by rgom 3 years ago.
Crash log

Change History

Changed 3 years ago by rgom

Crash log

comment:1 Changed 3 years ago by rgom

After recompiling Goom.vis, without stripping:

Program terminated with signal 11, Segmentation fault. #0 0xac91a34b in zoom_filter_mmx (prevX=512, prevY=512, expix1=0xaaefc080, expix2=0xaadfb080, brutS=0xa48fc080, brutD=0xa44fa080, buffratio=127, precalCoef=0xaf34d160) at mmx.c:56 56 asm volatile (

Well, this is assembler code, I am not good at all with that ...

comment:2 Changed 3 years ago by rgom

Crashes on rc1/rc2 as well.

comment:3 Changed 3 years ago by rgom

An attempt to debug assembler ...

0xaf626339 <+169>: pmullw %mm6,%mm0 0xaf62633c <+172>: pmullw %mm3,%mm1 0xaf62633f <+175>: paddw %mm1,%mm0 0xaf626342 <+178>: movq %mm4,%mm5 0xaf626345 <+181>: punpcklbw %mm7,%mm4 0xaf626348 <+184>: punpckhbw %mm7,%mm5

---Type <return> to continue, or q <return> to quit--- => 0xaf62634b <+187>: add 0x8(%ebp),%ebp

0xaf62634e <+190>: movq (%esi,%ebp,4),%mm1 0xaf626352 <+194>: movq %mm1,%mm2 0xaf626355 <+197>: punpcklbw %mm7,%mm1 0xaf626358 <+200>: punpckhbw %mm7,%mm2 0xaf62635b <+203>: pmullw %mm4,%mm1 0xaf62635e <+206>: pmullw %mm5,%mm2 0xaf626361 <+209>: paddw %mm1,%mm0 0xaf626364 <+212>: paddw %mm2,%mm0 0xaf626367 <+215>: psrlw $0x8,%mm0 0xaf62636b <+219>: packuswb %mm7,%mm0 0xaf62636e <+222>: movd %mm0,(%ecx)

(gdb) info registers eax 0x0 0 ecx 0xa8bfe080 -1463820160 edx 0x0 0 ebx 0xaf63d5bc -1352411716 esp 0xbfaf8b7c 0xbfaf8b7c ebp 0x0 0x0 esi 0xa8cff080 -1462767488 edi 0xa506c080 -1526284160 eip 0xaf62634b 0xaf62634b <zoom_filter_mmx+187> eflags 0x210246 [ PF ZF IF RF ID ] cs 0x73 115 ss 0x7b 123 ds 0x7b 123 es 0x7b 123 fs 0x0 0 gs 0x33 51

So EBP for some reason is null.

comment:4 Changed 3 years ago by rgom

   0xaf626339 <+169>:   pmullw %mm6,%mm0
   0xaf62633c <+172>:   pmullw %mm3,%mm1
   0xaf62633f <+175>:   paddw  %mm1,%mm0
   0xaf626342 <+178>:   movq   %mm4,%mm5
   0xaf626345 <+181>:   punpcklbw %mm7,%mm4
   0xaf626348 <+184>:   punpckhbw %mm7,%mm5
---Type <return> to continue, or q <return> to quit---
=> 0xaf62634b <+187>:   add    0x8(%ebp),%ebp
   0xaf62634e <+190>:   movq   (%esi,%ebp,4),%mm1
   0xaf626352 <+194>:   movq   %mm1,%mm2
   0xaf626355 <+197>:   punpcklbw %mm7,%mm1
   0xaf626358 <+200>:   punpckhbw %mm7,%mm2
   0xaf62635b <+203>:   pmullw %mm4,%mm1
   0xaf62635e <+206>:   pmullw %mm5,%mm2
   0xaf626361 <+209>:   paddw  %mm1,%mm0
   0xaf626364 <+212>:   paddw  %mm2,%mm0
   0xaf626367 <+215>:   psrlw  $0x8,%mm0
   0xaf62636b <+219>:   packuswb %mm7,%mm0
   0xaf62636e <+222>:   movd   %mm0,(%ecx)
(gdb) info  registers 
eax            0x0      0
ecx            0xa8bfe080       -1463820160
edx            0x0      0
ebx            0xaf63d5bc       -1352411716
esp            0xbfaf8b7c       0xbfaf8b7c
ebp            0x0      0x0
esi            0xa8cff080       -1462767488
edi            0xa506c080       -1526284160
eip            0xaf62634b       0xaf62634b <zoom_filter_mmx+187>
eflags         0x210246 [ PF ZF IF RF ID ]
cs             0x73     115
ss             0x7b     123
ds             0x7b     123
es             0x7b     123
fs             0x0      0
gs             0x33     51

comment:5 Changed 3 years ago by rgom

Yay! I've fixed it :) At least in my configuration. Patch:

diff --git a/xbmc/visualizations/Goom/goom2k4-0/src/mmx.c b/xbmc/visualizations/Goom/goom2k4-0/src/mmx.c
index fdf0649..8effa52 100644                                                                                                                                 
--- a/xbmc/visualizations/Goom/goom2k4-0/src/mmx.c                                                                                                            
+++ b/xbmc/visualizations/Goom/goom2k4-0/src/mmx.c                                                                                                            
@@ -91,7 +91,7 @@ void zoom_filter_mmx (int prevX, int prevY,                                                                                                 
                "punpckhbw %%mm7, %%mm5 \n\t"   /* 00-c4-00-c4-00-c4-00-c4 */                                                                                 
                                                                                                                                                              
                /* ajouter la longueur de ligne a esi */                                                                                                      
-               "addl 8(%%ebp),%1 \n\t"                                                                                                                       
+               "addl %4,%1 \n\t"                                                                                                                             
                                                                                                                                                              
                /* recuperation des 2 derniers pixels */                                                                                                      
                "movq (%3,%1,4), %%mm1 \n\t"                                                                                                                  
@@ -115,7 +115,7 @@ void zoom_filter_mmx (int prevX, int prevY,                                                                                               
                                                                                                                                                              
                "movd %%mm0,%0 \n\t"                                                                                                                          
                  :"=g"(expix2[loop])                                                                                                                         
-                 :"r"(pos),"r"(coeffs),"r"(expix1)                                                                                                           
+                 :"r"(pos),"r"(coeffs),"r"(expix1),"r"(prevX)                                                                                                
                                                                                                                                                              
                );                                                                                                                                            
 

The patch is to avoid EBP and/or function argument and use local variable explicitely.

comment:6 Changed 3 years ago by sho

  • Cc jmarshall, spiff, added

Is this something we want to handle before release?

comment:7 Changed 2 years ago by Martijn

bug fixed or is the patch still needed for Frodo?

comment:8 Changed 21 months ago by Github Janitor

  • Status changed from new to closed
  • Resolution set to Fixed

[visualizations] fix crash in goom fixes #12584

Changeset: d1196435876904453d49b93ce5cefa6903489162 By: Martijn Kaijser

comment:9 Changed 21 months ago by Github Janitor

Merge pull request #2146 from MartijnKaijser/12584

[visualizations] fix crash in goom fixes #12584

Changeset: 2493d303739da297870b2178bf1da647ac1cfe35 By: Martijn Kaijser

comment:10 Changed 21 months ago by Martijn

  • Milestone changed from Future / Pending to 12.1

comment:11 Changed 21 months ago by Github Janitor

[visualizations] fix crash in goom fixes #12584

Changeset: 32b276166ac78fca053d604ce9e40bd415fa029a By: Martijn Kaijser

Note: See TracTickets for help on using tickets.