[PR] tests/checkasm/llviddsp: Fix crash with AVX2 (PR #23969)

PR #23969 opened by mkver
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23969
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23969.patch

Also avoid using static variables in checkasm tests. This partially supersedes #23940.


>From 98d58853fa6bafc76f4cba6918cad442a17393ee Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <[email protected]>
Date: Thu, 30 Jul 2026 10:58:05 +0200
Subject: [PATCH 1/3] tests/checkasm/huffyuvencdsp: Avoid storing width in
 static variable

This has been done in order to use the same width
for benchmarks of different instruction sets to
make the benchmarks comparable. Yet it has a downside:
Only one width would ever be executed when using --repeat.
Luckily libcheckasm makes it easy to fix this:
At the start of every test function, the internal state
of rnd() is reset, so that it produces the same sequence
of random values. So just removing the static variable works.

This also makes this test trivially parallelizable with --repeat.

Signed-off-by: Andreas Rheinhardt <[email protected]>
---
 tests/checkasm/huffyuvencdsp.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/tests/checkasm/huffyuvencdsp.c b/tests/checkasm/huffyuvencdsp.c
index b5d02cda6d..8f31172321 100644
--- a/tests/checkasm/huffyuvencdsp.c
+++ b/tests/checkasm/huffyuvencdsp.c
@@ -72,14 +72,8 @@ static void check_sub_hfyu_median_pred_int16(const char *aligned, unsigned width
 
 void checkasm_check_huffyuvencdsp(void)
 {
-    static unsigned width = 0;
-
-    if (!width) {
-        width = rnd() % MAX_WIDTH;
-        width = width ? width : 1;
-    }
-
     const size_t align = av_cpu_max_align();
+    unsigned width = 1 + rnd() % MAX_WIDTH;
 
     check_sub_hfyu_median_pred_int16("_aligned", FFALIGN(width, align / sizeof(uint16_t)));
     report("sub_hfyu_median_pred_int16_aligned");
-- 
2.52.0


>From cdfa14e86ae42f099b6fde04c6381645edf6a99c Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <[email protected]>
Date: Thu, 30 Jul 2026 13:41:56 +0200
Subject: [PATCH 2/3] tests/checkasm/sbcdsp: Avoid using static variable

This has been done in order to use the same number of blocks
for benchmarks of different instruction sets to
make the benchmarks comparable. Yet it has a downside:
Only one number would ever be executed when using --repeat.
Luckily libcheckasm makes it easy to fix this:
At the start of every test function, the internal state
of rnd() is reset, so that it produces the same sequence
of random values. So just removing the static variable works.

This also makes this test trivially parallelizable with --repeat.

Signed-off-by: Andreas Rheinhardt <[email protected]>
---
 tests/checkasm/sbcdsp.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/tests/checkasm/sbcdsp.c b/tests/checkasm/sbcdsp.c
index 802eb430ed..b6170ca04f 100644
--- a/tests/checkasm/sbcdsp.c
+++ b/tests/checkasm/sbcdsp.c
@@ -69,7 +69,7 @@ static void check_sbc_analyze(SBCDSPContext *sbcdsp)
     report("sbc_analyze");
 }
 
-static void check_sbc_calc_scalefactors(const SBCDSPContext *const sbcdsp)
+static void check_sbc_calc_scalefactors(const SBCDSPContext *const sbcdsp, int blocks)
 {
     DECLARE_ALIGNED(SBC_ALIGN,  int32_t, sb_sample_f)[16][2][8];
     DECLARE_ALIGNED(SBC_ALIGN, uint32_t, scale_factor_ref)[2][8];
@@ -79,9 +79,6 @@ static void check_sbc_calc_scalefactors(const SBCDSPContext *const sbcdsp)
                        uint32_t scale_factor[2][8],
                        int blocks, int channels, int subbands);
 
-    static int blocks = 0;
-    if (!blocks)
-        blocks = ((const int[]){4, 8, 12, 15, 16})[rnd() % 5];
     int inited = 0;
 
     for (int ch = 1; ch <= 2; ++ch) {
@@ -111,11 +108,12 @@ static void check_sbc_calc_scalefactors(const SBCDSPContext *const sbcdsp)
 void checkasm_check_sbcdsp(void)
 {
     SBCDSPContext sbcdsp;
+    int blocks = ((const int[]){4, 8, 12, 15, 16})[rnd() % 5];
 
     ff_sbcdsp_init(&sbcdsp);
 
     check_sbc_analyze(&sbcdsp);
 
-    check_sbc_calc_scalefactors(&sbcdsp);
+    check_sbc_calc_scalefactors(&sbcdsp, blocks);
     report("calc_scalefactors");
 }
-- 
2.52.0


>From cef33cf4755bb12fbfe9987d8594259d003e3a23 Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <[email protected]>
Date: Fri, 31 Jul 2026 02:51:00 +0200
Subject: [PATCH 3/3] tests/checkasm/llviddsp: Fix crash with AVX2

llviddsp functions typically operate on whole lines,
i.e. the pointers are aligned to STRIDE_ALIGN. They
can therefore avoid tail handling by just clobbering
the padding (if any).

The test uses 16 * av_clip(rnd(), 16, 128) to get a random
width; in practice, it is very unlikely for rnd() to return
a value between 16 and 128, so width is typically 16*16 or
16*128. Buffers of this size are allocated later.

When rnd() returns an odd number in the allowed range
(this happens for the seed 4161216273), the buffers used
in the test don't contain the padding that exists in actual
usage, leading to invalid stores and also to segmentation faults
(usage of aligned load instructions on unaligned addresses).

Fix this by adding the necessary alignment to the buffers.

Also don't use a static variable to store width (it is unnecessary,
because since the switch to libcheckasm, rnd() always returns
the same sequence of random numbers for test runs with different
instruction sets) and use a really random width, not something
that is mostly just one of two values and always mod 16.

Signed-off-by: Andreas Rheinhardt <[email protected]>
---
 tests/checkasm/llviddsp.c | 73 +++++++++++++++++++++------------------
 1 file changed, 39 insertions(+), 34 deletions(-)

diff --git a/tests/checkasm/llviddsp.c b/tests/checkasm/llviddsp.c
index 1094638229..ce0688576e 100644
--- a/tests/checkasm/llviddsp.c
+++ b/tests/checkasm/llviddsp.c
@@ -42,12 +42,12 @@
     randomize_buffers(a0, width * sizeof(type));\
     memcpy(a1, a0, width*sizeof(type));\
 
-static void check_add_bytes(LLVidDSPContext *c, int width)
+static void check_add_bytes(LLVidDSPContext *c, int width, size_t aligned_width)
 {
-    uint8_t *dst0 = av_mallocz(width);
-    uint8_t *dst1 = av_mallocz(width);
-    uint8_t *src0 = av_calloc(width, sizeof(*src0));
-    uint8_t *src1 = av_calloc(width, sizeof(*src1));
+    uint8_t *dst0 = av_mallocz(aligned_width);
+    uint8_t *dst1 = av_mallocz(aligned_width);
+    uint8_t *src0 = av_malloc(aligned_width);
+    uint8_t *src1 = av_malloc(aligned_width);
     declare_func(void, uint8_t *dst, uint8_t *src, ptrdiff_t w);
 
     init_buffer(src0, src1, uint8_t, width);
@@ -68,14 +68,15 @@ static void check_add_bytes(LLVidDSPContext *c, int width)
     av_free(dst1);
 }
 
-static void check_add_median_pred(LLVidDSPContext *c, int width) {
+static void check_add_median_pred(LLVidDSPContext *c, int width, size_t aligned_width)
+{
     int a0, a1, b0, b1;
-    uint8_t *dst0 = av_mallocz(width);
-    uint8_t *dst1 = av_mallocz(width);
-    uint8_t *src0  = av_calloc(width, sizeof(*src0));
-    uint8_t *src1  = av_calloc(width, sizeof(*src1));
-    uint8_t *diff0 = av_calloc(width, sizeof(*diff0));
-    uint8_t *diff1 = av_calloc(width, sizeof(*diff1));
+    uint8_t *dst0  = av_mallocz(aligned_width);
+    uint8_t *dst1  = av_mallocz(aligned_width);
+    uint8_t *src0  = av_malloc(aligned_width);
+    uint8_t *src1  = av_malloc(aligned_width);
+    uint8_t *diff0 = av_malloc(aligned_width);
+    uint8_t *diff1 = av_malloc(aligned_width);
     declare_func(void, uint8_t *dst, const uint8_t *src1,
                  const uint8_t *diff, ptrdiff_t w,
                  int *left, int *left_top);
@@ -103,14 +104,14 @@ static void check_add_median_pred(LLVidDSPContext *c, int width) {
     av_free(dst1);
 }
 
-static void check_add_left_pred(LLVidDSPContext *c, int width, int acc)
+static void check_add_left_pred(LLVidDSPContext *c, int width, size_t aligned_width, int acc)
 {
     int res0, res1;
-    uint8_t *dst0 = av_mallocz(width);
-    uint8_t *dst1 = av_mallocz(width);
-    uint8_t *src0 = av_calloc(width, sizeof(*src0));
-    uint8_t *src1 = av_calloc(width, sizeof(*src1));
     declare_func(int, uint8_t *dst, const uint8_t *src, ptrdiff_t w, int acc);
+    uint8_t *dst0 = av_mallocz(aligned_width);
+    uint8_t *dst1 = av_mallocz(aligned_width);
+    uint8_t *src0 = av_malloc(aligned_width);
+    uint8_t *src1 = av_malloc(aligned_width);
 
     init_buffer(src0, src1, uint8_t, width);
 
@@ -129,15 +130,19 @@ static void check_add_left_pred(LLVidDSPContext *c, int width, int acc)
     av_free(dst1);
 }
 
-static void check_add_left_pred_16(LLVidDSPContext *c, unsigned mask, int width, unsigned acc)
+static void check_add_left_pred_16(LLVidDSPContext *c, unsigned mask, int width,
+                                   size_t align, unsigned acc)
 {
     int res0, res1;
-    uint16_t *dst0 = av_calloc(width, sizeof(*dst0));
-    uint16_t *dst1 = av_calloc(width, sizeof(*dst1));
-    uint16_t *src0 = av_calloc(width, sizeof(*src0));
-    uint16_t *src1 = av_calloc(width, sizeof(*src1));
+    uint16_t *dst0, *dst1, *src0, *src1;
+    size_t aligned_width = FFALIGN(width * sizeof(*dst0), align);
     declare_func(int, uint16_t *dst, const uint16_t *src, unsigned mask, ptrdiff_t w, unsigned acc);
 
+    dst0 = av_mallocz(aligned_width);
+    dst1 = av_mallocz(aligned_width);
+    src0 = av_malloc(aligned_width);
+    src1 = av_malloc(aligned_width);
+
     init_buffer(src0, src1, uint16_t, width);
 
     if (!dst0 || !dst1)
@@ -155,13 +160,14 @@ static void check_add_left_pred_16(LLVidDSPContext *c, unsigned mask, int width,
     av_free(dst1);
 }
 
-static void check_add_gradient_pred(LLVidDSPContext *c, int w) {
+static void check_add_gradient_pred(LLVidDSPContext *c, int w, size_t align)
+{
     int src_size, stride;
     uint8_t *src0, *src1;
     declare_func(void, uint8_t *src, const ptrdiff_t stride,
                  const ptrdiff_t width);
 
-    stride = w + 32;
+    stride = FFALIGN(w + 32, align);
     src_size = (stride + 32) * 2; /* dsp need previous line, and ignore the start of the line */
     src0 = av_mallocz(src_size);
     src1 = av_mallocz(src_size);
@@ -183,36 +189,35 @@ static void check_add_gradient_pred(LLVidDSPContext *c, int w) {
 void checkasm_check_llviddsp(void)
 {
     LLVidDSPContext c;
-    static int saved_width = 0;
-    int width = saved_width;
     int accRnd = rnd() & 0xFF;
 
-    if (!width)
-        saved_width = width = 16 * av_clip(rnd(), 16, 128);
+    size_t align = av_cpu_max_align();
+    int width  = 1 + rnd() % 16*128;
+    size_t aligned_width = FFALIGN(width, align);
 
     ff_llviddsp_init(&c);
 
     if (check_func(c.add_bytes, "add_bytes"))
-        check_add_bytes(&c, width);
+        check_add_bytes(&c, width, aligned_width);
     report("add_bytes");
 
     if (check_func(c.add_median_pred, "add_median_pred"))
-        check_add_median_pred(&c, width);
+        check_add_median_pred(&c, width, aligned_width);
     report("add_median_pred");
 
     if (check_func(c.add_left_pred, "add_left_pred_zero"))
-        check_add_left_pred(&c, width, 0);
+        check_add_left_pred(&c, width, aligned_width, 0);
     report("add_left_pred_zero");
 
     if (check_func(c.add_left_pred, "add_left_pred_rnd_acc"))
-        check_add_left_pred(&c, width, accRnd);
+        check_add_left_pred(&c, width, aligned_width, accRnd);
     report("add_left_pred_rnd_acc");
 
     if (check_func(c.add_left_pred_int16, "add_left_pred_int16"))
-        check_add_left_pred_16(&c, 255, width, accRnd);
+        check_add_left_pred_16(&c, 255, width, align, accRnd);
     report("add_left_pred_int16");
 
     if (check_func(c.add_gradient_pred, "add_gradient_pred"))
-        check_add_gradient_pred(&c, width);
+        check_add_gradient_pred(&c, width, align);
     report("add_gradient_pred");
 }
-- 
2.52.0

_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]