[FFmpeg-devel] [PATCH] avcodec/apedec: fix undefined left shifts of negative numbers

[Ronald S. Bultje]

Hi,

On Tue, Sep 29, 2015 at 11:04 AM, Ganesh Ajjanagadde <gajjanag at mit.edu>
wrote:

> On Tue, Sep 29, 2015 at 9:24 AM, Hendrik Leppkes <h.leppkes at gmail.com>
> wrote:
> > On Sun, Sep 20, 2015 at 4:18 AM, Ganesh Ajjanagadde
> > <gajjanagadde at gmail.com> wrote:
> >> This fixes -Wshift-negative-value reported with clang 3.7+, e.g
> >>
> http://fate.ffmpeg.org/log.cgi?time=20150919172459&log=compile&slot=x86_64-darwin-clang-polly-notiling-3.7
> .
> >> Note that the patch crucially depends on int >= 32 bits,
> >> an assumption made in many places in the codebase.
> >>
> >> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
> >> ---
> >>  libavcodec/apedec.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
> >> index 5536e0f..7b34d26 100644
> >> --- a/libavcodec/apedec.c
> >> +++ b/libavcodec/apedec.c
> >> @@ -1281,7 +1281,7 @@ static void do_apply_filter(APEContext *ctx, int
> version, APEFilter *f,
> >>              /* Update the adaption coefficients */
> >>              absres = FFABS(res);
> >>              if (absres)
> >> -                *f->adaptcoeffs = ((res & (-1<<31)) ^ (-1<<30)) >>
> >> +                *f->adaptcoeffs = ((res & (-(1<<31))) ^ (-(1<<30))) >>
> >>                                    (25 + (absres <= f->avg*3) + (absres
> <= f->avg*4/3));
> >>              else
> >>                  *f->adaptcoeffs = 0;
> >> --
> >> 2.5.2
> >>
> >
> > After this patch (GCC 5.2, x86)
> >
> > libavcodec/apedec.c: In function 'do_apply_filter':
> > libavcodec/apedec.c:1284:44: warning: integer overflow in expression
> > [-Woverflow]
> >                  *f->adaptcoeffs = ((res & (-(1<<31))) ^ (-(1<<30))) >>
>
> Good catch - made an off by one error in my assumptions. I don't see a
> nice, clean way of dealing with -(1 << 31).
> I propose one of the following:
> 1. use INT32_MIN.
> 2. use an explicit binary/hexadecimal mask.
> 3. use e.g (-2)*(1<<30).


0x80000000U is fine.

Ronald