Bug 94662

Summary: amarok crashes reproducable when changing id3 tags of certain mp3s
Product: taglib Reporter: Tilman Keskinöz <tilman>
Component: generalAssignee: Scott Wheeler <wheeler>
Status: RESOLVED FIXED    
Severity: crash    
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: FreeBSD Ports   
OS: FreeBSD   
Latest Commit: Version Fixed In:
Attachments: truncated sample of an mp3 that causes a taglib crash on my system

Description Tilman Keskinöz 2004-12-08 12:27:45 UTC
Version:           CVS as of 2004-08-12 (using KDE KDE 3.3.1)
Installed from:    FreeBSD Ports
Compiler:          gcc version 3.4.2 [FreeBSD] 20040728 
OS:                FreeBSD

Btw, changing the tags in Konqueror does work.
I am using taglib-1.3.1

Here is the backtrace:
Program terminated with signal 10, Bus error.

#0  0x000000080293d460 in TagLib::ID3v2::Tag::removeFrame (this=0xc12140,
    frame=0x1195240, del=true) at stl_list.h:130
No locals.
#1  0x000000080293d4c4 in TagLib::ID3v2::Tag::removeFrames (this=0xc12140,
    id=@0x119a000) at stl_list.h:130
        it = {_M_node = 0x13d00c0}
        l = {_vptr$List = 0x802a5d0f0, d = 0x13d0040}
#2  0x000000080293d8f6 in TagLib::ID3v2::Tag::setAlbum (this=0xc12140,
    s=@0x7fffffffd8f0) at id3v2tag.cpp:218
No locals.
#3  0x000000080293a9e7 in TagLib::MPEGTag::setAlbum (this=0xcdcf40, s=@0x7fffffffd8f0)
    at mpegfile.cpp:136
