Re: [Cocci] spatch / semantic patch missing some conversions?

[Julia Lawall]

On Thu, 17 Aug 2017, Dan Williams wrote:

> On Thu, Aug 17, 2017 at 10:06 AM, Julia Lawall  wrote:
> > The updated semantic patch is below.  It makes one pass to collect the
> > function names and then another pass to transform the functions, whereever
> > they are defined.
> >
> > I have attached the output I get on today's linux-next.  If you want to
> > run it your self, I suggest first running
> >
> > ~/coccinelle/scripts/idutils_index.sh
>
> I tried build id-utils from source on Fedora and using the one from
> Debian, both end up with a "mkid" that segfaults when running this
> script. However, I'm able to make progress without the index, just a
> bit slower. Mind sharing which version of id-utils is working for you?

mkid --version gives

mkid - 4.6

julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] spatch / semantic patch missing some conversions?

[Dan Williams]

On Thu, Aug 17, 2017 at 10:06 AM, Julia Lawall  wrote:
> The updated semantic patch is below.  It makes one pass to collect the
> function names and then another pass to transform the functions, whereever
> they are defined.
>
> I have attached the output I get on today's linux-next.  If you want to
> run it your self, I suggest first running
>
> ~/coccinelle/scripts/idutils_index.sh

I tried build id-utils from source on Fedora and using the one from
Debian, both end up with a "mkid" that segfaults when running this
script. However, I'm able to make progress without the index, just a
bit slower. Mind sharing which version of id-utils is working for you?
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

>> * Are there further development challenges to consider for the safe 
>> identification
>>   of unique statements by metavariables?
> 
> If you want to ensure that two metavariables match things in different
> places, then put a position variable on each match and use apython rule
> afterwards to discard the occurrences that are both in the same position.

I hoped for a more convenient search specification to reduce duplicate
source code a bit.

Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Comparing statement lists with SmPL

[Julia Lawall]

On Thu, 17 Aug 2017, SF Markus Elfring wrote:

