[FFmpeg-devel] [PATCH] avcodec/vp9: Fix undefined shifts in decode_frame_header()

Michael Niedermayer michaelni at gmx.at
Thu Mar 12 15:32:47 CET 2015


On Thu, Mar 12, 2015 at 10:26:55AM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Thu, Mar 12, 2015 at 10:16 AM, Michael Niedermayer <michaelni at gmx.at>
> wrote:
> 
> > On Thu, Mar 12, 2015 at 09:56:12AM -0400, Ronald S. Bultje wrote:
> > > Hi,
> > >
> > > On Thu, Mar 12, 2015 at 9:41 AM, Michael Niedermayer <michaelni at gmx.at>
> > > wrote:
> > >
> > > > On Thu, Mar 12, 2015 at 07:15:47AM -0400, Ronald S. Bultje wrote:
> > > > > Hi,
> > > > >
> > > > > On Wed, Mar 11, 2015 at 9:11 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/vp9.c |    4 ++--
> > > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
> > > > > > index 265dc7e..0405c05 100644
> > > > > > --- a/libavcodec/vp9.c
> > > > > > +++ b/libavcodec/vp9.c
> > > > > > @@ -689,10 +689,10 @@ static int decode_frame_header(AVCodecContext
> > > > *ctx,
> > > > > >          for (j = 1; j < 4; j++) {
> > > > > >              s->segmentation.feat[i].lflvl[j][0] =
> > > > > >                  av_clip_uintp2(lflvl + ((s->lf_delta.ref[j] +
> > > > > > -                                         s->lf_delta.mode[0]) <<
> > sh),
> > > > 6);
> > > > > > +                                         s->lf_delta.mode[0]) *
> > (1 <<
> > > > > > sh)), 6);
> > > > > >              s->segmentation.feat[i].lflvl[j][1] =
> > > > > >                  av_clip_uintp2(lflvl + ((s->lf_delta.ref[j] +
> > > > > > -                                         s->lf_delta.mode[1]) <<
> > sh),
> > > > 6);
> > > > > > +                                         s->lf_delta.mode[1]) *
> > (1 <<
> > > > > > sh)), 6);
> > > > >
> > > > >
> > > > > Same question: why is this undefined?
> > > >
> > > > same awnser, left shifts of negative values are undefined in C
> > > > if that was by someone forgetting to define it or intentional or they
> > > > just didnt find a good definition in light of non twos-complement
> > > > i do not know
> > >
> > >
> > > But the values aren't negative?
> >
> > if i apply:
> > @@ -687,6 +687,10 @@ static int decode_frame_header(AVCodecContext *ctx,
> >          s->segmentation.feat[i].lflvl[0][1] =
> >              av_clip_uintp2(lflvl + (s->lf_delta.ref[0] << sh), 6);
> >          for (j = 1; j < 4; j++) {
> > +            av_assert0((s->lf_delta.ref[j] +
> > +                                         s->lf_delta.mode[0]) >= 0);
> > +            av_assert0((s->lf_delta.ref[j] +
> > +                                         s->lf_delta.mode[1]) >= 0);
> >              s->segmentation.feat[i].lflvl[j][0] =
> >                  av_clip_uintp2(lflvl + ((s->lf_delta.ref[j] +
> >                                           s->lf_delta.mode[0]) << sh), 6);
> >
> > and run fate it fails all over the place
> >
> > so i think they are <0
> 
> 
> Bleh, I was missing one bracket; the delta is indeed negative; lflvl itself
> (and the sum) is not. So OK.

applied

thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- 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/699ecf95/attachment.asc>


More information about the ffmpeg-devel mailing list