Re: [PATCH] cocci: avoid normalization rules for memcpy

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jul 10, 2022, 14:45 UTC
Message-ID: <220710.86ilo580mb.gmgdl@evledraar.gmail.com>
In-Reply-To: <ded153d4-4aea-d4da-11cb-ec66d181e4c9@web.de>

On Sun, Jul 10 2022, René Scharfe wrote:

> Some of the rules for using COPY_ARRAY instead of memcpy with sizeof are
> intended to reduce the number of sizeof variants to deal with.  They can
> have unintended side effects if only they match, but not the one for the
> COPY_ARRAY conversion at the end.

Since ab/cocci-unused is marked for "next" it would be really nice to have this based on top so we can add tests for these transformations (the topic adds a way to do that).

But if you don't feel like  doing that this is fine too.

Show 110 quoted lines

> diff --git a/contrib/coccinelle/array.cocci b/contrib/coccinelle/array.cocci
> index 9a4f00cb1b..aa75937950 100644
> --- a/contrib/coccinelle/array.cocci
> +++ b/contrib/coccinelle/array.cocci
> @@ -1,60 +1,58 @@
> -@@
> -expression dst, src, n, E;
> -@@
> -  memcpy(dst, src, n * sizeof(
> -- E[...]
> -+ *(E)
> -  ))
> -
> -@@
> -type T;
> -T *ptr;
> -T[] arr;
> -expression E, n;
> -@@
> -(
> -  memcpy(ptr, E,
> -- n * sizeof(*(ptr))
> -+ n * sizeof(T)
> -  )
> -|
> -  memcpy(arr, E,
> -- n * sizeof(*(arr))
> -+ n * sizeof(T)
> -  )
> -|
> -  memcpy(E, ptr,
> -- n * sizeof(*(ptr))
> -+ n * sizeof(T)
> -  )
> -|
> -  memcpy(E, arr,
> -- n * sizeof(*(arr))
> -+ n * sizeof(T)
> -  )
> -)
> -
>  @@
>  type T;
>  T *dst_ptr;
>  T *src_ptr;
> -T[] dst_arr;
> -T[] src_arr;
>  expression n;
>  @@
> -(
> -- memcpy(dst_ptr, src_ptr, (n) * sizeof(T))
> +- memcpy(dst_ptr, src_ptr, (n) * \( sizeof(T)
> +-                                \| sizeof(*(dst_ptr))
> +-                                \| sizeof(*(src_ptr))
> +-                                \| sizeof(dst_ptr[...])
> +-                                \| sizeof(src_ptr[...])
> +-                                \) )
>  + COPY_ARRAY(dst_ptr, src_ptr, n)
> -|
> -- memcpy(dst_ptr, src_arr, (n) * sizeof(T))
> +
> +@@
> +type T;
> +T *dst_ptr;
> +T[] src_arr;
> +expression n;
> +@@
> +- memcpy(dst_ptr, src_arr, (n) * \( sizeof(T)
> +-                                \| sizeof(*(dst_ptr))
> +-                                \| sizeof(*(src_arr))
> +-                                \| sizeof(dst_ptr[...])
> +-                                \| sizeof(src_arr[...])
> +-                                \) )
>  + COPY_ARRAY(dst_ptr, src_arr, n)
> -|
> -- memcpy(dst_arr, src_ptr, (n) * sizeof(T))
> +
> +@@
> +type T;
> +T[] dst_arr;
> +T *src_ptr;
> +expression n;
> +@@
> +- memcpy(dst_arr, src_ptr, (n) * \( sizeof(T)
> +-                                \| sizeof(*(dst_arr))
> +-                                \| sizeof(*(src_ptr))
> +-                                \| sizeof(dst_arr[...])
> +-                                \| sizeof(src_ptr[...])
> +-                                \) )
>  + COPY_ARRAY(dst_arr, src_ptr, n)
> -|
> -- memcpy(dst_arr, src_arr, (n) * sizeof(T))
> +
> +@@
> +type T;
> +T[] dst_arr;
> +T[] src_arr;
> +expression n;
> +@@
> +- memcpy(dst_arr, src_arr, (n) * \( sizeof(T)
> +-                                \| sizeof(*(dst_arr))
> +-                                \| sizeof(*(src_arr))
> +-                                \| sizeof(dst_arr[...])
> +-                                \| sizeof(src_arr[...])
> +-                                \) )
>  + COPY_ARRAY(dst_arr, src_arr, n)
> -)
>
>  @@
>  type T;

Hrm, this seems like a lot of repetition, it's here in the rules you're editing already, but these repeated "sizeof" make it a lot more verbose.

Isn't there a way to avoid this by simply wrapping this across lines, I didn't test, but I think you can do this sort of thing in the cocci grammar:

- memcpy(
- COPY_ARRAY(
  (
  dst_arr
  |
  dst_ptr
  )
  ,
  (
  src_arr
  |
  src_ptr
  )
  ,
  (n) *
-  [your big sizeof alternate here]
  )

I.e. you want to preserve whatever we match in the 1st and 2nd arguments, but only want to munge part of the 3rd argument. The cocci grammar can "reach into" lines like that, it doesn't need to be limited to line-based diffs.

But I didn't try it in this caes, and maybe there's a good reason for why it can't happen in this case...

I also wonder if that won't be a lot faster, i.e. if you can condense this all into one rule it won't need to match this N times, but maybe the overall complexity of the rules makes it come out to the same thing in the end...