Opened 12 years ago

Closed 12 years ago

#1566 closed defect (fixed)

incorrect assembly code in libavcodec/x86/dsputil_mmx.c

Reported by: yang Owned by:
Priority: important Component: avcodec
Version: git-master Keywords: dsputil_mmx
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

Summary of the bug:
In file libavcodec/x86/dsputil_mmx.c, function ff_put_pixels_clamped_mmx(), there are two assembly code blocks. In the first block (in the unrolled loop), the instructions "movq 8%3, %%mm1 \n\t" etc have problem.
For above instruction, it is clear what the programmer wants: a load from p + 8. But this assembly code doesn’t guarantee that. It only works if the compiler puts p in a register to produce an instruction like this: “movq 8(%edi), %mm1”. During compiler optimization, it is possible that the compiler will be able to constant propagate into p. Suppose p = &x[10000]. Then operand 3 can become 10000(%edi), where %edi holds &x. And the instruction becomes “movq 810000(%edx)”. That is, it will stride by 810000 instead of 8.
This will cause the segmentation fault.
This error was fixed in the second block of the assembly code, but not in the unrolled loop.

How to reproduce:

This error is exposed when we build the ffmpeg using Intel C++ Compiler, IPO+PGO optimization. The ffmpeg was crashed when decoding a mjpeg video.

Attachments (1)

dsputil_mmx.c.patch (1.4 KB ) - added by yang 12 years ago.
patch file

Download all attachments as: .zip

Change History (3)

comment:1 by Carl Eugen Hoyos, 12 years ago

Please provide a unified diff (diff -u) as produced by git diff >dsputil_mmx.patch - we cannot read other diff formats.

by yang, 12 years ago

Attachment: dsputil_mmx.c.patch added

patch file

comment:2 by Michael Niedermayer, 12 years ago

Resolution: fixed
Status: newclosed

Patch applied

thanks

Note: See TracTickets for help on using tickets.