[FFmpeg-devel] [PATCH] checkasm: Fix the takdsp tests

[James Almer]

On 12/22/2023 6:49 AM, Martin Storsjö wrote:
> For memcpy and memcmp, we need to multiply by the element size,
> otherwise we're copying and comparing only a fraction of the buffer.
> 
> For decorrelate_sr, the buffer p1 is the one that is mutated;
> copy and check p1 instead of p2.
> 
> For decorrelate_sm, both buffers are mutated, so copy and check
> both of them.
> 
> For decorrelate_sm, the memcpy initialization of p1 and p1_2 was
> reversed - p1 is filled with randomize, but then memcpy copies from
> p1_2 to p1. As p1_2 is uninitialized at this point, clang concluded
> that the copy was bogus and omitted it entirely, triggering failures
> in this test on x86 (where there was an existing assembly implementation
> to test).
> ---
>   tests/checkasm/takdsp.c | 19 ++++++++++---------
>   1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/checkasm/takdsp.c b/tests/checkasm/takdsp.c
> index 495b7242c5..3aecee1f72 100644
> --- a/tests/checkasm/takdsp.c
> +++ b/tests/checkasm/takdsp.c
> @@ -44,12 +44,12 @@ static void test_decorrelate_ls(TAKDSPContext *s) {
>   
>           randomize(p1, BUF_SIZE);
>           randomize(p2, BUF_SIZE);
> -        memcpy(p2_2, p2, BUF_SIZE);
> +        memcpy(p2_2, p2, BUF_SIZE * sizeof(*p2));
>   
>           call_ref(p1, p2, BUF_SIZE);
>           call_new(p1, p2_2, BUF_SIZE);
>   
> -        if (memcmp(p2, p2_2, BUF_SIZE) != 0){
> +        if (memcmp(p2, p2_2, BUF_SIZE * sizeof(*p2)) != 0) {
>               fail();
>           }
>   
> @@ -65,17 +65,17 @@ static void test_decorrelate_sr(TAKDSPContext *s) {
>   
>       if (check_func(s->decorrelate_sr, "decorrelate_sr")) {
>           LOCAL_ALIGNED_32(int32_t, p1, [BUF_SIZE]);
> +        LOCAL_ALIGNED_32(int32_t, p1_2, [BUF_SIZE]);
>           LOCAL_ALIGNED_32(int32_t, p2, [BUF_SIZE]);
> -        LOCAL_ALIGNED_32(int32_t, p2_2, [BUF_SIZE]);
>   
>           randomize(p1, BUF_SIZE);
> +        memcpy(p1_2, p1, BUF_SIZE * sizeof(*p1));
>           randomize(p2, BUF_SIZE);
> -        memcpy(p2_2, p2, BUF_SIZE);
>   
>           call_ref(p1, p2, BUF_SIZE);
> -        call_new(p1, p2_2, BUF_SIZE);
> +        call_new(p1_2, p2, BUF_SIZE);
>   
> -        if (memcmp(p2, p2_2, BUF_SIZE) != 0){
> +        if (memcmp(p1, p1_2, BUF_SIZE * sizeof(*p1)) != 0) {
>               fail();
>           }
>   
> @@ -96,14 +96,15 @@ static void test_decorrelate_sm(TAKDSPContext *s) {
>           LOCAL_ALIGNED_32(int32_t, p2_2, [BUF_SIZE]);
>   
>           randomize(p1, BUF_SIZE);
> -        memcpy(p1, p1_2, BUF_SIZE);
> +        memcpy(p1_2, p1, BUF_SIZE * sizeof(*p1));
>           randomize(p2, BUF_SIZE);
> -        memcpy(p2_2, p2, BUF_SIZE);
> +        memcpy(p2_2, p2, BUF_SIZE * sizeof(*p2));
>   
>           call_ref(p1, p2, BUF_SIZE);
>           call_new(p1_2, p2_2, BUF_SIZE);
>   
> -        if (memcmp(p2, p2_2, BUF_SIZE) != 0){
> +        if (memcmp(p1, p1_2, BUF_SIZE * sizeof(*p1)) != 0 ||
> +            memcmp(p2, p2_2, BUF_SIZE * sizeof(*p2)) != 0) {
>               fail();
>           }

LGTM.