[cocci] Spatch corrupting struct variable

[cocci] Spatch corrupting struct variable

[Ilpo Järvinen]

Hi all,

I'm at total loss what coccinelle/spatch is trying to do here:

@@
struct resource *res;
expression size;
@@
- res->end = res->start + size - 1;
+ resource_set_size(res, size);

Spatching against Linux kernel tree results in this corruption:

HANDLING: drivers/bus/fsl-mc/fsl-mc-bus.c
diff = 
diff -u -p a/fsl-mc-bus.c b/fsl-mc-bus.c
--- a/fsl-mc-bus.c
+++ b/fsl-mc-bus.c
@@ -736,7 +736,7 @@ static int fsl_mc_device_get_mmio_region
                        goto error_cleanup_regions;
                }
 
-               regions[i].end = regions[i].start + region_desc.size - 1;
+               resource_set_size(regions, region_desc.size);
                regions[i].name = "fsl-mc object MMIO region";
                regions[i].flags = region_desc.flags & IORESOURCE_BITS;
                regions[i].flags |= IORESOURCE_MEM;

??? How did that [i] vanish from regions? Why spatch thought . and -> are 
equal?

$ spatch --version
spatch version 1.1.0 compiled with OCaml version 4.11.1
Flags passed to the configure script: --prefix=/usr --sysconfdir=/etc --libdir=/usr/lib --enable-ocaml --enable-python --enable-opt
OCaml scripting support: yes
Python scripting support: yes
Syntax of regular expressions: PCRE


--
 i.

Re: [cocci] Spatch corrupting struct variable

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 1594 bytes --]



On Thu, 21 Dec 2023, Ilpo Järvinen wrote:

> Hi all,
>
> I'm at total loss what coccinelle/spatch is trying to do here:
>
> @@
> struct resource *res;
> expression size;
> @@
> - res->end = res->start + size - 1;
> + resource_set_size(res, size);
>
> Spatching against Linux kernel tree results in this corruption:
>
> HANDLING: drivers/bus/fsl-mc/fsl-mc-bus.c
> diff =
> diff -u -p a/fsl-mc-bus.c b/fsl-mc-bus.c
> --- a/fsl-mc-bus.c
> +++ b/fsl-mc-bus.c
> @@ -736,7 +736,7 @@ static int fsl_mc_device_get_mmio_region
>                         goto error_cleanup_regions;
>                 }
>
> -               regions[i].end = regions[i].start + region_desc.size - 1;
> +               resource_set_size(regions, region_desc.size);
>                 regions[i].name = "fsl-mc object MMIO region";
>                 regions[i].flags = region_desc.flags & IORESOURCE_BITS;
>                 regions[i].flags |= IORESOURCE_MEM;
>
> ??? How did that [i] vanish from regions? Why spatch thought . and -> are
> equal?

There is an isomorphism that does that.  Maybe it should be removed...

In any case, you can put @disable ptr_to_array@ in place of the initial @@
in your rule.  Or if your rule has a name, put the disable ptr_to_array
after the name.

julia

>
> $ spatch --version
> spatch version 1.1.0 compiled with OCaml version 4.11.1
> Flags passed to the configure script: --prefix=/usr --sysconfdir=/etc --libdir=/usr/lib --enable-ocaml --enable-python --enable-opt
> OCaml scripting support: yes
> Python scripting support: yes
> Syntax of regular expressions: PCRE
>
>
> --
>  i.
>
>

Re: [cocci] Spatch corrupting struct variable

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1452 bytes --]

On Thu, 21 Dec 2023, Julia Lawall wrote:
> On Thu, 21 Dec 2023, Ilpo Järvinen wrote:
> 
> > Hi all,
> >
> > I'm at total loss what coccinelle/spatch is trying to do here:
> >
> > @@
> > struct resource *res;
> > expression size;
> > @@
> > - res->end = res->start + size - 1;
> > + resource_set_size(res, size);
> >
> > Spatching against Linux kernel tree results in this corruption:
> >
> > HANDLING: drivers/bus/fsl-mc/fsl-mc-bus.c
> > diff =
> > diff -u -p a/fsl-mc-bus.c b/fsl-mc-bus.c
> > --- a/fsl-mc-bus.c
> > +++ b/fsl-mc-bus.c
> > @@ -736,7 +736,7 @@ static int fsl_mc_device_get_mmio_region
> >                         goto error_cleanup_regions;
> >                 }
> >
> > -               regions[i].end = regions[i].start + region_desc.size - 1;
> > +               resource_set_size(regions, region_desc.size);
> >                 regions[i].name = "fsl-mc object MMIO region";
> >                 regions[i].flags = region_desc.flags & IORESOURCE_BITS;
> >                 regions[i].flags |= IORESOURCE_MEM;
> >
> > ??? How did that [i] vanish from regions? Why spatch thought . and -> are
> > equal?
> 
> There is an isomorphism that does that.  Maybe it should be removed...
> 
> In any case, you can put @disable ptr_to_array@ in place of the initial @@
> in your rule.  Or if your rule has a name, put the disable ptr_to_array
> after the name.

Thanks for the quick reply. Adding disable ptr_to_array indeed helped. :-)

-- 
 i.

Re: [cocci] Transforming data structure accesses with SmPL

[Markus Elfring]

> I'm at total loss what coccinelle/spatch is trying to do here:
>
> @@
> struct resource *res;
> expression size;
> @@
> - res->end = res->start + size - 1;
> + resource_set_size(res, size);

I suggest to take another look at possibilities for increasing the precision
for your source code transformation approach.
Would you like to restrict the usage of member access expressions on
non-pointer types anyhow?
https://en.cppreference.com/w/c/language/operator_member_access#Member_access


See also:
Extend support for data processing with member identifications
2020-09-25


…
> -               regions[i].end = regions[i].start + region_desc.size - 1;
> +               resource_set_size(regions, region_desc.size);
…
> How did that [i] vanish from regions? Why spatch thought . and -> are equal?
You might be interested in further background information for the application
of the isomorphism “ptr_to_array”.
https://gitlab.inria.fr/coccinelle/coccinelle/-/blob/db9431856ae32e21a3c66cfa119a64170242406e/standard.iso#L557

Regards,
Markus

Re: [cocci] Transforming data structure accesses with SmPL

[Markus Elfring]

> In general, I don't want to spend my time on perfecting a .cocci script as
> further improvements tend to be needed around the areas highlighted
> anyway. It's enough for me that the most mechanical transformation cases
> are covered by spatch and I can handle the troublemakers myself while
> inspecting the results.

How do you think about to support more software evolution in affected areas?

Regards,
Markus

Re: [cocci] Transforming data structure accesses with SmPL

[Julia Lawall]

On Thu, 21 Dec 2023, Markus Elfring wrote:

> > In general, I don't want to spend my time on perfecting a .cocci script as
> > further improvements tend to be needed around the areas highlighted
> > anyway. It's enough for me that the most mechanical transformation cases
> > are covered by spatch and I can handle the troublemakers myself while
> > inspecting the results.

Seems fine to me :)

julia