Re: [PATCH v1] Travis: also test on 32-bit Linux

["René Scharfe" <l.s.r@web.de>]

Am 11.03.2017 um 00:33 schrieb Junio C Hamano:
> René Scharfe <l.s.r@web.de> writes:
> 
>> 	@ depends on r @
>> 	expression E;
>> 	@@
>> 	- *&
>> 	  E
> 
> I guess my source of the confusion is that the tool that understands
> the semantics of the C language still needs to be told about that.

Understanding that E and *&E have the same type does not imply (for a
machine) that E alone is preferable.

> I was hoping that something that understands C only needs to be told
> only a single rule:
> 
> 	type T
> 	T src, dst
> 
> 	-memcpy(&dst, &src, sizeof(dst));
> 	+dst = src;
> 
> and then can apply that rule to this code in four ways:
> 
> 	struct foo A, *Bp;
> 
> 	memcpy(Bp, &A, sizeof(*Bp));
> 	memcpy(Bp, &A, sizeof(A));
> 	memcpy(&src, dstp, sizeof(A));
> 	memcpy(&src, dstp, sizeof(*Bp));

I guess src is A and dstp is Bp?

Coccinelle does not know that the sizeof expressions are equivalent,
but you could normalize them with an additional rule and then use a
single memcpy transformation like this:

	@ normalize_sizeof @
	type T;
	T var;
	@@
	- sizeof(var)
	+ sizeof(T)

	@ r @
	type T;
	T *dst;
	T *src;
	@@
	- memcpy(dst, src, sizeof(T));
	+ *dst = *src;

	@ depends on r @
	expression E;
	@@
	- *&
	  E

That would give you what you expected here:

> to obtain its rewrite:
> 
> 	struct foo A, *Bp;
> 
> 	*Bp = A;
> 	*Bp = A;
> 	A = *Bp;
> 	A = *Bp;

But that normalization rule would be applied everywhere because it's so
broad, and that's probably not what we want.  You could replace it with
an isomorphism like this one:

	Expression
	@@
	type T;
	T E;
	@@

	sizeof(T) => sizeof E

In your example it allows you to specify sizeof(struct foo) (or a
generic sizeof(T) as in rule r above) in the semantic patch and
Coccinelle would let that match sizeof(A) and sizeof(*Bp) in the C
file as well.

Isomorphisms are kept in a separate file, the default one is
/usr/lib/coccinelle/standard.iso on my machine and you can chose a
different one with --iso-file.  Perhaps we want to have our own for
git, but we'd need to synchronize it with upstream from time to time.

There is already a very similar isomorphism rule in the default file,
but I don't think I understand it:

	Expression
	@ sizeof_type_expr @
	pure type T; // pure because we drop a metavar
	T E;
	@@

	sizeof(T) => sizeof E

In particular I'm a bit worried about the lack of documentation for
"pure", and I don't understand the comment.  There's another comment
at the top of another rule in the same file:

// pure means that either the whole field reference expression is dropped,
// or E is context code and has no attached + code
// not really... pure means matches a unitary unplussed metavariable
// but this rule doesn't work anyway

Hmm.

Here's an isomorphism that allows you to use "sizeof *src" (the third
part for T is not strictly needed for your example):

	Expression
	@ sizeof_equiv @
	type T;
	T E1, E2;
	@@

	sizeof E1 <=> sizeof E2 <=> sizeof(T)

That's the kind of bidirectional equivalence you expected to be part
of Coccinelle's standard set of rules, right?  With that rule in place
the following semantic patch matches your four cases:

	@ r @
	type T;
	T *dst;
	T *src;
	@@
	- memcpy(dst, src, sizeof *dst);
	+ *dst = *src;

	@ depends on r @
	expression E;
	@@
	- *&
	  E

But I'd be careful with that as long as "pure" is present in that
standard rule and not fully understood.  The isomorphism sizeof_equiv
doesn't seem to cause any trouble with our existing semantic patches
and the code in master, though.

René