[FFmpeg-devel] [PATCH] avcodec/h264_mb: Fix undefined shifts

[Michael Niedermayer]

On Thu, Mar 12, 2015 at 04:17:30PM +0100, Michael Niedermayer wrote:
> On Thu, Mar 12, 2015 at 03:39:51PM +0100, Christophe Gisquet wrote:
> > Hi,
> > 
> > 2015-03-12 14:37 GMT+01:00 Michael Niedermayer <michaelni at gmx.at>:
> > >> >      const int mx      = h->mv_cache[list][scan8[n]][0] + src_x_offset * 8;
> > >> >      int my            = h->mv_cache[list][scan8[n]][1] + src_y_offset * 8;
> > >> >      const int luma_xy = (mx & 3) + ((my & 3) << 2);
> > >> > -    ptrdiff_t offset  = ((mx >> 2) << pixel_shift) + (my >> 2) *
> > >> > h->mb_linesize;
> > >> > +    ptrdiff_t offset  = (mx >> 2) * (1 << pixel_shift) + (my >> 2) *
> > >> > h->mb_linesize;
> > >>
> > >>
> > >> Why is this undefined?
> > >
> > > Because C sucks
> > 
> > I actually have an equivalent question to Ronald's. Is there a valid
> > input that causes the undefined behaviour? For an invalid input (e.g.
> > DoS tentative), is the behaviour worsened?
> 
> i belive any motion vector that points left outside the picture will
> trigger this one, its also happening with multiple fate samples
> This issue is about undefined behavor according to the C specification
> not about current gcc generating actually bad code from it AFAIK

also a compiler could very easily generate bad code from this, that
is it would be both allowed in C and make sense for it to do so
consider
((mx >> 2) << pixel_shift)
...
const int full_mx    = mx >> 2;
....
if (full_mx                <          0 - extra_width  ||


from "(mx >> 2) << pixel_shift)" a compiler could imply that
(mx >> 2) is >= 0 as the alterantive is undefined
that makes full_mx >= 0
at this point the only way for full_mx < - extra_width is a non
zero extra_width and that is impossible on the else path to
"if (mx & 7)" above so the compiler could optimize this check out
in that case

luckily gcc is not smart enough to do that

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 2
"100% positive feedback" - "All either got their money back or didnt complain"
"Best seller ever, very honest" - "Seller refunded buyer after failed scam"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150312/06495e19/attachment.asc>