> >> elfring@Sonne:~/Projekte/Linux/next-patched> git checkout next-20170803 && 
> >> spatch.opt ~/Projekte/Coccinelle/janitor/show_same_statements3.cocci 
> >> fs/ubifs/lpt.c
> >> …
> >> @@ -1974,10 +1974,6 @@ again:
> >>int ret, lnum = lprops->lnum;
> >>
> >>ret = scan_cb(c, lprops, path[h].in_tree, data);
> >> -  if (ret < 0) {
> >> -  err = ret;
> >> -  goto out;
> >> -  }
> >>if (ret & LPT_SCAN_ADD) {
> >>/* Add all the nodes in path to the tree in memory */
> >>for (h = 1; h < c->lpt_hght; h++) {
> >>
> >>
> >> Now I wonder how this test result should fit to my source code search 
> >> pattern.
> >
> > I guess there is a loop around this code, so it is reachable from itself.
>
> * How can we achieve progress with such a bit of information?
>
> * Should such false positives be avoided anyhow?
>
> * Will answers to other questions become more helpful again?
>
> * Are there further development challenges to consider for the safe 
> identification
>   of unique statements by metavariables?

If you want to ensure that two metavariables match things in different
places, then put a position variable on each match and use apython rule
afterwards to discard the occurrences that are both in the same position.

julia___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

>> elfring@Sonne:~/Projekte/Linux/next-patched> git checkout next-20170803 && 
>> spatch.opt ~/Projekte/Coccinelle/janitor/show_same_statements3.cocci 
>> fs/ubifs/lpt.c
>> …
>> @@ -1974,10 +1974,6 @@ again:
>>  int ret, lnum = lprops->lnum;
>>
>>  ret = scan_cb(c, lprops, path[h].in_tree, data);
>> -if (ret < 0) {
>> -err = ret;
>> -goto out;
>> -}
>>  if (ret & LPT_SCAN_ADD) {
>>  /* Add all the nodes in path to the tree in memory */
>>  for (h = 1; h < c->lpt_hght; h++) {
>>
>>
>> Now I wonder how this test result should fit to my source code search 
>> pattern.
> 
> I guess there is a loop around this code, so it is reachable from itself.

* How can we achieve progress with such a bit of information?

* Should such false positives be avoided anyhow?

* Will answers to other questions become more helpful again?

* Are there further development challenges to consider for the safe 
identification
  of unique statements by metavariables?

Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] spatch / semantic patch missing some conversions?

[Julia Lawall]

On Thu, 17 Aug 2017, Dan Williams wrote:

> On Thu, Aug 17, 2017 at 11:10 AM, Dan Williams  
> wrote:
> > On Thu, Aug 17, 2017 at 10:06 AM, Julia Lawall  wrote:
> >> The updated semantic patch is below.  It makes one pass to collect the
> >> function names and then another pass to transform the functions, whereever
> >> they are defined.
> >>
> >> I have attached the output I get on today's linux-next.  If you want to
> >> run it your self, I suggest first running
> >>
> >> ~/coccinelle/scripts/idutils_index.sh
> >>
> >> in your kernel tree and then using the following command line:
> >>
> >> spatch.opt mmap.cocci --use-idutils --no-includes --include-headers 
> >> --in-place /run/shm/linux-next -j X --very-quiet
> >>
> >> where X is the number of core you have available.
> >>
> >> Let me know if there are any problems.
> >
> > Wow! Thank you, this is great.
>
> May I add you sign-off to the patch that goes upstream?

OK, sure.

julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Comparing statement lists with SmPL

[Julia Lawall]

On Thu, 17 Aug 2017, SF Markus Elfring wrote:

> >>> Can the search for duplicated source code be improved by the means of the
> >>> semantic patch language?
> >>
> >> For two statements at least you could do:
> >
> > An other SmPL script variant can work to some degree.
>
> How do you think about the relevance of the SmPL construct “<+... ...+>”
> for this use case?
>
>
> I tried my intermediate SmPL script variant a bit more.
>
> Example:
>
> elfring@Sonne:~/Projekte/Linux/next-patched> git checkout next-20170803 && 
> spatch.opt ~/Projekte/Coccinelle/janitor/show_same_statements3.cocci 
> fs/ubifs/lpt.c
> …
> @@ -1974,10 +1974,6 @@ again:
>   int ret, lnum = lprops->lnum;
>
>   ret = scan_cb(c, lprops, path[h].in_tree, data);
> - if (ret < 0) {
> - err = ret;
> - goto out;
> - }
>   if (ret & LPT_SCAN_ADD) {
>   /* Add all the nodes in path to the tree in memory */
>   for (h = 1; h < c->lpt_hght; h++) {
>
>
> Now I wonder how this test result should fit to my source code search pattern.

I guess there is a loop around this code, so it is reachable from itself.

julia

> I find also more results questionable for this Linux software module.
> It seems that there are only two functions which would be an acceptable match.
>
> Regards,
> Markus
>___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] spatch / semantic patch missing some conversions?

[Dan Williams]

On Thu, Aug 17, 2017 at 11:10 AM, Dan Williams  wrote:
> On Thu, Aug 17, 2017 at 10:06 AM, Julia Lawall  wrote:
>> The updated semantic patch is below.  It makes one pass to collect the
>> function names and then another pass to transform the functions, whereever
>> they are defined.
>>
>> I have attached the output I get on today's linux-next.  If you want to
>> run it your self, I suggest first running
>>
>> ~/coccinelle/scripts/idutils_index.sh
>>
>> in your kernel tree and then using the following command line:
>>
>> spatch.opt mmap.cocci --use-idutils --no-includes --include-headers 
>> --in-place /run/shm/linux-next -j X --very-quiet
>>
>> where X is the number of core you have available.
>>
>> Let me know if there are any problems.
>
> Wow! Thank you, this is great.

May I add you sign-off to the patch that goes upstream?
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] spatch / semantic patch missing some conversions?

[Dan Williams]

On Thu, Aug 17, 2017 at 10:06 AM, Julia Lawall  wrote:
> The updated semantic patch is below.  It makes one pass to collect the
> function names and then another pass to transform the functions, whereever
> they are defined.
>
> I have attached the output I get on today's linux-next.  If you want to
> run it your self, I suggest first running
>
> ~/coccinelle/scripts/idutils_index.sh
>
> in your kernel tree and then using the following command line:
>
> spatch.opt mmap.cocci --use-idutils --no-includes --include-headers 
> --in-place /run/shm/linux-next -j X --very-quiet
>
> where X is the number of core you have available.
>
> Let me know if there are any problems.

Wow! Thank you, this is great.
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

>>> Can the search for duplicated source code be improved by the means of the
>>> semantic patch language?
>>
>> For two statements at least you could do:
> 
> An other SmPL script variant can work to some degree.

How do you think about the relevance of the SmPL construct “<+... ...+>”
for this use case?


I tried my intermediate SmPL script variant a bit more.

Example:

elfring@Sonne:~/Projekte/Linux/next-patched> git checkout next-20170803 && 
spatch.opt ~/Projekte/Coccinelle/janitor/show_same_statements3.cocci 
fs/ubifs/lpt.c
…
@@ -1974,10 +1974,6 @@ again:
int ret, lnum = lprops->lnum;
 
ret = scan_cb(c, lprops, path[h].in_tree, data);
-   if (ret < 0) {
-   err = ret;
-   goto out;
-   }
if (ret & LPT_SCAN_ADD) {
/* Add all the nodes in path to the tree in memory */
for (h = 1; h < c->lpt_hght; h++) {


Now I wonder how this test result should fit to my source code search pattern.
I find also more results questionable for this Linux software module.
It seems that there are only two functions which would be an acceptable match.

Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] spatch / semantic patch missing some conversions?

[Julia Lawall]

The updated semantic patch is below.  It makes one pass to collect the
function names and then another pass to transform the functions, whereever
they are defined.

I have attached the output I get on today's linux-next.  If you want to
run it your self, I suggest first running

~/coccinelle/scripts/idutils_index.sh

in your kernel tree and then using the following command line:

spatch.opt mmap.cocci --use-idutils --no-includes --include-headers --in-place 
/run/shm/linux-next -j X --very-quiet

where X is the number of core you have available.

Let me know if there are any problems.

julia

virtual after_start

@initialize:ocaml@
@@

let tbl = Hashtbl.create(100)

let add_if_not_present fn =
  if not(Hashtbl.mem tbl fn) then Hashtbl.add tbl fn ()

@ a @
identifier fn;
identifier ops;
@@

struct file_operations ops = { ..., .mmap = fn, ...};

@script:ocaml@
fn << a.fn;
@@

add_if_not_present fn

@finalize:ocaml depends on !after_start@
tbls << merge.tbl;
@@

List.iter (fun t -> Hashtbl.iter (fun f _ -> add_if_not_present f) t) tbls;
Hashtbl.iter
(fun f _ ->
  let it = new iteration() in
  it#add_virtual_rule After_start;
  it#add_virtual_identifier Fn f;
  it#register())
tbl

@depends on after_start@
identifier virtual.fn;
identifier x, y;
@@

int fn(struct file *x,
struct vm_area_struct *y
-   )
+   , unsigned long map_flags)
{
...
}

@depends on after_start@
identifier virtual.fn;
identifier x, y;
@@

int fn(struct file *x,
struct vm_area_struct *y
-   );
+   , unsigned long map_flags);

@depends on after_start@
identifier virtual.fn;
@@

int fn(struct file *,
struct vm_area_struct *
-   );
+   , unsigned long);diff -u -p a/sound/core/compress_offload.c b/sound/core/compress_offload.c
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -391,7 +391,8 @@ out:
return retval;
 }
 
-static int snd_compr_mmap(struct file *f, struct vm_area_struct *vma)
+static int snd_compr_mmap(struct file *f, struct vm_area_struct *vma,
+ unsigned long map_flags)
 {
return -ENXIO;
 }
diff -u -p a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -444,7 +444,8 @@ static const struct vm_operations_struct
.page_mkwrite = sel_mmap_policy_fault,
 };
 
