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

[Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>]

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.

> 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...