| Message ID | 8cfb78ebf68076f26aee97f951fc135e6aede100.camel@haerdin.se |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/5] lavu/common.h: Fix UB in av_clipl_int32_c() | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/configure_loongarch64 | warning | Failed to apply patch |
| andriy/configure_x86 | warning | Failed to apply patch |
Tomas Härdin: > static av_always_inline av_const int av_clip_intp2_c(int a, int p) > { > - if (((unsigned)a + (1 << p)) & ~((2 << p) - 1)) > + if (((unsigned)a + (1U << p)) & ~((2U << p) - 1)) > return (a >> 31) ^ ((1 << p) - 1); > else > return a; This will support p == 30 (but not 31); but the first change is not UB in this range. - Andreas
tor 2024-05-30 klockan 00:24 +0200 skrev Andreas Rheinhardt: > Tomas Härdin: > > static av_always_inline av_const int av_clip_intp2_c(int a, int p) > > { > > - if (((unsigned)a + (1 << p)) & ~((2 << p) - 1)) > > + if (((unsigned)a + (1U << p)) & ~((2U << p) - 1)) > > return (a >> 31) ^ ((1 << p) - 1); > > else > > return a; > > This will support p == 30 (but not 31); but the first change is not > UB > in this range. p=31 is most definitely UB before this change. 1<<31 is signed overflow with 32-bit int. The compiler has therefore been allowed to do whatever for p=31 up until this point. To me it seems the intent of the code is preserved Personally I dislike bithacks because they are difficult to verify. A good enough compiler will gain peephole optimizations for them sooner or later anyway /Tomas
From 7b18f24c0bedfeebcdfb23ea837cea8d4c35cf30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se> Date: Thu, 16 May 2024 16:33:44 +0200 Subject: [PATCH 2/5] lavu/common.h: Fix UB in av_clip_intp2_c() Found by value analysis --- libavutil/common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/common.h b/libavutil/common.h index ac68c0cfff..715f0a594c 100644 --- a/libavutil/common.h +++ b/libavutil/common.h @@ -264,7 +264,7 @@ static av_always_inline av_const int32_t av_clipl_int32_c(int64_t a) */ static av_always_inline av_const int av_clip_intp2_c(int a, int p) { - if (((unsigned)a + (1 << p)) & ~((2 << p) - 1)) + if (((unsigned)a + (1U << p)) & ~((2U << p) - 1)) return (a >> 31) ^ ((1 << p) - 1); else return a; -- 2.39.2