No locals.
#4  0x00000000004f81fd in TagDialog::writeTag (this=0x7fffffffd8f0, mb=
      {static Undetermined = <optimized out>, static Irrelevant = <optimized out>, static Unavailable = <optimized out>, static null = {static Undetermined = <optimized out>, static Irrelevant = <optimized out>, static Unavailable = <optimized out>, static null = <same as static member of an already seen type>, m_url = {m_strProtocol = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_strUser = {
static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_strPass = {static null = {static null = <same as static member of an already seen type>,d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_strHost = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_strPath = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_strRef_encoded = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_strQuery_encoded = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null =0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_bIsMalformed = true, m_iUriMode = KURL::Auto, freeForUse = 0, m_iPort = 0, m_strPath_encoded = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, d = 0x0}, m_title = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_artist = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}
, m_album = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_year = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_comment = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_genre = {static null = {static null = <same as static memberof an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_track = {static null = {static null = <same as staticmember of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_streamName = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_streamUrl = {static null = {staticnull = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_bitrate = 0, m_length =0, m_sampleRate = 0}, m_url = {m_strProtocol = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d= 0x7126c0, static shared_null = 0x70f800}, m_strUser = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_strPass = {static null = {static
 null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_strHost = {static null= {static null = <same as static member of an already seen type>, d = 0x70f800, staticshared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_strPath = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0xcdcea0, static shared_null = 0x70f800}, m_strRef_encoded = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_strQuery_encoded = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_bIsMalformed = false, m_iUriMode = KURL::URL, freeForUse = 0, m_iPort = 0, m_strPath_encoded = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, d = 0x8023cb17d}, m_title = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null= 0x70f800}, d = 0x14c9c20, static shared_null = 0x70f800}, m_artist = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0xb64400, static shared_null = 0x70f800}, m_album = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x14cee00, static shared_null = 0x70f800}, m_year = {s
tatic null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x8f91a0, static shared_null = 0x70f800}, m_comment = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x14c7ce0, static shared_null = 0x70f800}, m_genre = {static null = {static null = <same as static member of an already seentype>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x14e8880, static shared_null= 0x70f800}, m_track = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x884040, static shared_null = 0x70f800}, m_streamName = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800,static shared_null = 0x70f800}, m_streamUrl = {static null = {static null = <same as static member of an already seen type>, d = 0x70f800, static shared_null = 0x70f800}, d = 0x70f800, static shared_null = 0x70f800}, m_bitrate = 128, m_length = 184, m_sampleRate = 44100}, updateCB=true) at qmemarray.h:77
        t = (class TagLib::Tag *) 0xcdcf40
        result = 64
#5  0x00000000004fa349 in TagDialog::saveTags (this=0xc33400) at tagdialog.cpp:38
        it = {node = 0x10dba00}
#6  0x00000000004fa41e in TagDialog::accept (this=0xc33400) at tagdialog.cpp:82
No locals.
#7  0x00000000004fb264 in TagDialog::qt_invoke (this=0xc33400, _id=51,
    _o=0x7fffffffdca0) at tagdialog.moc:99
No locals.
#8  0x0000000802177948 in QObject::activate_signal ()
   from /usr/X11R6/lib/libqt-mt.so.3
No symbol table info available.
#9  0x00000008021780b2 in QObject::activate_signal ()
   from /usr/X11R6/lib/libqt-mt.so.3
No symbol table info available.
#10 0x00000008021a9859 in QWidget::event () from /usr/X11R6/lib/libqt-mt.so.3
No symbol table info available.
#11 0x000000080211ee6d in QApplication::internalNotify ()
   from /usr/X11R6/lib/libqt-mt.so.3
No symbol table info available.
#12 0x000000080211f39e in QApplication::notify () from /usr/X11R6/lib/libqt-mt.so.3
No symbol table info available.
#13 0x00000008019446de in KApplication::notify (this=0x7fffffffe740,
    receiver=0xc40100, event=0x7fffffffe070) at kapplication.cpp:495
        key = {m_sym = 9191624, m_mod = 0}
        _selectAll = (const KShortcut &) @0x801bbdb08: {m_nSeqs = 1, m_rgseq = {{
---Type <return> to continue, or q <return> to quit---
      m_nKeys = 1 '\001', m_bTriggerOnRelease = 0 '\0', m_rgvar = {{m_sym = 97,
          m_mod = 2}, {m_sym = 0, m_mod = 0}, {m_sym = 0, m_mod = 0}, {m_sym = 0,
          m_mod = 0}}, d = 0x801bbf540}, {m_nKeys = 0 '\0',
      m_bTriggerOnRelease = 0 '\0', m_rgvar = {{m_sym = 0, m_mod = 0}, {m_sym = 0,
          m_mod = 0}, {m_sym = 0, m_mod = 0}, {m_sym = 0, m_mod = 0}},
      d = 0x8032b7b02}}, d = 0x2}
        edit = (struct QLineEdit *) 0x1
        medit = (struct QTextEdit *) 0x1
        t = MouseButtonRelease
#14 0x00000008020c4237 in QETWidget::translateMouseEvent ()
   from /usr/X11R6/lib/libqt-mt.so.3
No symbol table info available.
#15 0x00000008020c2ef8 in QApplication::x11ProcessEvent ()
   from /usr/X11R6/lib/libqt-mt.so.3
No symbol table info available.
#16 0x00000008020d3e4b in QEventLoop::processEvents ()
   from /usr/X11R6/lib/libqt-mt.so.3
No symbol table info available.
#17 0x0000000802132f65 in QEventLoop::enterLoop () from /usr/X11R6/lib/libqt-mt.so.3
No symbol table info available.
Comment 1 Tilman Keskinöz 2004-12-08 14:04:18 UTC
Markus Brueffer reminded me to add that, this is an amd64 system:

% uname -a
FreeBSD mchammer.arved.de 6.0-CURRENT FreeBSD 6.0-CURRENT #9: Tue Dec  7 10:10:38 CET 2004     root@mchammer.arved.de:/usr/obj/usr/src/sys/MCHAMMER  amd64
Comment 2 Tilman Keskinöz 2004-12-08 16:09:46 UTC
Adding the following Observations:

Markus Brueffer was able to edit the tags of the same file on a i386. After editing, he sent the file back to me, and now i am able to edit the file too.

So it looks like a combination of a 64-bit issue and certain tags.

The tag that makes amarok crash looks like this in hexedit:
% hd Sertab_Erener-Here_I_am.mp3 | more
00000000  49 44 33 03 00 00 00 00  0b 2a 54 52 43 4b 00 00  |ID3......*TRCK..|
00000010  00 01 00 00 00 54 45 4e  43 00 00 00 01 40 00 00  |.....TENC....@..|
00000020  57 58 58 58 00 00 00 02  00 00 00 00 54 43 4f 50  |WXXX........TCOP|
00000030  00 00 00 01 00 00 00 54  4f 50 45 00 00 00 01 00  |.......TOPE.....|
00000040  00 00 54 43 4f 4d 00 00  00 01 00 00 00 43 4f 4d  |..TCOM.......COM|
00000050  4d 00 00 00 05 00 00 00  00 08 00 00 54 43 4f 4e  |M...........TCON|
00000060  00 00 00 08 00 00 00 28  31 33 29 50 6f 70 54 59  |.......(13)PopTY|
00000070  45 52 00 00 00 01 00 00  00 54 41 4c 42 00 00 00  |ER.......TALB...|
00000080  01 00 00 00 54 50 45 31  00 00 00 13 00 00 00 30  |....TPE1.......0|
00000090  38 20 2d 20 53 65 72 74  61 62 20 45 72 65 6e 65  |8 - Sertab Erene|
000000a0  72 54 49 54 32 00 00 00  0a 00 00 00 48 65 72 65  |rTIT2.......Here|
000000b0  20 49 20 41 6d 00 00 00  00 00 00 00 00 00 00 00  | I Am...........|
000000c0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
000005b0  00 00 00 00 ff fb 92 40  00 00 02 4f 08 d0 69 89  |.......@

The editable tag looks like this:
hd Sertab_Erener-Here_I_am.mp3| more
00000000  49 44 33 04 00 00 00 00  0b 2a 54 52 43 4b 00 00  |ID3......*TRCK..|
00000010  00 02 00 00 00 38 54 49  54 32 00 00 00 0a 00 00  |.....8TIT2......|
00000020  00 48 65 72 65 20 49 20  41 6d 54 50 45 31 00 00  |.Here I AmTPE1..|
00000030  00 0e 00 00 00 53 65 72  74 61 62 20 45 72 65 6e  |.....Sertab Eren|
00000040  65 72 54 44 52 43 00 00  00 05 00 00 00 32 30 30  |erTDRC.......200|
00000050  34 54 43 4f 4e 00 00 00  03 00 00 00 31 33 00 00  |4TCON.......13..|
00000060  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
000005b0  00 00 00 00 ff fb 92 40  00 00 02 4f 08 d0 69 89  |.......@
Comment 3 Mark Kretschmann 2004-12-09 09:03:47 UTC
Looks to me like a TagLib bug, as it crashes in TagLib::ID3v2::Tag::removeFrame. I'm reassigning this to TagLib. 

Scott, if you think this is not a TagLib bug, please assign it back to us. Cheers.
Comment 4 Tilman Keskinöz 2004-12-11 11:49:59 UTC
Okay, I upgraded to taglib CVS, but the bug is still present.

Just for the record another sample that triggers a crash:

00000000  49 44 33 04 00 00 00 00  09 20 54 49 54 32 00 00  |ID3...... TIT2..|
00000010  00 1b 00 00 00 42 61 62  79 20 6f 6e 65 20 6d 6f  |.....Baby one mo|
00000020  72 65 20 74 69 6d 65 20  2d 20 52 65 6d 69 78 54  |re time - RemixT|
00000030  50 45 31 00 00 00 0f 00  00 00 42 72 69 74 6e 65  |PE1.......Britne|
00000040  79 20 53 70 65 61 72 73  54 41 4c 42 00 00 00 1e  |y SpearsTALB....|
00000050  00 00 00 55 70 65 64 20  62 79 20 41 75 73 74 69  |...Uped by Austi|
00000060  6e 31 20 77 77 77 2e 73  70 65 61 72 73 2e 64 65  |n1 www.spears.de|
00000070  54 44 52 43 00 00 00 05  00 00 00 31 39 39 39 43  |TDRC.......1999C|
00000080  4f 4d 4d 00 00 00 14 00  00 00 20 20 20 00 6d 70  |OMM.......   .mp|
00000090  33 68 69 74 7a 2e 74 73  78 2e 6f 72 67 54 43 4f  |3hitz.tsx.orgTCO|
000000a0  4e 00 00 00 03 00 00 00  31 33 00 00 00 00 00 00  |N.......13......|
000000b0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
Comment 5 Tilman Keskinöz 2004-12-11 12:35:26 UTC
Okay, i commented out "delete *it" from void ID3v2::Tag::removeFrame in id3v2tag.cpp and the tag editing seems to work now.

So why does it fail for some tags?
Comment 6 Tilman Keskinöz 2005-01-23 22:06:49 UTC
Created attachment 9238 [details]
truncated sample of an mp3 that causes a taglib crash on my system

Scott asked for the first 10 KB of a sample file
Comment 7 Scott Wheeler 2005-01-23 22:13:01 UTC
Hmm.  And you can reproduce the crash with this sample?  Here it works just fine...
Comment 8 Tilman Keskinöz 2005-01-23 22:36:21 UTC
Yes, this is reproducable with todays amarok-CVS, 
Maybe it works for you because you are running i386 and not amd64?
Comment 9 Scott Wheeler 2005-01-23 22:55:15 UTC
Quite possibly.  There have been a couple of weird bugs with x86_64 (I actually have one, but run in 32-bit mode.).  It may even be some interaction of the libc on FBSD and x86_64.  I've already worked around a couple of glibc quirks on x86_64.

Another thing you might try is doing an unoptimized build of TagLib and see if the backtrace comes out the same.
Comment 10 Tilman Keskinöz 2005-01-24 00:04:13 UTC
The unoptimized build crashes too, but i haven't got a backtrace, because i first need to disable the new amarok crashhandler, which i suspect to be the cause why i am getting unusable .core files.

If you have any other idea what i might want to try, please followup. Thanks again for your work!
Comment 11 Gunnar Roth 2005-02-02 19:33:28 UTC
This i a bug.

replace 

  if(del)
     delete *it
by

  if(del)
    delete frame;


after doing  d->frameListMap[frame->frameID()].erase(it);
it is invalid, because the list has changed, but not the iterator. It points to 
the element after the erased element.
So  delete *it actually deletes not the earased element but it successor.
On next iteration of the calling ID3v2::Tag::removeFrames method,
an already deleted element is used.

But as we already have a nice pointer for the to be deletd element 
we can simply call "delete frame".



Comment 12 Scott Wheeler 2005-02-02 20:05:05 UTC
CVS commit by wheeler: 

Nice catch from Gunnar Roth -- the iterator was invalid here.

BUG:94662


  M +1 -1      id3v2tag.cpp   1.49


--- kdesupport/taglib/mpeg/id3v2/id3v2tag.cpp  #1.48:1.49
@@ -321,5 +321,5 @@ void ID3v2::Tag::removeFrame(Frame *fram
   // ...and delete as desired
   if(del)
-    delete *it;
+    delete frame;
 }