[cocci] HELP: multiple inserts at one position

[cocci] HELP: multiple inserts at one position

[Bernhard Reutner-Fischer]

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

Hi!

I'm trying to follow Julias advise in
https://www.mail-archive.com/cocci@systeme.lip6.fr/msg06465.html
---8<---
> >   Unfortunately, I stumble on the error message “replacement: already 
> > tagged token:
> >   C code context” then.

This is what I would expect.  You could use one rule with an exists to put
a position variable in the place where you want to put a kfree, and then
use another rule to put a kfree at that position.
---8<---

I'm attaching my sample inp.c and my attempt at the script and would be
glad if you could give some guidance how to properly phrase the
replacement rule.

In effect, i want to insert *multiple* hunks, each hunk before the
respective single return statement.

TIA for your help or any pointers and cheers,

PS:
In case it matters, i'm using the debian package which is
  spatch version 1.1.1 compiled with OCaml version 4.13.1

The two attached scripts are identical, just
  sed -i -e s/mpz/mpfr/g
If i patch the mpfr part of inp.c, then all works like i would
envision, since there is just a single replacement to be done:

$ spatch --sp-file ~/mpfr_free.0.cocci /tmp/inp.cc;echo EOF
init_defs_builtins: /usr/lib/coccinelle/standard.h
HANDLING: /tmp/inp.cc
diff = 
--- /tmp/inp.cc
+++ /tmp/cocci-output-1395250-146bec-inp.cc
@@ -34,9 +34,11 @@ CompileExpr::compile_float_literal (cons
       rust_error_at (expr.get_locus (),
 		     "decimal overflows the respective type %<%s%>",
 		     tyty->get_name ().c_str ());
+      mpfr_clear(fval);
       return error_mark_node;
     }
 
+  mpfr_clear(fval);
   return real_value;
 }
 
EOF

But when attempting to insert multiple "free()" for a handful of
identifiers in the mpz case, then:
$ spatch --sp-file ~/mpz_free.0.cocci /tmp/inp.cc ; echo EOF
init_defs_builtins: /usr/lib/coccinelle/standard.h
HANDLING: /tmp/inp.cc
     
previous modification:

  <<< mpz_clear(i);
CONTEXT

According to environment 4:
   mpz_0_find.i -> id ival

   mpz_0_find.ret_pos -> poss[(/tmp/inp.cc,CompileExpr::compile_integer_literal,((44,0),(80,1)),(71,6),(71,12))]

   mpz_0_find.i -> id ival

   mpz_0_find.ret_pos -> poss[(/tmp/inp.cc,CompileExpr::compile_integer_literal,((44,0),(80,1)),(71,6),(71,12))]


current modification:

  <<< mpz_clear(i);
CONTEXT

According to environment 4:
   mpz_0_find.i -> id type_max

   mpz_0_find.ret_pos -> poss[(/tmp/inp.cc,CompileExpr::compile_integer_literal,((44,0),(80,1)),(71,6),(71,12))]

   mpz_0_find.i -> id type_max

   mpz_0_find.ret_pos -> poss[(/tmp/inp.cc,CompileExpr::compile_integer_literal,((44,0),(80,1)),(71,6),(71,12))]


mpz_0_replace: already tagged token:
C code context
File "/tmp/inp.cc", line 71, column 6, charpos = 1993
  around = 'return',
  whole content =       return error_mark_node;
EOF

Isn't this OK nevertheless?

