| Message ID | AS8P250MB07445A3517237022C3BDBCBD8F2B2@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel] avformat/avidec: Fix integer overflow iff ULONG_MAX < INT64_MAX | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On 3/12/2024 7:57 PM, Andreas Rheinhardt wrote: > Affects many FATE-tests, see > http://fate.ffmpeg.org/report.cgi?time=20240312011016&slot=ppc-linux-gcc-13.2-ubsan-altivec-qemu > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavformat/avidec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/avidec.c b/libavformat/avidec.c > index f3183b2698..b7cbf148af 100644 > --- a/libavformat/avidec.c > +++ b/libavformat/avidec.c > @@ -1696,7 +1696,7 @@ static int check_stream_max_drift(AVFormatContext *s) > int *idx = av_calloc(s->nb_streams, sizeof(*idx)); > if (!idx) > return AVERROR(ENOMEM); > - for (min_pos = pos = 0; min_pos != INT64_MAX; pos = min_pos + 1LU) { > + for (min_pos = pos = 0; min_pos != INT64_MAX; pos = min_pos + (uint64_t)1) { nit: 1ULL > int64_t max_dts = INT64_MIN / 2; > int64_t min_dts = INT64_MAX / 2; > int64_t max_buffer = 0;
James Almer: > On 3/12/2024 7:57 PM, Andreas Rheinhardt wrote: >> Affects many FATE-tests, see >> http://fate.ffmpeg.org/report.cgi?time=20240312011016&slot=ppc-linux-gcc-13.2-ubsan-altivec-qemu >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >> --- >> libavformat/avidec.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavformat/avidec.c b/libavformat/avidec.c >> index f3183b2698..b7cbf148af 100644 >> --- a/libavformat/avidec.c >> +++ b/libavformat/avidec.c >> @@ -1696,7 +1696,7 @@ static int >> check_stream_max_drift(AVFormatContext *s) >> int *idx = av_calloc(s->nb_streams, sizeof(*idx)); >> if (!idx) >> return AVERROR(ENOMEM); >> - for (min_pos = pos = 0; min_pos != INT64_MAX; pos = min_pos + 1LU) { >> + for (min_pos = pos = 0; min_pos != INT64_MAX; pos = min_pos + >> (uint64_t)1) { > > nit: 1ULL > The other variables are int64_t, not long long int, so using uint64_t is appropriate. - Andreas
On 3/12/2024 8:01 PM, Andreas Rheinhardt wrote: > James Almer: >> On 3/12/2024 7:57 PM, Andreas Rheinhardt wrote: >>> Affects many FATE-tests, see >>> http://fate.ffmpeg.org/report.cgi?time=20240312011016&slot=ppc-linux-gcc-13.2-ubsan-altivec-qemu >>> >>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >>> --- >>> libavformat/avidec.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/libavformat/avidec.c b/libavformat/avidec.c >>> index f3183b2698..b7cbf148af 100644 >>> --- a/libavformat/avidec.c >>> +++ b/libavformat/avidec.c >>> @@ -1696,7 +1696,7 @@ static int >>> check_stream_max_drift(AVFormatContext *s) >>> int *idx = av_calloc(s->nb_streams, sizeof(*idx)); >>> if (!idx) >>> return AVERROR(ENOMEM); >>> - for (min_pos = pos = 0; min_pos != INT64_MAX; pos = min_pos + 1LU) { >>> + for (min_pos = pos = 0; min_pos != INT64_MAX; pos = min_pos + >>> (uint64_t)1) { >> >> nit: 1ULL >> > > The other variables are int64_t, not long long int, so using uint64_t is > appropriate. In practice it's not only the same, but also cleaner looking and done all across the codebase. But if you really want it to be uint64_t, then maybe use UINT64_C(1) instead (which, fwiw, will be expanded to 1ULL or even 1UL depending on target). > > - Andreas
Andreas: On Tue, Mar 12, 2024 at 6:57 PM Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote: > > Affects many FATE-tests, see > http://fate.ffmpeg.org/report.cgi?time=20240312011016&slot=ppc-linux-gcc-13.2-ubsan-altivec-qemu > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavformat/avidec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/avidec.c b/libavformat/avidec.c > index f3183b2698..b7cbf148af 100644 > --- a/libavformat/avidec.c > +++ b/libavformat/avidec.c > @@ -1696,7 +1696,7 @@ static int check_stream_max_drift(AVFormatContext *s) > int *idx = av_calloc(s->nb_streams, sizeof(*idx)); > if (!idx) > return AVERROR(ENOMEM); > - for (min_pos = pos = 0; min_pos != INT64_MAX; pos = min_pos + 1LU) { > + for (min_pos = pos = 0; min_pos != INT64_MAX; pos = min_pos + (uint64_t)1) { > int64_t max_dts = INT64_MIN / 2; > int64_t min_dts = INT64_MAX / 2; > int64_t max_buffer = 0; > -- > 2.40.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". Confirming that this fixes fate-v210 (and probably many more) on my PowerPC QEMU setup -- it was not failing on POWER7 (ppc64) or POWER9 (ppc64le) and remains so with the patch applied. Thanks, Sean McGovern
diff --git a/libavformat/avidec.c b/libavformat/avidec.c index f3183b2698..b7cbf148af 100644 --- a/libavformat/avidec.c +++ b/libavformat/avidec.c @@ -1696,7 +1696,7 @@ static int check_stream_max_drift(AVFormatContext *s) int *idx = av_calloc(s->nb_streams, sizeof(*idx)); if (!idx) return AVERROR(ENOMEM); - for (min_pos = pos = 0; min_pos != INT64_MAX; pos = min_pos + 1LU) { + for (min_pos = pos = 0; min_pos != INT64_MAX; pos = min_pos + (uint64_t)1) { int64_t max_dts = INT64_MIN / 2; int64_t min_dts = INT64_MAX / 2; int64_t max_buffer = 0;
Affects many FATE-tests, see http://fate.ffmpeg.org/report.cgi?time=20240312011016&slot=ppc-linux-gcc-13.2-ubsan-altivec-qemu Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavformat/avidec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)