| Message ID | 20240507002723.1603-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] checkasm/blockdsp: don't randomize the buffers for fill_block_tab | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Mon, 6 May 2024, James Almer wrote: > It ignores and overwrites the previous values. > Fixes running the test under ubsan. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > tests/checkasm/blockdsp.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) The change is probably correct, but what issue is ubsan complaining about? If this would just be a dead store of unused random values, that shouldn't be an ubsan issue in general, right? // Martin
Martin Storsjö: > On Mon, 6 May 2024, James Almer wrote: > >> It ignores and overwrites the previous values. >> Fixes running the test under ubsan. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> tests/checkasm/blockdsp.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) > > The change is probably correct, but what issue is ubsan complaining > about? If this would just be a dead store of unused random values, that > shouldn't be an ubsan issue in general, right? > UBSan complains about unaligned stores in randomize_buffers; which is obvious given that i is incremented by 1, not by 2. I sent a patch that fixes this without removing randomization: https://ffmpeg.org/pipermail/ffmpeg-devel/2024-May/326945.html (Ideally, this test should be extended to cover cases of stride != width.) - Andreas
On Tue, 7 May 2024, Andreas Rheinhardt wrote: > Martin Storsjö: >> On Mon, 6 May 2024, James Almer wrote: >> >>> It ignores and overwrites the previous values. >>> Fixes running the test under ubsan. >>> >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> tests/checkasm/blockdsp.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> The change is probably correct, but what issue is ubsan complaining >> about? If this would just be a dead store of unused random values, that >> shouldn't be an ubsan issue in general, right? >> > > UBSan complains about unaligned stores in randomize_buffers; which is > obvious given that i is incremented by 1, not by 2. I sent a patch that > fixes this without removing randomization: > https://ffmpeg.org/pipermail/ffmpeg-devel/2024-May/326945.html Thanks, that explains it. Those two patches LGTM. // Martin
On 5/7/2024 7:49 AM, Andreas Rheinhardt wrote: > Martin Storsjö: >> On Mon, 6 May 2024, James Almer wrote: >> >>> It ignores and overwrites the previous values. >>> Fixes running the test under ubsan. >>> >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> tests/checkasm/blockdsp.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> The change is probably correct, but what issue is ubsan complaining >> about? If this would just be a dead store of unused random values, that >> shouldn't be an ubsan issue in general, right? >> > > UBSan complains about unaligned stores in randomize_buffers; which is > obvious given that i is incremented by 1, not by 2. I sent a patch that > fixes this without removing randomization: There's no reason to keep the randomization for this test. That's why i replaced it with a memset instead of fixing the unaligned writes. > https://ffmpeg.org/pipermail/ffmpeg-devel/2024-May/326945.html > > (Ideally, this test should be extended to cover cases of stride != width.) > > - Andreas > > _______________________________________________ > 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".
diff --git a/tests/checkasm/blockdsp.c b/tests/checkasm/blockdsp.c index 19d69b8687..ab87fc8fa4 100644 --- a/tests/checkasm/blockdsp.c +++ b/tests/checkasm/blockdsp.c @@ -71,7 +71,8 @@ static void check_fill(BlockDSPContext *h){ ptrdiff_t line_size, int h); if (check_func(h->fill_block_tab[t], "blockdsp.%s", tests[t].name)) { uint8_t value = rnd(); - randomize_buffers(tests[t].size); + memset(buf0, 0, sizeof(*buf0) * n * n); + memset(buf1, 0, sizeof(*buf0) * n * n); call_ref(buf0, value, n, n); call_new(buf1, value, n, n); if (memcmp(buf0, buf1, sizeof(*buf0) * n * n))
It ignores and overwrites the previous values. Fixes running the test under ubsan. Signed-off-by: James Almer <jamrial@gmail.com> --- tests/checkasm/blockdsp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)