[-- Attachment #2: inp.cc.txt --]
[-- Type: text/plain, Size: 2167 bytes --]

tree
CompileExpr::compile_float_literal (const HIR::LiteralExpr &expr,
				    const TyTy::BaseType *tyty)
{
  rust_assert (expr.get_lit_type () == HIR::Literal::FLOAT);
  const auto literal_value = expr.get_literal ();

  mpfr_t fval;
  if (mpfr_init_set_str (fval, literal_value.as_string ().c_str (), 10,
			 MPFR_RNDN)
      != 0)
    {
      rust_error_at (expr.get_locus (), "bad number in literal");
      mpfr_clears(expr, fval, NULL);
      mpfr_clear(fval);
      return error_mark_node;
    }

  tree type = TyTyResolveCompile::compile (ctx, tyty);

  // taken from:
  // see go/gofrontend/expressions.cc:check_float_type
  mpfr_exp_t exp = mpfr_get_exp (fval);
  bool real_value_overflow = exp > TYPE_PRECISION (type);

  REAL_VALUE_TYPE r1;
  real_from_mpfr (&r1, fval, type, GMP_RNDN);
  REAL_VALUE_TYPE r2;
  real_convert (&r2, TYPE_MODE (type), &r1);

  tree real_value = build_real (type, r2);
  if (TREE_OVERFLOW (real_value) || real_value_overflow)
    {
      rust_error_at (expr.get_locus (),
		     "decimal overflows the respective type %<%s%>",
		     tyty->get_name ().c_str ());
      return error_mark_node;
    }

  return real_value;
}


tree
CompileExpr::compile_integer_literal (const HIR::LiteralExpr &expr,
				      const TyTy::BaseType *tyty)
{
  rust_assert (expr.get_lit_type () == HIR::Literal::INT);
  const auto literal_value = expr.get_literal ();

  tree type = TyTyResolveCompile::compile (ctx, tyty);

  mpz_t ival;
  if (mpz_init_set_str (ival, literal_value.as_string ().c_str (), 10) != 0)
    {
      rust_error_at (expr.get_locus (), "bad number in literal");
      return error_mark_node;
    }

  mpz_t type_min;
  mpz_t type_max;
  mpz_init (type_min);
  mpz_init (type_max);
  get_type_static_bounds (type, type_min, type_max);

  if (mpz_cmp (ival, type_min) < 0 || mpz_cmp (ival, type_max) > 0)
    {
      rust_error_at (expr.get_locus (),
		     "integer overflows the respective type %<%s%>",
		     tyty->get_name ().c_str ());
      return error_mark_node;
    }

  tree result = wide_int_to_tree (type, wi::from_mpz (type, ival, true));

  mpz_clear (type_min);
  mpz_clear (type_max);

  return result;
}

[-- Attachment #3: mpfr_free.0.cocci.txt --]
[-- Type: text/plain, Size: 587 bytes --]

/// mpfr ///////////////////////////////////////////////////////////////

@ mpfr_0_find exists@
identifier i;
type mpfr_t;
position ret_pos;
@@
mpfr_t i;
...
(
mpfr_init_set_str (i, ...)
|
mpfr_init (i)
|
mpfr_init2 (i, ...)
|
mpfr_init3 (i, ...)
)
... when != mpfr_clear (i)
    when != mpfr_clears (...,i,...)
(
  return \(<+...i...+>\);
|
//Ideally i would prefer to just have this single rule and just do:
//+ mpfr_clear (i);
return@ret_pos ...;
)

@ mpfr_0_replace @
identifier mpfr_0_find.i;
//expression E;
position mpfr_0_find.ret_pos;
@@
+ mpfr_clear (i);
? return@ret_pos ...;

[-- Attachment #4: mpz_free.0.cocci.txt --]
[-- Type: text/plain, Size: 572 bytes --]

/// mpz ///////////////////////////////////////////////////////////////

@ mpz_0_find exists@
identifier i;
type mpz_t;
position ret_pos;
@@
mpz_t i;
...
(
mpz_init_set_str (i, ...)
|
mpz_init (i)
|
mpz_init2 (i, ...)
|
mpz_init3 (i, ...)
)
... when != mpz_clear (i)
    when != mpz_clears (...,i,...)
(
  return \(<+...i...+>\);
|
//Ideally i would prefer to just have this single rule and just do:
//+ mpz_clear (i);
return@ret_pos ...;
)

@ mpz_0_replace @
identifier mpz_0_find.i;
//expression E;
position mpz_0_find.ret_pos;
@@
+ mpz_clear (i);
? return@ret_pos ...;

Re: [cocci] HELP: multiple inserts at one position

[Julia Lawall]

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



On Thu, 9 Mar 2023, Julia Lawall wrote:

>
>
> On Thu, 9 Mar 2023, Bernhard Reutner-Fischer wrote:
>
> > Hi!
> >
> > I'm trying to follow Julias advise in
> > https://www.mail-archive.com/cocci@systeme.lip6.fr/msg06465.html
> > ---8<---
> > > >   Unfortunately, I stumble on the error message “replacement: already
> > > > tagged token:
> > > >   C code context” then.
> >
> > This is what I would expect.  You could use one rule with an exists to put
> > a position variable in the place where you want to put a kfree, and then
> > use another rule to put a kfree at that position.
> > ---8<---
>
> Use ++ instead of +.  Then you can add multiple things in one place.
> There is no guarantee on the order in which they will appear.

Another issue is that your semantic patch could be more efficient.  You
have the declaration

type mpz_t;

I think you meant:

typedef mpz_t;

Then the only goal of the pattern

mpz_t i;
...

seems to be to ensure the type of i.  This can be done in the metavariable
declaration:

local idexpression mpz_t i

and then in the second rule, it would be:

local idexpression mpz_t mpz_0_find.i;

In this way, you will not be matching from the top of the function, but
rather only from the relevant call to the return.

julia


>
> julia
>
> >
> > I'm attaching my sample inp.c and my attempt at the script and would be
> > glad if you could give some guidance how to properly phrase the
> > replacement rule.
> >
> > In effect, i want to insert *multiple* hunks, each hunk before the
> > respective single return statement.
> >
> > TIA for your help or any pointers and cheers,
> >
> > PS:
> > In case it matters, i'm using the debian package which is
> >   spatch version 1.1.1 compiled with OCaml version 4.13.1
> >
> > The two attached scripts are identical, just
> >   sed -i -e s/mpz/mpfr/g
> > If i patch the mpfr part of inp.c, then all works like i would
> > envision, since there is just a single replacement to be done:
> >
> > $ spatch --sp-file ~/mpfr_free.0.cocci /tmp/inp.cc;echo EOF
> > init_defs_builtins: /usr/lib/coccinelle/standard.h
> > HANDLING: /tmp/inp.cc
> > diff =
> > --- /tmp/inp.cc
> > +++ /tmp/cocci-output-1395250-146bec-inp.cc
> > @@ -34,9 +34,11 @@ CompileExpr::compile_float_literal (cons
> >        rust_error_at (expr.get_locus (),
> >  		     "decimal overflows the respective type %<%s%>",
> >  		     tyty->get_name ().c_str ());
> > +      mpfr_clear(fval);
> >        return error_mark_node;
> >      }
> >
> > +  mpfr_clear(fval);
> >    return real_value;
> >  }
> >
> > EOF
> >
> > But when attempting to insert multiple "free()" for a handful of
> > identifiers in the mpz case, then:
> > $ spatch --sp-file ~/mpz_free.0.cocci /tmp/inp.cc ; echo EOF
> > init_defs_builtins: /usr/lib/coccinelle/standard.h
> > HANDLING: /tmp/inp.cc
> >
> > previous modification:
> >
> >   <<< mpz_clear(i);
> > CONTEXT
> >
> > According to environment 4:
> >    mpz_0_find.i -> id ival
> >
> >    mpz_0_find.ret_pos -> poss[(/tmp/inp.cc,CompileExpr::compile_integer_literal,((44,0),(80,1)),(71,6),(71,12))]
> >
> >    mpz_0_find.i -> id ival
> >
> >    mpz_0_find.ret_pos -> poss[(/tmp/inp.cc,CompileExpr::compile_integer_literal,((44,0),(80,1)),(71,6),(71,12))]
> >
> >
> > current modification:
> >
> >   <<< mpz_clear(i);
> > CONTEXT
> >
> > According to environment 4:
> >    mpz_0_find.i -> id type_max
> >
> >    mpz_0_find.ret_pos -> poss[(/tmp/inp.cc,CompileExpr::compile_integer_literal,((44,0),(80,1)),(71,6),(71,12))]
> >
> >    mpz_0_find.i -> id type_max
> >
> >    mpz_0_find.ret_pos -> poss[(/tmp/inp.cc,CompileExpr::compile_integer_literal,((44,0),(80,1)),(71,6),(71,12))]
> >
> >
> > mpz_0_replace: already tagged token:
> > C code context
> > File "/tmp/inp.cc", line 71, column 6, charpos = 1993
> >   around = 'return',
> >   whole content =       return error_mark_node;
> > EOF
> >
> > Isn't this OK nevertheless?
> >

Re: [cocci] HELP: multiple inserts at one position

[Bernhard Reutner-Fischer]

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

On Thu, 9 Mar 2023 09:47:17 +0100 (CET)
Julia Lawall <julia.lawall@inria.fr> wrote:

> On Thu, 9 Mar 2023, Julia Lawall wrote:

> Another issue is that your semantic patch could be more efficient.  You
> have the declaration
> 
> type mpz_t;
> 
> I think you meant:
> 
> typedef mpz_t;
> 
> Then the only goal of the pattern
> 
> mpz_t i;
> ...
> 
> seems to be to ensure the type of i.  This can be done in the metavariable
> declaration:

I initially had a typedef, but thought i'd better simplify it, so yea.

> 
> local idexpression mpz_t i

Ok, i once had a local idexpression but without the type, thanks for
the hint!

> 
> and then in the second rule, it would be:
> 
> local idexpression mpz_t mpz_0_find.i;
> 
> In this way, you will not be matching from the top of the function, but
> rather only from the relevant call to the return.

Ah, excellent, many thanks! Works marvellous as attached.

Now, mere cosmetics, but is there a way to retain the spaces before the
braces in the replacement?
That would spare me a sed to obey to the coding conventions in gcc.
With the attached i get:
$ spatch --sp-file /tmp/mpz_free.0.cocci.txt /tmp/inp.cc
diff = 
--- /tmp/inp.cc
+++ /tmp/cocci-output-1431557-e530f9-inp.cc
@@ -54,6 +54,7 @@ CompileExpr::compile_integer_literal (co
   if (mpz_init_set_str (ival, literal_value.as_string ().c_str (), 10) != 0)
     {
       rust_error_at (expr.get_locus (), "bad number in literal");
+      mpz_clear(ival);
       return error_mark_node;
     }
 
@@ -69,6 +70,8 @@ CompileExpr::compile_integer_literal (co
 		     "integer overflows the respective type %<%s%>",
 		     tyty->get_name ().c_str ());
       mpz_clears (ival, expr, NULL); /* fake, just checking.. */
+      mpz_clear(type_min);
+      mpz_clear(type_max);
       return error_mark_node;
     }
 
@@ -77,5 +80,6 @@ CompileExpr::compile_integer_literal (co
   mpz_clear (type_min);
   mpz_clear (type_max);
 
+  mpz_clear(ival);
   return result;
 }


Thanks again!

[-- Attachment #2: mpz_free.0.cocci.txt --]
[-- Type: text/plain, Size: 489 bytes --]

/// mpz ///////////////////////////////////////////////////////////////

@ mpz_0_find exists@
typedef mpz_t;
local idexpression mpz_t i;
position ret_pos;
@@
(
mpz_init_set_str (i, ...)
|
mpz_init (i)
|
mpz_init2 (i, ...)
|
mpz_init3 (i, ...)
)
... when != mpz_clear (i)
    when != mpz_clears (...,i,...)
(
  return \(<+...i...+>\);
|
return@ret_pos ...;
)

@ mpz_0_replace @
local idexpression mpz_t mpz_0_find.i;
position mpz_0_find.ret_pos;
@@
++ mpz_clear (i);
? return@ret_pos ...;

Re: [cocci] HELP: multiple inserts at one position

[Julia Lawall]

> Now, mere cosmetics, but is there a way to retain the spaces before the
> braces in the replacement?
> That would spare me a sed to obey to the coding conventions in gcc.
> With the attached i get:

Try the argument --smpl-spacing.

julia

> $ spatch --sp-file /tmp/mpz_free.0.cocci.txt /tmp/inp.cc
> diff =
> --- /tmp/inp.cc
> +++ /tmp/cocci-output-1431557-e530f9-inp.cc
> @@ -54,6 +54,7 @@ CompileExpr::compile_integer_literal (co
>    if (mpz_init_set_str (ival, literal_value.as_string ().c_str (), 10) != 0)
>      {
>        rust_error_at (expr.get_locus (), "bad number in literal");
> +      mpz_clear(ival);
>        return error_mark_node;
>      }
>
> @@ -69,6 +70,8 @@ CompileExpr::compile_integer_literal (co
>  		     "integer overflows the respective type %<%s%>",
>  		     tyty->get_name ().c_str ());
>        mpz_clears (ival, expr, NULL); /* fake, just checking.. */
> +      mpz_clear(type_min);
> +      mpz_clear(type_max);
>        return error_mark_node;
>      }
>
> @@ -77,5 +80,6 @@ CompileExpr::compile_integer_literal (co
>    mpz_clear (type_min);
>    mpz_clear (type_max);
>
> +  mpz_clear(ival);
>    return result;
>  }
>
>
> Thanks again!
>

Re: [cocci] HELP: multiple inserts at one position

[Bernhard Reutner-Fischer]

On Thu, 9 Mar 2023 10:38:50 +0100 (CET)
Julia Lawall <julia.lawall@inria.fr> wrote:

> > Now, mere cosmetics, but is there a way to retain the spaces before the
> > braces in the replacement?
> 
> Try the argument --smpl-spacing.

That does the trick, thanks!

Is there a way to use regexp captures for the matching and replacement
part? Think templates for (parts of) the name of the identifiers.

In the specific case of mpz and mpfr, both use a similar API for the
allocators {mpfr,mpz}_init{,2,3,_set_str} and deallocators
{mpfr,mpz}_clear{,s}

To avoid duplicating the rules for "mpfr" and "mpz", it would be handy
to match and replace per the underlying type or, maybe, a part of the
matched identifier name. Is that something that is readily available?

Just curious, it's not all that important..

thanks for your prompt help so far!

Re: [cocci] HELP: multiple inserts at one position

[Julia Lawall]

On Thu, 9 Mar 2023, Bernhard Reutner-Fischer wrote:

> On Thu, 9 Mar 2023 10:38:50 +0100 (CET)
> Julia Lawall <julia.lawall@inria.fr> wrote:
>
> > > Now, mere cosmetics, but is there a way to retain the spaces before the
> > > braces in the replacement?
> >
> > Try the argument --smpl-spacing.
>
> That does the trick, thanks!
>
> Is there a way to use regexp captures for the matching and replacement
> part? Think templates for (parts of) the name of the identifiers.
>
> In the specific case of mpz and mpfr, both use a similar API for the
> allocators {mpfr,mpz}_init{,2,3,_set_str} and deallocators
> {mpfr,mpz}_clear{,s}
>
> To avoid duplicating the rules for "mpfr" and "mpz", it would be handy
> to match and replace per the underlying type or, maybe, a part of the
> matched identifier name. Is that something that is readily available?
>
> Just curious, it's not all that important..

You can use regular expressions, but I'm not sure that there would be an
easy way to correlate the added code to the matched variant.

julia

Re: [cocci] HELP: multiple inserts at one position

[Bernhard Reutner-Fischer]

On Thu, 9 Mar 2023 09:47:17 +0100 (CET)
Julia Lawall <julia.lawall@inria.fr> wrote:

> On Thu, 9 Mar 2023, Julia Lawall wrote:
> > On Thu, 9 Mar 2023, Bernhard Reutner-Fischer wrote:
[]
> Another issue is that your semantic patch could be more efficient.  You
> have the declaration
> 
> type mpz_t;
> 
> I think you meant:
> 
> typedef mpz_t;
> 
> Then the only goal of the pattern
> 
> mpz_t i;
> ...
> 
> seems to be to ensure the type of i.  This can be done in the metavariable
> declaration:
> 
> local idexpression mpz_t i
> 
> and then in the second rule, it would be:
> 
> local idexpression mpz_t mpz_0_find.i;
> 
> In this way, you will not be matching from the top of the function, but
> rather only from the relevant call to the return.

mhm. But if there is no return statement whatsoever in that scope?

Consider:
int myround (double dbl)
{
  int ret = 0;
  // mpfr_t outer; mpfr_init (outer);
  if (global_var)
    {
      unsigned long ul;
      mpfr_t i;
      mpfr_init (i);
      mpfr_frac (i, dbl, MPFR_RNDZ);
      if (mpfr_cmp_si (i, 0) != 0)
        {
	  ret = 42;
	  moan ("round");
	}
      /* mpfr_clear (i); missing here */
    }
  // mpfr_clear (outer) insertion works, there is a return stmt.
  // probably breaks in a void function like in the scope above..
  // There is no 'i' to clear _here_, of course!
  return ret;
}
EOF

If i'd understand a local idexpression 'i' to have a scope, i would have
hoped to somehow get at the position of i at end of scope?
But i think i need to match either a return or end-of-block, so i can 
insert the mpfr_clear properly in either case. Somehow.

Ideas?
many thanks,

Re: [cocci] HELP: multiple inserts at one position

[Julia Lawall]

On Fri, 10 Mar 2023, Bernhard Reutner-Fischer wrote:

> On Thu, 9 Mar 2023 09:47:17 +0100 (CET)
> Julia Lawall <julia.lawall@inria.fr> wrote:
>
> > On Thu, 9 Mar 2023, Julia Lawall wrote:
> > > On Thu, 9 Mar 2023, Bernhard Reutner-Fischer wrote:
> []
> > Another issue is that your semantic patch could be more efficient.  You
> > have the declaration
> >
> > type mpz_t;
> >
> > I think you meant:
> >
> > typedef mpz_t;
> >
> > Then the only goal of the pattern
> >
> > mpz_t i;
> > ...
> >
> > seems to be to ensure the type of i.  This can be done in the metavariable
> > declaration:
> >
> > local idexpression mpz_t i
> >
> > and then in the second rule, it would be:
> >
> > local idexpression mpz_t mpz_0_find.i;
> >
> > In this way, you will not be matching from the top of the function, but
> > rather only from the relevant call to the return.
>
> mhm. But if there is no return statement whatsoever in that scope?
>
> Consider:
> int myround (double dbl)
> {
>   int ret = 0;
>   // mpfr_t outer; mpfr_init (outer);
>   if (global_var)
>     {
>       unsigned long ul;
>       mpfr_t i;
>       mpfr_init (i);
>       mpfr_frac (i, dbl, MPFR_RNDZ);
>       if (mpfr_cmp_si (i, 0) != 0)
>         {
> 	  ret = 42;
> 	  moan ("round");
> 	}
>       /* mpfr_clear (i); missing here */
>     }
>   // mpfr_clear (outer) insertion works, there is a return stmt.
>   // probably breaks in a void function like in the scope above..
>   // There is no 'i' to clear _here_, of course!
>   return ret;
> }
> EOF
>
> If i'd understand a local idexpression 'i' to have a scope, i would have
> hoped to somehow get at the position of i at end of scope?
> But i think i need to match either a return or end-of-block, so i can
> insert the mpfr_clear properly in either case. Somehow.

OK, there are some hacks to deal with the issue at the end of the
function, but not in the case of an arbitrary scope.

Typically missing frees affect if branches, so the problem doesn't arise,
but if you want the whole scope, it seems that you would need the {}.

julia

Re: [cocci] HELP: multiple inserts at one position

[Bernhard Reutner-Fischer]

On Fri, 10 Mar 2023 11:12:27 +0100 (CET)
Julia Lawall <julia.lawall@inria.fr> wrote:

> On Fri, 10 Mar 2023, Bernhard Reutner-Fischer wrote:
> 
> > On Thu, 9 Mar 2023 09:47:17 +0100 (CET)
> > Julia Lawall <julia.lawall@inria.fr> wrote:
> >  
> > > On Thu, 9 Mar 2023, Julia Lawall wrote:  
> > > > On Thu, 9 Mar 2023, Bernhard Reutner-Fischer wrote:  
> > []  
> > > Another issue is that your semantic patch could be more efficient.  You
> > > have the declaration
> > >
> > > type mpz_t;
> > >
> > > I think you meant:
> > >
> > > typedef mpz_t;
> > >
> > > Then the only goal of the pattern
> > >
> > > mpz_t i;
> > > ...
> > >
> > > seems to be to ensure the type of i.  This can be done in the metavariable
> > > declaration:
> > >
> > > local idexpression mpz_t i
> > >
> > > and then in the second rule, it would be:
> > >
> > > local idexpression mpz_t mpz_0_find.i;
> > >
> > > In this way, you will not be matching from the top of the function, but
> > > rather only from the relevant call to the return.  
> >
> > mhm. But if there is no return statement whatsoever in that scope?
> >
> > Consider:
> > int myround (double dbl)
> > {
> >   int ret = 0;
> >   // mpfr_t outer; mpfr_init (outer);
> >   if (global_var)
> >     {
> >       unsigned long ul;
> >       mpfr_t i;
> >       mpfr_init (i);
> >       mpfr_frac (i, dbl, MPFR_RNDZ);
> >       if (mpfr_cmp_si (i, 0) != 0)
> >         {
> > 	  ret = 42;
> > 	  moan ("round");
> > 	}
> >       /* mpfr_clear (i); missing here */
> >     }
> >   // mpfr_clear (outer) insertion works, there is a return stmt.
> >   // probably breaks in a void function like in the scope above..
> >   // There is no 'i' to clear _here_, of course!
> >   return ret;
> > }
> > EOF
> >
> > If i'd understand a local idexpression 'i' to have a scope, i would have
> > hoped to somehow get at the position of i at end of scope?
> > But i think i need to match either a return or end-of-block, so i can
> > insert the mpfr_clear properly in either case. Somehow.  
> 
> OK, there are some hacks to deal with the issue at the end of the
> function, but not in the case of an arbitrary scope.
> 
> Typically missing frees affect if branches, so the problem doesn't arise,
> but if you want the whole scope, it seems that you would need the {}.

So IIUC i would need to attach a position to the closing curly brace
'}'.
But how would i do that? I tried:

/// mpfr ///////////////////////////////////////////////////////////////

@ mpfr_0_find exists @
type mpfr_t;
identifier i;
position ret_pos, e_pos, s_pos;
expression E0, E1;
statement s0;
@@
{...
mpfr_t i;...
// rejected: <...{...>
( mpfr_init_set_str (i, ...)
| mpfr_init (i)
| mpfr_init2 (i, ...)
| mpfr_init3 (i, ...)
)
<+... when != mpfr_clear (i)
    when != mpfr_clears (...,i,...)
( return \(<+...i...+>\);
| return@ret_pos ...;
//rejected: | }@e_pos
)
...+>
//rejected: <...}...>
// the below does not seem to work for me (2)
}@e_pos

@ mpfr_0_replace @
identifier mpfr_0_find.i;
expression mpfr_0_find.E0;
statement mpfr_0_find.s0;
position mpfr_0_find.ret_pos;
position mpfr_0_find.e_pos;
position mpfr_0_find.s_pos;
@@
(
++ mpfr_clear (i) /* ret */;
? return@ret_pos ...;
|
++ mpfr_clear (i) /* E0 */;
// below an attempt to reference e_pos as per (2) above:
//assertion failed:  e_pos;
//rejected:  @e_pos
//rejected, no semicolon?:  E0@e_pos
//rejected: no semicolon?:? E0@e_pos
//the below does not work, E0 was not set
 E0@e_pos;
|
++ mpfr_clear (i) /* s0 */;
  s0@s_pos;
)

Re: [cocci] HELP: multiple inserts at one position

[Julia Lawall]

On Mon, 13 Mar 2023, Bernhard Reutner-Fischer wrote:

> On Fri, 10 Mar 2023 11:12:27 +0100 (CET)
> Julia Lawall <julia.lawall@inria.fr> wrote:
>
> > On Fri, 10 Mar 2023, Bernhard Reutner-Fischer wrote:
> >
> > > On Thu, 9 Mar 2023 09:47:17 +0100 (CET)
> > > Julia Lawall <julia.lawall@inria.fr> wrote:
> > >
> > > > On Thu, 9 Mar 2023, Julia Lawall wrote:
> > > > > On Thu, 9 Mar 2023, Bernhard Reutner-Fischer wrote:
> > > []
> > > > Another issue is that your semantic patch could be more efficient.  You
> > > > have the declaration
> > > >
> > > > type mpz_t;
> > > >
> > > > I think you meant:
> > > >
> > > > typedef mpz_t;
> > > >
> > > > Then the only goal of the pattern
> > > >
> > > > mpz_t i;
> > > > ...
> > > >
> > > > seems to be to ensure the type of i.  This can be done in the metavariable
> > > > declaration:
> > > >
> > > > local idexpression mpz_t i
> > > >
> > > > and then in the second rule, it would be:
> > > >
> > > > local idexpression mpz_t mpz_0_find.i;
> > > >
> > > > In this way, you will not be matching from the top of the function, but
> > > > rather only from the relevant call to the return.
> > >
> > > mhm. But if there is no return statement whatsoever in that scope?
> > >
> > > Consider:
> > > int myround (double dbl)
> > > {
> > >   int ret = 0;
> > >   // mpfr_t outer; mpfr_init (outer);
> > >   if (global_var)
> > >     {
> > >       unsigned long ul;
> > >       mpfr_t i;
> > >       mpfr_init (i);
> > >       mpfr_frac (i, dbl, MPFR_RNDZ);
> > >       if (mpfr_cmp_si (i, 0) != 0)
> > >         {
> > > 	  ret = 42;
> > > 	  moan ("round");
> > > 	}
> > >       /* mpfr_clear (i); missing here */
> > >     }
> > >   // mpfr_clear (outer) insertion works, there is a return stmt.
> > >   // probably breaks in a void function like in the scope above..
> > >   // There is no 'i' to clear _here_, of course!
> > >   return ret;
> > > }
> > > EOF
> > >
> > > If i'd understand a local idexpression 'i' to have a scope, i would have
> > > hoped to somehow get at the position of i at end of scope?
> > > But i think i need to match either a return or end-of-block, so i can
> > > insert the mpfr_clear properly in either case. Somehow.
> >
> > OK, there are some hacks to deal with the issue at the end of the
> > function, but not in the case of an arbitrary scope.
> >
> > Typically missing frees affect if branches, so the problem doesn't arise,
> > but if you want the whole scope, it seems that you would need the {}.
>
> So IIUC i would need to attach a position to the closing curly brace
> '}'.
> But how would i do that? I tried:

I would suggest ot have one set of rules that takes care of the cases
where the return is explicit in the code.  Then you can have a rule like
the following one, that just takes care of the final trailing return case
(bcause youhave already taken care of the explicit returns cases):

@exists@
typedef mpfr_t;
identifier i, f;
@@

void f(...) {
 ... when any
 mpfr_t i;
...
 mpfr_init (i)
 ... when != mpfr_clear (i)
++mpfr_clear (i);
}

julia





>
> /// mpfr ///////////////////////////////////////////////////////////////
>
> @ mpfr_0_find exists @
> type mpfr_t;
> identifier i;
> position ret_pos, e_pos, s_pos;
> expression E0, E1;
> statement s0;
> @@
> {...
> mpfr_t i;...
> // rejected: <...{...>
> ( mpfr_init_set_str (i, ...)
> | mpfr_init (i)
> | mpfr_init2 (i, ...)
> | mpfr_init3 (i, ...)
> )
> <+... when != mpfr_clear (i)
>     when != mpfr_clears (...,i,...)
> ( return \(<+...i...+>\);
> | return@ret_pos ...;
> //rejected: | }@e_pos
> )
> ...+>
> //rejected: <...}...>
> // the below does not seem to work for me (2)
> }@e_pos
>
> @ mpfr_0_replace @
> identifier mpfr_0_find.i;
> expression mpfr_0_find.E0;
> statement mpfr_0_find.s0;
> position mpfr_0_find.ret_pos;
> position mpfr_0_find.e_pos;
> position mpfr_0_find.s_pos;
> @@
> (
> ++ mpfr_clear (i) /* ret */;
> ? return@ret_pos ...;
> |
> ++ mpfr_clear (i) /* E0 */;
> // below an attempt to reference e_pos as per (2) above:
> //assertion failed:  e_pos;
> //rejected:  @e_pos
> //rejected, no semicolon?:  E0@e_pos
> //rejected: no semicolon?:? E0@e_pos
> //the below does not work, E0 was not set
>  E0@e_pos;
> |
> ++ mpfr_clear (i) /* s0 */;
>   s0@s_pos;
> )
>