Version: 1.4 (using KDE KDE 3.4.3) Installed from: Debian stable Packages OS: Linux I have an mp3 file that causes several apps to hang up when trying to display the tags. I've tracked it down to what looks like a bad tag header. The tag size is greater than the file size. When the size is used, the reader goes off to never return. I thnk I've fixed the problem with the following... void ID3v2::Tag::read() { if(d->file && d->file->isOpen()) { d->file->seek(d->tagOffset); d->header.setData(d->file->readBlock(Header::size())); // if the tag size is 0, then this is an invalid tag (tags must contain at // least one frame) if(d->header.tagSize() == 0) return; //fix begin if(d->header.tagSize() > ulong(d->file->length())) { debug("ID3v2::Tag::read() - tagSize does not fit within size of file!" ); d->header.setTagSize(0); return; } //fix end parse(d->file->readBlock(d->header.tagSize())); } }
FYI...I picked Debian from the OS list, because Ubuntu was not in the list.
I verified the hang with the tagreader example. I verified that the id3lib id3info example and XMMS do not have a problem with this file.
Created attachment 13864 [details] Fix infinite loop in ID3v2::Tag::read() with bad header
Judging from the source, readBlock() shouldn't hang waiting for a larger tag than normal, so it looks like the parse() call is what is actually hanging. So I would like to see what is causing parse() to hang and fix it there in case an invalid tag with a small enough header is read and trips the bug that way. If TagLib was compiled with debugging information it would be nice to get a backtrace of the application when it is frozen in TagLib. There is a nice HOWTO at http://ktown.kde.org/~wheeler/freeze.html Otherwise if you could email the smallest file with the invalid tag to michael.pyne (AT) kdemail.net I could try to figure it out.
Let me know if this is not what you asked for...I'm new to linux application debugging. Let me know if you want the mp3...maybe I can split it at the first frame to cut the size down. Thanks in advance!! /bin/sh -c /usr/bin/libtool gdb tagreader -fullname -nx -quiet (gdb) set edit off (gdb) set confirm off *** Warning: inferring the mode of operation is deprecated. *** Future versions of Libtool will require -mode=MODE be specified. Using host libthread_db library "/lib/libthread_db.so.1". (gdb) (gdb) (gdb) set print static-members off (gdb) tty /dev/pts/10 (gdb) set width 0 (gdb) set height 0 (gdb) set stop-on 1 (gdb) handle SIG32 pass nostop noprint (gdb) handle SIG41 pass nostop noprint (gdb) handle SIG43 pass nostop noprint (gdb) set print asm-demangle on (gdb) set output-radix 10 (gdb) cd /home/mememe/taglib-1.4/examples/ (gdb) set args '/home/mememe/_dnloads_to_file/11 - Nobodys Listening.mp3' (gdb) break id3v2header.cpp:140 No source file named id3v2header.cpp. (gdb) break id3v2header.cpp:172 No source file named id3v2header.cpp. (gdb) run Stopped due to shared library event (gdb) break id3v2header.cpp:140 Breakpoint 1 at 0x2aaaaac1f959: file id3v2header.cpp, line 140. (gdb) break id3v2header.cpp:172 Breakpoint 2 at 0x2aaaaac1fb25: file id3v2header.cpp, line 172. (gdb) continue Stopped due to shared library event (gdb) continue Stopped due to shared library event (gdb) continue Program received signal SIGINT, Interrupt. 0x00002aaaaac46d50 in std::_Destroy<__gnu_cxx::__normal_iterator<char*, std::vector<char, std::allocator<char> > >, std::allocator<char> > (__first={_M_current = 0x50d8f0 ""}, __last={_M_current = 0x50da80 ""}, __alloc=@0x7ffffffa489f) at stl_construct.h:173 /usr/include/c++/4.0.2/bits/stl_construct.h:173:5725:beg:0x2aaaaac46d50 (gdb) backtrace #0 0x00002aaaaac46d50 in std::_Destroy<__gnu_cxx::__normal_iterator<char*, std::vector<char, std::allocator<char> > >, std::allocator<char> > (__first={_M_current = 0x50d8f0 ""}, __last={_M_current = 0x50da80 ""}, __alloc=@0x7ffffffa489f) at stl_construct.h:173 #1 0x00002aaaaac478c7 in std::vector<char, std::allocator<char> >::erase (this=0x50ade8, __first={_M_current = 0x50d680 ""}, __last={_M_current = 0x50da80 ""}) at vector.tcc:125 #2 0x00002aaaaac463c7 in TagLib::ByteVector::resize (this=0x7ffffffa4960, size=0, padding=0 '\0') at tbytevector.cpp:460 #3 0x00002aaaaac4a681 in TagLib::File::readBlock (this=0x507690, length=1024) at tfile.cpp:98 #4 0x00002aaaaac1701f in TagLib::MPEG::File::nextFrameOffset (this=0x507690, position=231485135) at mpegfile.cpp:453 #5 0x00002aaaaac17154 in TagLib::MPEG::File::firstFrameOffset (this=0x507690) at mpegfile.cpp:496 #6 0x00002aaaaac18143 in TagLib::MPEG::Properties::read (this=0x50afb0) at mpegproperties.cpp:136 #7 0x00002aaaaac18882 in Properties (this=0x50 afb0, file=0x507690, style=TagLib::AudioProperties::Average) at mpegproperties.cpp:64 #8 0x00002aaaaac16b91 in TagLib::MPEG::File::read (this=0x507690, readProperties=true, propertiesStyle=TagLib::AudioProperties::Average) at mpegfile.cpp:553 #9 0x00002aaaaac16d97 in File (this=0x507690, file=0x7ffffffa6c05 "/home/mememe/_dnloads_to_file/11 - Nobodys Listening.mp3", readProperties=true, propertiesStyle=TagLib::AudioProperties::Average) at mpegfile.cpp:229 #10 0x00002aaaaac140db in TagLib::FileRef::create (fileName=0x7ffffffa6c05 "/home/mememe/_dnloads_to_file/11 - Nobodys Listening.mp3", readAudioProperties=true, audioPropertiesStyle=TagLib::AudioProperties::Average) at fileref.cpp:169 #11 0x00002aaaaac14299 in FileRef (this=0x7ffffffa4e30, fileName=0x7ffffffa6c05 "/home/mememe/_dnloads_to_file/11 - Nobodys Listening.mp3", readAudioProperties=true, audioPropertiesStyle=TagLib::AudioProperties::Average) at fileref.cpp:59 #12 0x0000000000400dad in main (argc=2, argv=0x7ffffffa4fa8) at tagreader.cpp:46 (gdb) frame 0 #0 0x00002aaaaac46d50 in std::_Destroy<__gnu_cxx::__normal_iterator<char*, std::vector<char, std::allocator<char> > >, std::allocator<char> > (__first={_M_current = 0x50d8f0 ""}, __last={_M_current = 0x50da80 ""}, __alloc=@0x7ffffffa489f) at stl_construct.h:173 /usr/include/c++/4.0.2/bits/stl_construct.h:173:5725:beg:0x2aaaaac46d50 On 12 Dec 2005 02:25:22 -0000, Michael Pyne <michael.pyne@kdemail.net> wrote: [bugs.kde.org quoted mail] Let me know if this is not what you asked for...I'm new to linux application debugging. Let me know if you want the mp3...maybe I can split it at the first frame to cut the size down.<br> Thanks in advance!!<br> <br> /bin/sh -c /usr/bin/libtool gdb tagreader -fullname -nx -quiet<br> (gdb) set edit off <br> (gdb) set confirm off <br> *** Warning: inferring the mode of operation is deprecated.<br> *** Future versions of Libtool will require -mode=MODE be specified.<br> Using host libthread_db library "/lib/libthread_db.so.1".<br> (gdb) <br> (gdb) <br> (gdb) set print static-members off <br> (gdb) tty /dev/pts/10 <br> (gdb) set width 0 <br> (gdb) set height 0 <br> (gdb) set stop-on 1 <br> (gdb) handle SIG32 pass nostop noprint <br> (gdb) handle SIG41 pass nostop noprint <br> (gdb) handle SIG43 pass nostop noprint <br> (gdb) set print asm-demangle on <br> (gdb) set output-radix 10 <br> (gdb) cd /home/mememe/taglib-1.4/examples/ <br> (gdb) set args '/home/mememe/_dnloads_to_file/11 - Nobodys Listening.mp3' <br> (gdb) break id3v2header.cpp:140 <br> No source file named id3v2header.cpp.<br> <br> (gdb) break id3v2header.cpp:172 <br> No source file named id3v2header.cpp.<br> <br> (gdb) run <br> Stopped due to shared library event<br> (gdb) break id3v2header.cpp:140 <br> Breakpoint 1 at 0x2aaaaac1f959: file id3v2header.cpp, line 140.<br> (gdb) break id3v2header.cpp:172 <br> Breakpoint 2 at 0x2aaaaac1fb25: file id3v2header.cpp, line 172.<br> (gdb) continue <br> Stopped due to shared library event<br> (gdb) continue <br> Stopped due to shared library event<br> (gdb) continue <br> <br> Program received signal SIGINT, Interrupt.<br> 0x00002aaaaac46d50 in std::_Destroy<__gnu_cxx::__normal_iterator<char*, std::vector<char, std::allocator<char> > >, std::allocator<char> > (__first={_M_current = 0x50d8f0 ""}, __last={_M_current = 0x50da80 ""}, __alloc=@0x7ffffffa489f) at stl_construct.h:173<br> /usr/include/c++/4.0.2/bits/stl_construct.h:173:5725:beg:0x2aaaaac46d50<br> (gdb) backtrace <br> #0 0x00002aaaaac46d50 in std::_Destroy<__gnu_cxx::__normal_iterator<char*, std::vector<char, std::allocator<char> > >, std::allocator<char> > (__first={_M_current = 0x50d8f0 ""}, __last={_M_current = 0x50da80 ""}, __alloc=@0x7ffffffa489f) at stl_construct.h:173<br> #1 0x00002aaaaac478c7 in std::vector<char, std::allocator<char> >::erase (this=0x50ade8, __first={_M_current = 0x50d680 ""}, __last={_M_current = 0x50da80 ""}) at vector.tcc:125<br> #2 0x00002aaaaac463c7 in TagLib::ByteVector::resize (this=0x7ffffffa4960, size=0, padding=0 '\0') at tbytevector.cpp:460<br> #3 0x00002aaaaac4a681 in TagLib::File::readBlock (this=0x507690, length=1024) at tfile.cpp:98<br> #4 0x00002aaaaac1701f in TagLib::MPEG::File::nextFrameOffset (this=0x507690, position=231485135) at mpegfile.cpp:453<br> #5 0x00002aaaaac17154 in TagLib::MPEG::File::firstFrameOffset (this=0x507690) at mpegfile.cpp:496<br> #6 0x00002aaaaac18143 in TagLib::MPEG::Properties::read (this=0x50afb0) at mpegproperties.cpp:136<br> #7 0x00002aaaaac18882 in Properties (this=0x50<br> afb0, file=0x507690, style=TagLib::AudioProperties::Average) at mpegproperties.cpp:64<br> #8 0x00002aaaaac16b91 in TagLib::MPEG::File::read (this=0x507690, readProperties=true, propertiesStyle=TagLib::AudioProperties::Average) at mpegfile.cpp:553<br> #9 0x00002aaaaac16d97 in File (this=0x507690, file=0x7ffffffa6c05 "/home/mememe/_dnloads_to_file/11 - Nobodys Listening.mp3", readProperties=true, propertiesStyle=TagLib::AudioProperties::Average) at mpegfile.cpp:229<br> #10 0x00002aaaaac140db in TagLib::FileRef::create (fileName=0x7ffffffa6c05 "/home/mememe/_dnloads_to_file/11 - Nobodys Listening.mp3", readAudioProperties=true, audioPropertiesStyle=TagLib::AudioProperties::Average) at fileref.cpp:169<br> #11 0x00002aaaaac14299 in FileRef (this=0x7ffffffa4e30, fileName=0x7ffffffa6c05 "/home/mememe/_dnloads_to_file/11 - Nobodys Listening.mp3", readAudioProperties=true, audioPropertiesStyle=TagLib::AudioProperties::Average) at fileref.cpp:59<br> #12 0x0000000000400dad in main (argc=2, argv=0x7ffffffa4fa8) at tagreader.cpp:46<br> (gdb) frame 0 <br> #0 0x00002aaaaac46d50 in std::_Destroy<__gnu_cxx::__normal_iterator<char*, std::vector<char, std::allocator<char> > >, std::allocator<char> > (__first={_M_current = 0x50d8f0 ""}, __last={_M_current = 0x50da80 ""}, __alloc=@0x7ffffffa489f) at stl_construct.h:173<br> /usr/include/c++/4.0.2/bits/stl_construct.h:173:5725:beg:0x2aaaaac46d50<br> <br><br><div><span class="gmail_quote">On 12 Dec 2005 02:25:22 -0000, <b class="gmail_sendername">Michael Pyne</b> <<a href="mailto:michael.pyne@kdemail.net">michael.pyne@kdemail.net</a>> wrote:</span><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> ------- You are receiving this mail because: -------<br>You reported the bug, or are watching the reporter.<br><br><a href="http://bugs.kde.org/show_bug.cgi?id=114469">http://bugs.kde.org/show_bug.cgi?id=114469</a><br><br> <br><br><br>------- Additional Comments From michael.pyne kdemail net 2005-12-12 03:25 -------<br>Judging from the source, readBlock() shouldn't hang waiting for a larger tag than normal, so it looks like the parse() call is what is actually hanging.<br><br>So I would like to see what is causing parse() to hang and fix it there in case an invalid tag with a small enough header is read and trips the bug that way.<br><br>If TagLib was compiled with debugging information it would be nice to get a backtrace of the application when it is frozen in TagLib. There is a nice HOWTO at <a href="http://ktown.kde.org/~wheeler/freeze.html">http://ktown.kde.org/~wheeler/freeze.html</a><br><br>Otherwise if you could email the smallest file with the invalid tag to michael.pyne (AT) <a href="http://kdemail.net">kdemail.net</a> I could try to figure it out.<br></blockquote></div><br>
OK, well judging by the backtrace it really is crashing in readBlock(). But it looks like an STL error. ByteVector was initialized with a size of 1024, and is being resize()'ed to 0. The underlying code should work (and a testcase I constructed here of that event does work) but perhaps there's a bug in your STL. I'll attach a patch that might work around it if you want to try it.
Created attachment 13935 [details] Patch to clear ByteVector when resizing to 0. Doesn't actually fix a bug in TagLib, but if I'm right it may work around the bug the user is experiencing. P.S. I still have no testcase. If you can extract the relevant part of the file and still reproduce the crash that would be awesome. :)
Created attachment 13980 [details] a tiny mp3 file to demonstrate the problem (gzipped)
I'm pretty sure I applied the patch properly, but still have the problem.
markey has confirmed the attached testcase causes the app to hang. The testcase works fine here though. I'm not sure what the deal is. :(
I'm also unable to reproduce this one.
Is it possible that this is a 64bit bug....I may not have mentioned that I'm using and Amd64 dualcore.
Looking more closely, the real problem here is something like this: - nextFrameOffset() is called with a position beyond the end of the file - there is a seek beyond the end of the file (from the value above) - the next call to readBlock() should return an empty ByteVector, but apparently doesn't - since readBlock() is constantly returning a non-empty ByteVector the loop in nextFrameOffset() continues infinitely Shao -- can you try to add a 'debug("looping...");" line inside the while loop in readBlock() (in tfile.cpp) to test this theory? If so, I've got a couple more things I could send your way for testing. I have two or three patches here that would probably fix the specific issues that you're seeing, but this could even be a glibc error (if fread() isn't giving the correct return values). My other guess would be something around the size_t to const int conversion in the return value of fread(), but well, zero should be zero in both types. ;-)
I found the problem! There are two varialbes in nextFrameOffset with the same name. If the buffer variable in the outer scope is not size 0 on initial read, the while loop will never end, becaue it has a variable of the same name inside that goes out of scope before the while is evalutated..ie, the outer buffer variable is alwasy checked. Either rename the inner buffer or reuse the outer one. Let me know if I need to submit this as a patch. long MPEG::File::nextFrameOffset(long position) { // TODO: This will miss syncs spanning buffer read boundaries. ByteVector buffer = readBlock(bufferSize()); while(buffer.size() > 0) { seek(position); // ByteVector buffer = readBlock(bufferSize()); buffer = readBlock(bufferSize()); for(uint i = 0; i < buffer.size(); i++) { if(uchar(buffer[i]) == 0xff && secondSynchByte(buffer[i + 1])) return position + i; } position += bufferSize(); } return -1; }
...which is why Michael and I couldn't reproduce it, since that was already fixed in subversion. :-) At any rate -- I'm glad we found out what was causing the problem.