[Ffmpeg-devel] random crashes decoding MP3s

[Jacob Meuser]

On Mon, Apr 03, 2006 at 01:44:58PM +0100, M??ns Rullg??rd wrote:
> 
> Jacob Meuser said:
> > On Mon, Apr 03, 2006 at 08:50:03AM +0100, M?ns Rullg?rd wrote:
> >> Jacob Meuser <jakemsr at jakemsr.com> writes:
> >>
> >> > On Sun, Apr 02, 2006 at 03:09:07AM -0400, Rich Felker wrote:
> >> >> On Sat, Apr 01, 2006 at 08:31:43PM -0800, Jacob Meuser wrote:
> >> >> > yes, there is no memlign() on OpenBSD.
> >> >> >
> >> >> > quoting malloc(3)
> >> >> >
> >> >> >     The allocated space is suitably aligned (after possible
> >> >> >     pointer coercion) for storage of any type of object.  If the
> >> >> >     space is of pagesize or larger, the memory returned will be
> >> >> >     page-aligned.
> >> >>
> >> >> This text is not meaningful to what we're talking about. ISO C
> >> >> requires that the return value of malloc be "suitably aligned for
> >> >> storage of any type of object", but "object" is defined as in ISO C,
> >> >> and this has nothing to do with the alignment requirements of various
> >> >> asm constructs. A true memalign is needed..
> >> >
> >> > I have always disabled MEMALIGN_HACK and never had any problems.
> >>
> >> Maybe OpenBSD malloc() aligns more than is required.
> >
> > not sure what you mean here.
> 
> The C standard requires malloc() to return memory suitably aligned for any
> data type.  On 32-bit machines the required alignment is usually 32 bits
> (or less).  On 64-bit machines it is typically 64 bits for 64-bit data
> types.  It is possible that some malloc() implementation always returns
> addresses aligned to, e.g., 64 bits even on 32-bit hardware.  IIRC, glibc
> malloc() does something like this.

OK.  perhaps I misunderstand, but it looks like you are saying the
problem here would be on x86 and not x86_64?  or is that just 
one possibility?

> > but anyway, that wouldn't explain why FFmpeg sources from a year ago
> > work consistently, but current ones don't.
> 
> Indeed.
> 
> > it looks like the only change in huffman_decode() that could matter
> > was the get_vlc -> get_vlc2 change.  this is consistent with gdb
> > giving line 1656 as the crash point, no?
> >
> > hmmm, that seems to be the problem.  putting the old get_vlc() back
> > into bitstream.h and changing the get_vlc2() to get_vlc() on line
> > 1653 of mpegaudiodec.c make the crashes stop.
> 
> OK, so does get_vlc2() use any mmx/sse instructions, or otherwise do
> something that might have stricter alignment requirements than whatever
> get_vlc() does?

not as far as I can see.  they both use GET_VLC, which uses asm.

and, the crashes do not go away if I enable the memalign hack.

-- 
<jakemsr at jakemsr.com>