[FFmpeg-devel] [PATCH] avcodec/apedec: fix undefined left shifts of negative numbers
gajjanag at mit.edu
Tue Sep 29 19:53:44 CEST 2015
On Tue, Sep 29, 2015 at 11:22 AM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
> On Tue, Sep 29, 2015 at 11:04 AM, Ganesh Ajjanagadde <gajjanag at mit.edu>
>> On Tue, Sep 29, 2015 at 9:24 AM, Hendrik Leppkes <h.leppkes at gmail.com>
>> > 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
>> >> 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.
This should be ok - res will be promoted to unsigned. The bit
representation being identical is not guaranteed, but in the 2's
complement world it should be fine.
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
More information about the ffmpeg-devel