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

[Ronald S. Bultje]

Hi,

On Thu, Mar 12, 2015 at 9:37 AM, Michael Niedermayer <michaelni at gmx.at>
wrote:

> On Thu, Mar 12, 2015 at 07:14:54AM -0400, Ronald S. Bultje wrote:
> > Hi,
> >
> > On Wed, Mar 11, 2015 at 9:00 PM, Michael Niedermayer <michaelni at gmx.at>
> > wrote:
> >
> > > Found-by: Clang -fsanitize=shift
> > > Reported-by: Thierry Foucu <tfoucu at google.com>
> > > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > > ---
> > >  libavcodec/h264_mb.c |   14 +++++++-------
> > >  1 file changed, 7 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/libavcodec/h264_mb.c b/libavcodec/h264_mb.c
> > > index dd406c7..a4653aa 100644
> > > --- a/libavcodec/h264_mb.c
> > > +++ b/libavcodec/h264_mb.c
> > > @@ -213,7 +213,7 @@ static av_always_inline void
> mc_dir_part(H264Context
> > > *h, H264Picture *pic,
> > >      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
>
> 6.5.7 Bitwise shift operators
> ...
> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated bits
> are filled with
>   zeros. If E1 has an unsigned type, the value of the result is E1 * 2^E2
> , reduced modulo
>   one more than the maximum value representable in the result type. If E1
> has a signed
>   type and nonnegative value, and E1 * 2^E2 is representable in the result
> type, then that is
>   the resulting value; otherwise, the behavior is undefined.


Hm... I guess mx itself is unbounded; ok.

Ronald