Re: coccinelle: adjustments for array.cocci?

From: René Scharfe <l.s.r@web.de>
Date: Nov 19, 2019, 19:14 UTC
Message-ID: <321802c9-e5ea-452f-a3fd-7e01ab84b1f9@web.de>
In-Reply-To: <eff19da9-3f9f-0cf0-1e88-64d2acdbabcd@web.de>

Am 17.11.19 um 19:19 schrieb Markus Elfring:

Show 10 quoted lines

>> Whitespace is not what makes the above example more complicated than the
>> equivalent rule below;
>
> A different code layout might help in a better understanding for such
> change specifications.
>
>
>> separating the pieces of simple expressions does.
>
> Will there occasionally be a need to change only the required source code parts?

Changing parts that don't need to be changed does not make sense to me. Why do you ask and how does it relate to the example at hand?

Show 11 quoted lines

>>>> than what we currently have:
>>>>   @@
>>>>   expression dst, src, n, E;
>>>>   @@
>>>>     memcpy(dst, src, n * sizeof(
>>>>   - E[...]
>>>>   + *(E)
>>>>     ))
>
> Are any circumstances to consider where only the essential implementation details
> should be touched by an automatic software transformation?

I don't understand this question.

Show 8 quoted lines

>> It looks like a bug in Coccinelle to me
>
> We might stumble also on just another (temporary) software limitation.
>
>
>> and I'd like to see it fixed
>
> Would you like to support corresponding development anyhow?

I don't see me learning OCaml in the near future. Or are you looking for donations? :)

>> But at least until then our semantic patches need to work around it.
>
> Would another concrete fix for the currently discussed SmPL script
> be better than a “workaround”?

These are different things. Fixes (repairs) are always welcome. But they should not rely on SmPL constructs that only work properly using unreleased versions of Coccinelle.

Show 5 quoted lines

>>> Would you like to use the SmPL code “*( \( src_ptr \| src_arr \) )” instead?
>>
>> That leaves out dst_ptr and dst_arr.
>
> How many items should finally be filtered in the discussed SmPL disjunction?

Let's see: dst and src can be pointers or array references, which makes four combinations. sizeof could either operate the shared type or on an element of dst or an element of src. An element can be accessed either using dereference (*) or subscript ([]). That makes five possible variations for the sizeof, right? So twenty combinations in total.

Show 6 quoted lines

>> And what would it mean to match e.g. this ?
>>
>> 	memcpy(dst_ptr, src_ptr, n * sizeof(*src_arr))
>
> The Coccinelle software takes care for commutativity by isomorphisms.
> https://github.com/coccinelle/coccinelle/blob/19ee1697bf152d37a78a20cefe148775bf4b0e0d/standard.iso#L241

OK, but I had a different concern (more below).

>> At least the element size would be the same, but I'd rather shy away from
>> transforming weird cases like this automatically.
>
> Do you mean to specify additional restrictions by SmPL code?

Let's take this silly C fragment as an example:

	char *src = strdup("foo");
	size_t src_len = strlen(src);
	char *dst = malloc(src_len);
	char unrelated[17];
	memcpy(dst, src, src_len * sizeof(*unrelated));

My point is that taking the size of something that is neither source nor destination is weird enough that it should be left alone by semantic patches. Matching should be precise enough to avoid false transformations.

Show 8 quoted lines

>>   void *memmove(void *dest, const void *src, size_t n);
>>   void *memcpy(void *dest, const void *src, size_t n);
>>
>>   COPY_ARRAY(dst, src, n)
>>   MOVE_ARRAY(dst, src, n)
>
> Can the replacement of these functions by macro calls be combined further
> by improved SmPL code?

Very likely.

Show 6 quoted lines

>>> Possible nicer run time characteristics by the Coccinelle software.
>>
>> How much faster is it exactly?
>
> The answer will depend on efforts which you would like to invest
> in corresponding (representative) measurements.

Is that some kind of quantum effect? ;-)

When I try to convince people to apply a patch that is intended to speed up something, I often use https://github.com/sharkdp/hyperfine these days.

René