-static int sel_mmap_policy(struct file *filp, struct vm_area_struct *vma)
+static int sel_mmap_policy(struct file *filp, struct vm_area_struct *vma,
+  unsigned long map_flags)
 {
if (vma->vm_flags & VM_SHARED) {
/* do not allow mprotect to make mapping writable */
diff -u -p a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -109,7 +109,8 @@ extern int cifs_lock(struct file *, int,
 extern int cifs_fsync(struct file *, loff_t, loff_t, int);
 extern int cifs_strict_fsync(struct file *, loff_t, loff_t, int);
 extern int cifs_flush(struct file *, fl_owner_t id);
-extern int cifs_file_mmap(struct file * , struct vm_area_struct *);
+extern int cifs_file_mmap(struct file * , struct vm_area_struct *,
+ unsigned long);
 extern int cifs_file_strict_mmap(struct file * , struct vm_area_struct *);
 extern const struct file_operations cifs_dir_ops;
 extern int cifs_dir_open(struct inode *inode, struct file *file);
diff -u -p a/fs/cifs/file.c b/fs/cifs/file.c
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -3495,7 +3495,8 @@ int cifs_file_strict_mmap(struct file *f
return rc;
 }
 
-int cifs_file_mmap(struct file *file, struct vm_area_struct *vma)
+int cifs_file_mmap(struct file *file, struct vm_area_struct *vma,
+  unsigned long map_flags)
 {
int rc, xid;
 
diff -u -p a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -255,7 +255,8 @@ static struct drm_ioctl_desc vgem_ioctls
DRM_IOCTL_DEF_DRV(VGEM_FENCE_SIGNAL, vgem_fence_signal_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
 };
 
-static int vgem_mmap(struct file *filp, struct vm_area_struct *vma)
+static int vgem_mmap(struct file *filp, struct vm_area_struct *vma,
+unsigned long map_flags)
 {
unsigned long flags = vma->vm_flags;
int ret;
diff -u -p a/fs/ncpfs/ncp_fs.h b/fs/ncpfs/ncp_fs.h
--- a/fs/ncpfs/ncp_fs.h
+++ b/fs/ncpfs/ncp_fs.h
@@ -92,7 +92,7 @@ extern const struct file_operations ncp_
 int ncp_make_open(struct inode *, int);
 
 /* linux/fs/ncpfs/mmap.c */
-int ncp_mmap(struct file *, struct vm_area_struct *);
+int ncp_mmap(struct file *, struct vm_area_struct *, unsigned long);
 
 /* linux/fs/ncpfs/ncplib_kernel.c */
 int ncp_make_closed(struct inode *);
diff -u -p a/fs/ncpfs/mmap.c b/fs/ncpfs/mmap.c
--- a/fs/ncpfs/mmap.c
+++ b/fs/ncpfs/mmap.c
@@ -100,7 +100,8 @@ static const struct vm_operations_struct
 
 
 /* This is used for a general mmap of a 

Re: [Cocci] spatch / semantic patch missing some conversions?

[Julia Lawall]

On Thu, 17 Aug 2017, Dan Williams wrote:

> Hi,
>
> I'm trying to write a semantic patch that will add a flags argument to
> all mmap routines that appear in a struct file_operations instance.
> The semantic patch at the bottom of this mail mostly works, but I
> still end up needing to add rules like:
>
> @@
> identifier x, y;
> @@
>
> int bochs_mmap(struct file *x,
> struct vm_area_struct *y
> - )
> + , unsigned long map_flags)
> {
> ...
> }
>
> ...to catch straggling routines that get missed. What is confusing me
> is that when I run the below with --all-includes the *header* file
> where bochs_mmap() is declared is properly updated, but not the actual
> definition.  Is it possible that the identification of bochs_mmap() as
> an mmap operation occurs after we've already processed the file with
> the definition? That seems to be the common pattern among the failures
> I've seen (cirrus_mmap, qxl_mmap, hibmc_mmap...). I.e. occasions where
> the mmap routine is defined in a different file than the struct
> file_operations that consumes it get missed.
>
> ---
>
> @ a @
> identifier fn;
> identifier ops;
> @@
>
> struct file_operations ops = { ..., .mmap = fn, ...};
>
> @@
> identifier a.fn;
> identifier x, y;
> @@
>
> int fn(struct file *x,
> struct file *y

Actually, the main problem is here.  y should have type vm_area_struct.
So I guess that no function definitions were getting updated at all,
regardless of the file in which they were defined.  I have made the more
general rule in any case, and will send it as soon as it looks like
everything is ok.

julia

> -   )
> +   , unsigned long map_flags)
> {
> ...
> }
>
> @@
> identifier a.fn;
> identifier x, y;
> @@
>
> int fn(struct file *x,
> struct vm_area_struct *y
> -   );
> +   , unsigned long map_flags);
>
> @@
> identifier a.fn;
> @@
>
> int fn(struct file *,
> struct vm_area_struct *
> -   );
> +   , unsigned long);
> ___
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Improving size determinations with SmPL

[SF Markus Elfring]

>> ... when any should be <...
> 
> I have told you exactly what to do here.

You are right in the sense that I misinterpreted your change suggestion.


> The line above currently contains ... when any.  It (and only it) should be
> completely removed and changed to <...

But I find it inappropriate to enclose the function call code by an optional
SmPL nest construct.


> Nowhere do I suggest to put <... ... when any ...>

The suggested script variant seems to work finally.
Thanks for your information.

@replacement@
identifier action, var, work;
type T, X;
@@
 T work(...)
 {
 ... when any
 X* var;
 <...
 var = action(...,
  sizeof(
-X
+*var
),
  ...
 )
 ...>
 }


Now I wonder why the SmPL construct “<+... ...+>” does not produce the same
transformation result for the source file “fs/pstore/ram.c”.

Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Improving size determinations with SmPL

[SF Markus Elfring]

>>> ... when any should be ...>
>>
>> I am unsure if I interpret your suggestion correctly.
> 
> No.  Go read the original message again. It says exactly what to do and
> where it should be done.

Your hint might be nice.


> Eg, my message has two comments, but you have made three changes, which is 
> not correct.

I have tried out to convert the SmPL ellipsis into the optional dot variant.

But I wonder still about the error message “minus: parse error:”
for an adjusted SmPL script variant.

@replacement@
identifier action, var, work;
type T, X;
@@
 T work(...)
 {
 ... when any
 X* var;
 <... ... when any ...>
 var = action(...,
  sizeof(
-X
+*var
),
  ...
 )
 <... ... when any ...>
 }


Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Improving size determinations with SmPL

[Julia Lawall]

On Thu, 17 Aug 2017, SF Markus Elfring wrote:

> > ... when any should be ...>
>
> I am unsure if I interpret your suggestion correctly.

No.  Go read the original message again. It says exactly what to do and
where it should be done.  Eg, my message has two comments, but you have
made three changes, which is not correct.

julia

>
>
> @replacement@
> identifier action, var, work;
> type T, X;
> @@
>  T work(...)
>  {
>  <... ... when any ...>
>  X* var;
>  <... ... when any ...>
>  var = action(...,
>   sizeof(
> -X
> +*var
> ),
>   ...
>  )
>  <... ... when any ...>
>  }
>
>
> elfring@Sonne:~/Projekte/Linux/next-patched> git checkout next-20170803 && 
> spatch.opt ~/Projekte/Coccinelle/janitor/safer_size_determination2.cocci 
> fs/pstore/ram.c
> …
> minus: parse error:
>   File 
> "/home/elfring/Projekte/Coccinelle/janitor/safer_size_determination2.cocci", 
> line 7, column 6, charpos = 80
>   around = '...',
>   whole content =  <... ... when any ...>
>
>
>
> > You request that the sizeof appear on every execution path.
>
> I hope not. - I would like to express that this operator can appear for
> a parameter of a function call.
>
> Does the variable declaration “struct persistent_ram_zone *tmp_prz, 
> *prz_next;”
> in the function “ramoops_pstore_read” trigger challenges for the attempted
> source code transformation?
>
> Regards,
> Markus
>___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Improving size determinations with SmPL

[SF Markus Elfring]

> ... when any should be ...>

I am unsure if I interpret your suggestion correctly.


@replacement@
identifier action, var, work;
type T, X;
@@
 T work(...)
 {
 <... ... when any ...>
 X* var;
 <... ... when any ...>
 var = action(...,
  sizeof(
-X
+*var
),
  ...
 )
 <... ... when any ...>
 }


elfring@Sonne:~/Projekte/Linux/next-patched> git checkout next-20170803 && 
spatch.opt ~/Projekte/Coccinelle/janitor/safer_size_determination2.cocci 
fs/pstore/ram.c
…
minus: parse error: 
  File 
"/home/elfring/Projekte/Coccinelle/janitor/safer_size_determination2.cocci", 
line 7, column 6, charpos = 80
  around = '...',
  whole content =  <... ... when any ...>



> You request that the sizeof appear on every execution path.

I hope not. - I would like to express that this operator can appear for
a parameter of a function call.

Does the variable declaration “struct persistent_ram_zone *tmp_prz, *prz_next;”
in the function “ramoops_pstore_read” trigger challenges for the attempted
source code transformation?

Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] spatch / semantic patch missing some conversions?

[Julia Lawall]

On Thu, 17 Aug 2017, Dan Williams wrote:

> Hi,
>
> I'm trying to write a semantic patch that will add a flags argument to
> all mmap routines that appear in a struct file_operations instance.
> The semantic patch at the bottom of this mail mostly works, but I
> still end up needing to add rules like:
>
> @@
> identifier x, y;
> @@
>
> int bochs_mmap(struct file *x,
> struct vm_area_struct *y
> - )
> + , unsigned long map_flags)
> {
> ...
> }
>
> ...to catch straggling routines that get missed. What is confusing me
> is that when I run the below with --all-includes the *header* file
> where bochs_mmap() is declared is properly updated, but not the actual
> definition.  Is it possible that the identification of bochs_mmap() as
> an mmap operation occurs after we've already processed the file with
> the definition? That seems to be the common pattern among the failures
> I've seen (cirrus_mmap, qxl_mmap, hibmc_mmap...). I.e. occasions where
> the mmap routine is defined in a different file than the struct
> file_operations that consumes it get missed.

Coccinelle processes only one .c file at a time.  It doesn't learn anthing
about its processing in one file to extend the treatment of another file.
So when the file_operations definition and the function definition are in
separate files, the function definition will not normally be updated.  The
header file is probably included in both files, so it gets updated on its
inclusion in the file with the file_operations structure definition.

It is possible to iterate coccinelle, which should solve the problem.
Doing so should also improve performance, since it won't be necessary to
use --all-includes to pick up the declarations in the header files.  I
will fix it up and send you a new version shortly.

julia

>
> ---
>
> @ a @
> identifier fn;
> identifier ops;
> @@
>
> struct file_operations ops = { ..., .mmap = fn, ...};
>
> @@
> identifier a.fn;
> identifier x, y;
> @@
>
> int fn(struct file *x,
> struct file *y
> -   )
> +   , unsigned long map_flags)
> {
> ...
> }
>
> @@
> identifier a.fn;
> identifier x, y;
> @@
>
> int fn(struct file *x,
> struct vm_area_struct *y
> -   );
> +   , unsigned long map_flags);
>
> @@
> identifier a.fn;
> @@
>
> int fn(struct file *,
> struct vm_area_struct *
> -   );
> +   , unsigned long);
> ___
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

[Cocci] spatch / semantic patch missing some conversions?

[Dan Williams]

Hi,

I'm trying to write a semantic patch that will add a flags argument to
all mmap routines that appear in a struct file_operations instance.
The semantic patch at the bottom of this mail mostly works, but I
still end up needing to add rules like:

@@
identifier x, y;
@@

int bochs_mmap(struct file *x,
struct vm_area_struct *y
- )
+ , unsigned long map_flags)
{
...
}

...to catch straggling routines that get missed. What is confusing me
is that when I run the below with --all-includes the *header* file
where bochs_mmap() is declared is properly updated, but not the actual
definition.  Is it possible that the identification of bochs_mmap() as
an mmap operation occurs after we've already processed the file with
the definition? That seems to be the common pattern among the failures
I've seen (cirrus_mmap, qxl_mmap, hibmc_mmap...). I.e. occasions where
the mmap routine is defined in a different file than the struct
file_operations that consumes it get missed.

---

@ a @
identifier fn;
identifier ops;
@@

struct file_operations ops = { ..., .mmap = fn, ...};

@@
identifier a.fn;
identifier x, y;
@@

int fn(struct file *x,
struct file *y
-   )
+   , unsigned long map_flags)
{
...
}

@@
identifier a.fn;
identifier x, y;
@@

int fn(struct file *x,
struct vm_area_struct *y
-   );
+   , unsigned long map_flags);

@@
identifier a.fn;
@@

int fn(struct file *,
struct vm_area_struct *
-   );
+   , unsigned long);
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Improving size determinations with SmPL

[Julia Lawall]

On Thu, 17 Aug 2017, SF Markus Elfring wrote:

> Hello,
>
> I have constructed the following small script for another application of the
> semantic patch language.
>
>
> @replacement@
> identifier action, var, work;
> type T, X;
> @@
>  T work(...)
>  {
>  ... when any
>  X* var;
>  ... when any

... when any should be <...

>  var = action(...,
>   sizeof(
> -X
> +*var
> ),
>   ...
>  )
>  ... when any

... when any should be ...>

You request that the sizeof appear on every execution path.  <... ...>
will cause the transformation to be performed wherever appropriate, but
allow it not to be relevant on some execution paths.

julia

>  }
>
>
> I know that this transformation approach is working to some degree.
> I came along another source file where I do not get the desired code 
> adjustment
> with the software version “1.0.6-00186-g0acd38ee” automatically.
>
> elfring@Sonne:~/Projekte/Linux/next-patched> git checkout next-20170803 && 
> spatch.opt ~/Projekte/Coccinelle/janitor/safer_size_determination1.cocci 
> fs/pstore/ram.c
>
>
> How should the software situation be changed further so that the function
> “ramoops_pstore_read” will be also handled in the way like I published it in 
> the update suggestion “pstore: Improve a size determination in three 
> functions”?
> http://elixir.free-electrons.com/linux/v4.12.8/source/fs/pstore/ram.c#L291
> https://patchwork.kernel.org/patch/9904523/
> https://lkml.kernel.org/r/<23798ef4-5559-3454-ecf4-e6844b644...@users.sourceforge.net>
>
> Regards,
> Markus
> ___
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

[Cocci] Improving size determinations with SmPL

[SF Markus Elfring]

Hello,

I have constructed the following small script for another application of the
semantic patch language.


@replacement@
identifier action, var, work;
type T, X;
@@
 T work(...)
 {
 ... when any
 X* var;
 ... when any
 var = action(...,
  sizeof(
-X
+*var
),
  ...
 )
 ... when any
 }


I know that this transformation approach is working to some degree.
I came along another source file where I do not get the desired code adjustment
with the software version “1.0.6-00186-g0acd38ee” automatically.

elfring@Sonne:~/Projekte/Linux/next-patched> git checkout next-20170803 && 
spatch.opt ~/Projekte/Coccinelle/janitor/safer_size_determination1.cocci 
fs/pstore/ram.c


How should the software situation be changed further so that the function
“ramoops_pstore_read” will be also handled in the way like I published it in 
the update suggestion “pstore: Improve a size determination in three functions”?
http://elixir.free-electrons.com/linux/v4.12.8/source/fs/pstore/ram.c#L291
https://patchwork.kernel.org/patch/9904523/
https://lkml.kernel.org/r/<23798ef4-5559-3454-ecf4-e6844b644...@users.sourceforge.net>

Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH] Coccinelle: add atomic_as_refcounter script

[Julia Lawall]

> +identifier fname =~ ".*free.*";
> +identifier fname2 =~ ".*destroy.*";
> +identifier fname3 =~ ".*del.*";
> +identifier fname4 =~ ".*queue_work.*";
> +identifier fname5 =~ ".*schedule_work.*";
> +identifier fname6 =~ ".*call_rcu.*";

Personally, I find the above regular expressions much easier to understand
than the merged version that Markus proposed.  But the performance issue is
only on whether to use regular expressions or not.  If you use regular
expressions, Coccinelle will not do some optimizations.  But once you
decide to use regular expressions, the performance hit is already taken -
for a good cause here, to my understanding.  So just put whatever you find
convenient, in terms of readability, precision, etc.  It seems that del is
not very precise, because it is a substring of multiple words with
different meanings.  Maybe it should be improved, or maybe one can just
live with the false positives (eg delay), if they actually are false
positives.

julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci