[Cocci] C++ Parse error with new in else
I know C++ support isn't really finished but thought I'd point out this
strange bug I found just in case some body wanted to fix it.
Simple C++ program:
struct foo
{
int i;
};
int main(int argc, const char* argv[])
{
foo * a;
if (argc == 1) {
a = new foo;
a->i = argc;
} else {
//a = new foo; // line is marked BAD:! if uncommented
a->i = argc + 1;
}
delete a;
return 0;
}
Simple cocci file:
@@
@@
- a
+ b
I run:
spatch -sp_file cocci_broken.cocci cocci_broken.cpp --c++ --debug
It works fine as long as that first line in the else is commented out. If I
uncomment it I get:
Parse error
= error in cocci_broken.cpp; set verbose_parsing for more info
badcount: 17
bad: };
bad:
bad: int main(int argc, const char* argv[])
bad: {
bad: foo * a;
bad:
bad: if (argc == 1) {
bad: a = new foo;
bad: a->i = argc;
bad: } else {
BAD:! a = new foo; // line is marked BAD:! if uncommented
bad: a->i = argc + 1;
bad: }
bad:
bad: delete a;
bad:
bad: return 0;
bad: }
I'm using spatch version 1.0.8.
It seems quite odd everything is fine until that line is enabled since it's
the same thing that's in the if clause.
Cheers,
George McCollister
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] using cocci to switch to formatted log/print function
On Fri, Mar 27, 2020 at 1:37 PM Julia Lawall wrote:
>
>
>
> On Fri, 27 Mar 2020, George McCollister wrote:
>
> > Hello,
> >
> > I'm working with an old code base that makes excessive use of the
> > following sort of construct:
> >
> > char display[128];
> >
> > ...
> >
> > snprintf(display, sizeof(display), "example log message %d", i);
> > log_buffer(level, strlen(display)+1, display);
> >
> > I'd like to replace this mess with a single call to a function named
> > log_formatted(). This involves moving the format string and arguments
> > passed to snprintf into the log_formatted() call, removing snprintf
> > call and (here's where it gets a bit tricky) remove the buffer if it
> > isn't used for anything else.
> >
> > I have this all working with the following script with the caveat that
> > running it on moderately complicated source files makes it never
> > finish (after an hour or so the spatch process crashes with a stack
> > overflow error).
> > I've tried --no-loops which seem to speed things up but complicated
> > source files still result in it never finishing.
> >
> > @r1@
> > type T;
> > identifier disp;
> > expression level;
> > expression list prnt;
> > @@
> >
> > {
> > ... when any
>
> The above is not necessary. YOu don't have to start the match from a
> block. You can just start it from the declaration.
>
> On the other hand, the declaration may not be needed either. After your
> metavariable type T, you can put
>
> local idexpression T[] disp2;
>
> Then the match should be:
>
> snprintf(disp2@disp, sizeof(disp), prnt);
> ... when != disp
> - log_buffer(level, strlen(disp)+1, disp);
> + log_formatted(level, prnt);
>
> The notation disp2@disp will match both disp2 (idexpression of a
> particular type) and disp (identifier) against the first argument of
> snprintf. This is needed because 1) you want to give a type, which
> requires and idexpression, and 2) in later rules you want to change a
> variable declaration, which requires an identifier.
>
>
> > (
> > T disp[...];
> > |
> > T disp[...]="";
> > )
> > <+...
> > snprintf(disp, sizeof(disp), prnt);
> > ... when != disp
> > - log_buffer(level, strlen(disp)+1, disp);
> > + log_formatted(level, prnt);
> > ...+>
> > }
> >
> > // Only remove the display variable and snprintf if there are no
> > // other references to the variable.
> >
> > @r2 depends on r1@
> > type r1.T;
> > identifier r1.disp;
> > expression list r1.prnt;
> > @@
> >
> > {
> > ... when any
> > (
> > - T disp[...];
> > |
> > - T disp[...]="";
> > )
> > <+... when != disp
> > - snprintf(disp, sizeof(disp), prnt);
> > ...+>
> > }
>
> Here you could try considering the problem from the opposite point of
> view.
This was the key to getting it work, thanks!
>
> @r2 exists@ // exists is what helps you drop the complexity, by needing to
> type r1.T; // find only one matching path
> identifier r1.disp;;
> expression list r1.prnt;
> position p;
> @@
>
> (
> T@p disp[...];
> |
> T@p disp[...]="";
> )
> ...
> snprintf(disp, sizeof(disp), prnt);
>
> @r3@
> position p != r2.p;
> type r1.T;
> identifier r1.disp;;
> @@
>
> (
> - T@p disp[...];
> |
> - T@p disp[...]="";
> )
>
>
> Does it matter that the initial value of disp is ""? In the proposed
> first rule I have dropped that constraint.
>
> julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [v2 04/10] mmap locking API: use coccinelle to convert mmap_sem rwsem call sites
On Fri, Mar 27, 2020 at 1:00 AM Markus Elfring wrote: > > >>> The change is generated using coccinelle with the following rules: > >> > >> Would you like to apply only a single SmPL rule here? > > > > I think this version of the patch is already a single rule, > > similar to what you suggested ? > > Yes. - But you repeated the wording “rules” in the change description. > Are there any other software extensions still in the waiting queue? Ah yes, I did not change the wording before the included .cocci file. I do not have any more coccinelle patches to push in the near future. ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2 04/10] mmap locking API: use coccinelle to convert mmap_sem rwsem call sites
On Fri, Mar 27, 2020 at 12:22 AM Markus Elfring wrote: > > > This change converts the existing mmap_sem rwsem calls to use the new > > mmap locking API instead. > > > > The change is generated using coccinelle with the following rules: > > Would you like to apply only a single SmPL rule here? I think this version of the patch is already a single rule, similar to what you suggested ? > > // spatch --sp-file mmap_lock_api.cocci --in-place --include-headers --dir . > > Command parameters like “--jobs 8 --chunksize 1” can be also helpful > for a parallel execution of the desired software transformation. > > I suggest to consider another possibility for a bit of fine-tuning in the > shown > SmPL script if you would eventually care for nicer run time characteristics > for the application of such a SmPL disjunction. > How do you think about to order the elements according to a probable > function call frequency? I'm not sure it matters that much, as long as it produces the correct end result. The run takes about 25 seconds before any optimizations, which I find very acceptable. -- Michel "Walken" Lespinasse A program is never fully debugged until the last user dies. ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] using cocci to switch to formatted log/print function
On Fri, 27 Mar 2020, George McCollister wrote:
> Hello,
>
> I'm working with an old code base that makes excessive use of the
> following sort of construct:
>
> char display[128];
>
> ...
>
> snprintf(display, sizeof(display), "example log message %d", i);
> log_buffer(level, strlen(display)+1, display);
>
> I'd like to replace this mess with a single call to a function named
> log_formatted(). This involves moving the format string and arguments
> passed to snprintf into the log_formatted() call, removing snprintf
> call and (here's where it gets a bit tricky) remove the buffer if it
> isn't used for anything else.
>
> I have this all working with the following script with the caveat that
> running it on moderately complicated source files makes it never
> finish (after an hour or so the spatch process crashes with a stack
> overflow error).
> I've tried --no-loops which seem to speed things up but complicated
> source files still result in it never finishing.
>
> @r1@
> type T;
> identifier disp;
> expression level;
> expression list prnt;
> @@
>
> {
> ... when any
The above is not necessary. YOu don't have to start the match from a
block. You can just start it from the declaration.
On the other hand, the declaration may not be needed either. After your
metavariable type T, you can put
local idexpression T[] disp2;
Then the match should be:
snprintf(disp2@disp, sizeof(disp), prnt);
... when != disp
- log_buffer(level, strlen(disp)+1, disp);
+ log_formatted(level, prnt);
The notation disp2@disp will match both disp2 (idexpression of a
particular type) and disp (identifier) against the first argument of
snprintf. This is needed because 1) you want to give a type, which
requires and idexpression, and 2) in later rules you want to change a
variable declaration, which requires an identifier.
> (
> T disp[...];
> |
> T disp[...]="";
> )
> <+...
> snprintf(disp, sizeof(disp), prnt);
> ... when != disp
> - log_buffer(level, strlen(disp)+1, disp);
> + log_formatted(level, prnt);
> ...+>
> }
>
> // Only remove the display variable and snprintf if there are no
> // other references to the variable.
>
> @r2 depends on r1@
> type r1.T;
> identifier r1.disp;
> expression list r1.prnt;
> @@
>
> {
> ... when any
> (
> - T disp[...];
> |
> - T disp[...]="";
> )
> <+... when != disp
> - snprintf(disp, sizeof(disp), prnt);
> ...+>
> }
Here you could try considering the problem from the opposite point of
view.
@r2 exists@ // exists is what helps you drop the complexity, by needing to
type r1.T; // find only one matching path
identifier r1.disp;;
expression list r1.prnt;
position p;
@@
(
T@p disp[...];
|
T@p disp[...]="";
)
...
snprintf(disp, sizeof(disp), prnt);
@r3@
position p != r2.p;
type r1.T;
identifier r1.disp;;
@@
(
- T@p disp[...];
|
- T@p disp[...]="";
)
Does it matter that the initial value of disp is ""? In the proposed
first rule I have dropped that constraint.
julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] using cocci to switch to formatted log/print function
Hello,
I'm working with an old code base that makes excessive use of the
following sort of construct:
char display[128];
...
snprintf(display, sizeof(display), "example log message %d", i);
log_buffer(level, strlen(display)+1, display);
I'd like to replace this mess with a single call to a function named
log_formatted(). This involves moving the format string and arguments
passed to snprintf into the log_formatted() call, removing snprintf
call and (here's where it gets a bit tricky) remove the buffer if it
isn't used for anything else.
I have this all working with the following script with the caveat that
running it on moderately complicated source files makes it never
finish (after an hour or so the spatch process crashes with a stack
overflow error).
I've tried --no-loops which seem to speed things up but complicated
source files still result in it never finishing.
@r1@
type T;
identifier disp;
expression level;
expression list prnt;
@@
{
... when any
(
T disp[...];
|
T disp[...]="";
)
<+...
snprintf(disp, sizeof(disp), prnt);
... when != disp
- log_buffer(level, strlen(disp)+1, disp);
+ log_formatted(level, prnt);
...+>
}
// Only remove the display variable and snprintf if there are no
// other references to the variable.
@r2 depends on r1@
type r1.T;
identifier r1.disp;
expression list r1.prnt;
@@
{
... when any
(
- T disp[...];
|
- T disp[...]="";
)
<+... when != disp
- snprintf(disp, sizeof(disp), prnt);
...+>
}
Any suggestions on changes to my script that would make this work on
lengthy source files would be greatly appreciated!
Thanks,
George McCollister
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Matching against a declarer macro
On Fri, 27 Mar 2020, Christoph Böhmwalder wrote: > Am 27.03.20 um 16:47 schrieb Julia Lawall:> Are you sure that the C code is > parsed successfully? I'm not at all sure > > that static is allowed in an argument list. Types are allowed, bu static > > is only part of a type. > > I'm pretty sure it is parsed successfully. At least spatch doesn't complain > about it, no matter how many debug flags I specify. Did you try spatch --parse-c file.c? > > I really only want to swap out the last parameter, but I'm having trouble > coming up with the syntax. This should match, right? > > @@ > declarer name RB_DECLARE_CALLBACKS_MAX; > identifier NODE_END; > @@ > RB_DECLARE_CALLBACKS_MAX(..., > - NODE_END > + compute_subtree_last > ); If the code is getting parsed, this should be fine. Do you want to actually match NODE_END? If so, it shouldn't be declared as a metavariable. julia > > For augment_callbacks, either identifier or expression would be fine. > > Coccinelle has no idea what is going to happen to augment_callbacks > > afterwards. It just sees a sequence of characters and classifies it as an > > identifier. > > That's what I thought, thank you for confirming. > > > julia >___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Matching against a declarer macro
Am 27.03.20 um 16:47 schrieb Julia Lawall:> Are you sure that the C code is parsed successfully? I'm not at all sure that static is allowed in an argument list. Types are allowed, bu static is only part of a type. I'm pretty sure it is parsed successfully. At least spatch doesn't complain about it, no matter how many debug flags I specify. I really only want to swap out the last parameter, but I'm having trouble coming up with the syntax. This should match, right? @@ declarer name RB_DECLARE_CALLBACKS_MAX; identifier NODE_END; @@ RB_DECLARE_CALLBACKS_MAX(..., - NODE_END + compute_subtree_last ); For augment_callbacks, either identifier or expression would be fine. Coccinelle has no idea what is going to happen to augment_callbacks afterwards. It just sees a sequence of characters and classifies it as an identifier. That's what I thought, thank you for confirming. julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Matching against a declarer macro
On Fri, 27 Mar 2020, Christoph Böhmwalder wrote: > Hi again, > > I'm having a little trouble matching against this line of code: > > RB_DECLARE_CALLBACKS_MAX(static, augment_callbacks, struct > drbd_interval, rb, sector_t, end, NODE_END); > > This is especially tricky because it contains a lot of macro magic. > I think the biggest problem is the first argument, which is the keyword > "static". What do I use to match against this? expression? identifier? > symbol? > > Also, the "augment_callbacks" is not really an identifier either, it > just gets used to generate the function names. But what is it? An > expression? > > @@ > typedef sector_t; > declarer name RB_DECLARE_CALLBACKS_MAX; > > identifier augment_callbacks; > identifier rb; > identifier end; > identifier NODE_END; > @@ > -RB_DECLARE_CALLBACKS_MAX(static, augment_callbacks, struct > drbd_interval, rb, sector_t, end, NODE_END); > > Nothing I have tried has made it match yet. > > Any ideas on how to solve this would be appreciated, thanks! Are you sure that the C code is parsed successfully? I'm not at all sure that static is allowed in an argument list. Types are allowed, bu static is only part of a type. For augment_callbacks, either identifier or expression would be fine. Coccinelle has no idea what is going to happen to augment_callbacks afterwards. It just sees a sequence of characters and classifies it as an identifier. julia > > -- > Christoph Böhmwalder > LINBIT | Keeping the Digital World Running > DRBD HA — Disaster Recovery — Software defined Storage > ___ > 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] Matching against a declarer macro
Hi again, I'm having a little trouble matching against this line of code: RB_DECLARE_CALLBACKS_MAX(static, augment_callbacks, struct drbd_interval, rb, sector_t, end, NODE_END); This is especially tricky because it contains a lot of macro magic. I think the biggest problem is the first argument, which is the keyword "static". What do I use to match against this? expression? identifier? symbol? Also, the "augment_callbacks" is not really an identifier either, it just gets used to generate the function names. But what is it? An expression? @@ typedef sector_t; declarer name RB_DECLARE_CALLBACKS_MAX; identifier augment_callbacks; identifier rb; identifier end; identifier NODE_END; @@ -RB_DECLARE_CALLBACKS_MAX(static, augment_callbacks, struct drbd_interval, rb, sector_t, end, NODE_END); Nothing I have tried has made it match yet. Any ideas on how to solve this would be appreciated, thanks! -- Christoph Böhmwalder LINBIT | Keeping the Digital World Running DRBD HA — Disaster Recovery — Software defined Storage ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2 05/10] mmap locking API: convert mmap_sem call sites missed by coccinelle
> Convert the last few remaining mmap_sem rwsem calls to use the new > mmap locking API. These were missed by coccinelle for some reason I find such a software situation still unfortunate. * Should the data processing results be clarified any further for the shown transformation approach? * Will additional improvements be considered? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [v2 04/10] mmap locking API: use coccinelle to convert mmap_sem rwsem call sites
>>> The change is generated using coccinelle with the following rules: >> >> Would you like to apply only a single SmPL rule here? > > I think this version of the patch is already a single rule, > similar to what you suggested ? Yes. - But you repeated the wording “rules” in the change description. Are there any other software extensions still in the waiting queue? > I'm not sure it matters that much, as long as it produces the correct > end result. The run takes about 25 seconds before any optimizations, > which I find very acceptable. I am used to look also at the run time characteristics of SmPL script execution. The discussed SmPL code can be good enough for your current needs. Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2 04/10] mmap locking API: use coccinelle to convert mmap_sem rwsem call sites
> This change converts the existing mmap_sem rwsem calls to use the new > mmap locking API instead. > > The change is generated using coccinelle with the following rules: Would you like to apply only a single SmPL rule here? > // spatch --sp-file mmap_lock_api.cocci --in-place --include-headers --dir . Command parameters like “--jobs 8 --chunksize 1” can be also helpful for a parallel execution of the desired software transformation. I suggest to consider another possibility for a bit of fine-tuning in the shown SmPL script if you would eventually care for nicer run time characteristics for the application of such a SmPL disjunction. How do you think about to order the elements according to a probable function call frequency? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
