Re: [Cocci] Checking statement order for patch generation with SmPL support
> > Execution can go from queuefree to the top of the loop, to the first if in > > the loop to the second if in the loop that has the dereference. That is > > how loops work. > > I can agree to this view. > > But I find the existence of a loop not so relevant for the source code > search pattern which is discussed. yes it is. If you make a pattern like A ... B It matches A, and then goes forth along all control flow paths, whether forwards or backwards, until it reaches a B. If there is a loop, it will go around the loop and match code that appears before A in terms of line numbers. The fact that in your case both A and B are in the same if branch is irrelevant. > > > >> @@ -1199,14 +1199,11 @@ void dpcm_be_disconnect(struct snd_soc_p > >>stream ? "<-" : "->", dpcm->be->dai_link->name); > >> > >>/* BEs still alive need new FE */ > >> - dpcm_be_reparent(fe, dpcm->be, stream); > >> > >> #ifdef CONFIG_DEBUG_FS > >> - debugfs_remove(dpcm->debugfs_state); > >> #endif > >>list_del(&dpcm->list_be); > >>list_del(&dpcm->list_fe); > >> - kfree(dpcm); > >>} > >> } > >> > >> > >> I find the shown matches also questionable for this test result. > >> Would you like to clarify such software situations a bit more > >> for the desired handling of statement sequences? > > > > The list_for_each_entry_safe operator also makes a loop. > > Yes. - But how could the Coccinelle software know more about this identifier > during execution of the small script “show_use_after_free3.cocci” than > that it is a macro call in the implementation of the function > “dpcm_be_disconnect” > (when extra include parameters were not specified)? > http://elixir.free-electrons.com/linux/v4.13/source/include/linux/list.h#L542 > http://elixir.free-electrons.com/linux/v4.13/source/sound/soc/soc-pcm.c#L1184 Coccinelle has a number of hard coded heuristics about macros, including knowing that list_for_each and ohter similar things represent loops. > Do you find the minus characters appropriate at the beginning of these three > lines? The behavior observed corresponds comepletely to the semantic patch you have written. julia___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking statement order for patch generation with SmPL support
>> I wonder also about the information how an ordinary for loop could influence >> the shown source code analysis result for the function >> “snd_seq_queue_find_name” >> when the questionable marked statements are contained in a single if branch. >> http://elixir.free-electrons.com/linux/v4.13/source/sound/core/seq/seq_queue.c#L241 >> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/sound/core/seq/seq_queue.c?id=c6be5a0e3cebc145127d46a58350e05d2bcf6323#n252 > > I don't understand the question. I hope that we can achieve a better common understanding also for the mentioned test examples. > I already explained the issue here. You tried it. > Execution can go from queuefree to the top of the loop, to the first if in > the loop to the second if in the loop that has the dereference. That is > how loops work. I can agree to this view. But I find the existence of a loop not so relevant for the source code search pattern which is discussed. >> @@ -1199,14 +1199,11 @@ void dpcm_be_disconnect(struct snd_soc_p >> stream ? "<-" : "->", dpcm->be->dai_link->name); >> >> /* BEs still alive need new FE */ >> -dpcm_be_reparent(fe, dpcm->be, stream); >> >> #ifdef CONFIG_DEBUG_FS >> -debugfs_remove(dpcm->debugfs_state); >> #endif >> list_del(&dpcm->list_be); >> list_del(&dpcm->list_fe); >> -kfree(dpcm); >> } >> } >> >> >> I find the shown matches also questionable for this test result. >> Would you like to clarify such software situations a bit more >> for the desired handling of statement sequences? > > The list_for_each_entry_safe operator also makes a loop. Yes. - But how could the Coccinelle software know more about this identifier during execution of the small script “show_use_after_free3.cocci” than that it is a macro call in the implementation of the function “dpcm_be_disconnect” (when extra include parameters were not specified)? http://elixir.free-electrons.com/linux/v4.13/source/include/linux/list.h#L542 http://elixir.free-electrons.com/linux/v4.13/source/sound/soc/soc-pcm.c#L1184 Do you find the minus characters appropriate at the beginning of these three lines? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking statement order for patch generation with SmPL support
> Thers is a control-flow path from the bottom of a loop back up to the top. I wonder also about the information how an ordinary for loop could influence the shown source code analysis result for the function “snd_seq_queue_find_name” when the questionable marked statements are contained in a single if branch. http://elixir.free-electrons.com/linux/v4.13/source/sound/core/seq/seq_queue.c#L241 https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/sound/core/seq/seq_queue.c?id=c6be5a0e3cebc145127d46a58350e05d2bcf6323#n252 I have tried the following SmPL script variant out on another source file. @usage@ identifier action!~"^.+free$", member, release=~"^.+free$"; expression context, ex; @@ *release(context); ... when != context = ex when any *action(..., (context)->member, ...) elfring@Sonne:~/Projekte/Linux/next-patched> spatch.opt ~/Projekte/Coccinelle/janitor/show_use_after_free3.cocci sound/soc/soc-pcm.c … @@ -1199,14 +1199,11 @@ void dpcm_be_disconnect(struct snd_soc_p stream ? "<-" : "->", dpcm->be->dai_link->name); /* BEs still alive need new FE */ - dpcm_be_reparent(fe, dpcm->be, stream); #ifdef CONFIG_DEBUG_FS - debugfs_remove(dpcm->debugfs_state); #endif list_del(&dpcm->list_be); list_del(&dpcm->list_fe); - kfree(dpcm); } } I find the shown matches also questionable for this test result. Would you like to clarify such software situations a bit more for the desired handling of statement sequences? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking statement order for patch generation with SmPL support
On Thu, 7 Sep 2017, SF Markus Elfring wrote: > > Thers is a control-flow path from the bottom of a loop back up to the top. > > I wonder also about the information how an ordinary for loop could influence > the shown source code analysis result for the function > “snd_seq_queue_find_name” > when the questionable marked statements are contained in a single if branch. > http://elixir.free-electrons.com/linux/v4.13/source/sound/core/seq/seq_queue.c#L241 > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/sound/core/seq/seq_queue.c?id=c6be5a0e3cebc145127d46a58350e05d2bcf6323#n252 I don't understand the question. I already explained the issue here. Execution can go from queuefree to the top of the loop, to the first if in the loop to the second if in the loop that has the dereference. That is how loops work. > > I have tried the following SmPL script variant out on another source file. > > @usage@ > identifier action!~"^.+free$", member, release=~"^.+free$"; > expression context, ex; > @@ > *release(context); > ... when != context = ex > when any > *action(..., (context)->member, ...) > > > elfring@Sonne:~/Projekte/Linux/next-patched> spatch.opt > ~/Projekte/Coccinelle/janitor/show_use_after_free3.cocci sound/soc/soc-pcm.c > … > @@ -1199,14 +1199,11 @@ void dpcm_be_disconnect(struct snd_soc_p > stream ? "<-" : "->", dpcm->be->dai_link->name); > > /* BEs still alive need new FE */ > - dpcm_be_reparent(fe, dpcm->be, stream); > > #ifdef CONFIG_DEBUG_FS > - debugfs_remove(dpcm->debugfs_state); > #endif > list_del(&dpcm->list_be); > list_del(&dpcm->list_fe); > - kfree(dpcm); > } > } > > > I find the shown matches also questionable for this test result. > Would you like to clarify such software situations a bit more > for the desired handling of statement sequences? The list_for_each_entry_safe operator also makes a loop. julia___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking statement order for patch generation with SmPL support
>> But I have got difficulties to interpret it in an useful way. > > Coccinelle follows control-flow paths. This information is generally fine. > Thers is a control-flow path from the bottom of a loop back up to the top. I can not follow with my intermediate understanding to such a view at the moment. Why was the combination of a call for the function “strncmp” before a “queuefree” really displayed as an update candidate? >>> @usage@ >>> identifier action, member, release=~"^.+free$"; >>> expression context,e; >>> @@ >>> *release(context); >>> ... when != context = e >>> when any // to get all results >>> *action(..., (context)->member, ...) >> >> Should the SmPL construct “<+.. ...+>” work also similar to your suggestion? > > <+... ...+> would also not allow context = e after the last match of the > pattern inside the nest. This information sounds promising. I am looking still for possibilities to clarify the overlap better in the shown functionality. > A when on a <+... ...+> applies to the entire matched region. Can the parameter “when” be combined with this construct anyhow? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking statement order for patch generation with SmPL support
On Thu, 7 Sep 2017, SF Markus Elfring wrote: > >> Now I wonder why the software “Coccinelle 1.0.6-00242-g3f038a5d” finds > >> this place relevant when the function call sequence does not fit to the > >> order > >> I tried to express for a known use case. > >> I would appreciate further advice. > > > > Because there is a loop, > > This information is appropriate. > > But I have got difficulties to interpret it in an useful way. Coccinelle follows control-flow paths. Thers is a control-flow path from the bottom of a loop back up to the top. > > > > and you did nothing to prevent an update to q because the free and the > > dereference. > > I omitted an additional constraint for a simple test. > > > > @usage@ > > identifier action, member, release=~"^.+free$"; > > expression context,e; > > @@ > > *release(context); > > ... when != context = e > > when any // to get all results > > *action(..., (context)->member, ...) > > Should the SmPL construct “<+.. ...+>” work also similar to your suggestion? <+... ...+> would also not allow context = e after the last match of the pattern inside the nest. A when on a <+... ...+> applies to the entire matched region. julia___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking statement order for patch generation with SmPL support
>> Now I wonder why the software “Coccinelle 1.0.6-00242-g3f038a5d” finds >> this place relevant when the function call sequence does not fit to the order >> I tried to express for a known use case. >> I would appreciate further advice. > > Because there is a loop, This information is appropriate. But I have got difficulties to interpret it in an useful way. > and you did nothing to prevent an update to q because the free and the > dereference. I omitted an additional constraint for a simple test. > @usage@ > identifier action, member, release=~"^.+free$"; > expression context,e; > @@ > *release(context); > ... when != context = e > when any // to get all results > *action(..., (context)->member, ...) Should the SmPL construct “<+.. ...+>” work also similar to your suggestion? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Checking statement order for patch generation with SmPL support
Hello,
I have constructed another small script for the semantic patch language.
@usage@
identifier action, member, release=~"^.+free$";
expression context;
@@
*release(context);
<+...
*action(..., (context)->member, ...)
...+>
The following source code place can be found by such a simple approach
for further software development considerations.
https://lkml.org/lkml/2017/9/6/669
elfring@Sonne:~/Projekte/Linux/next-patched> git checkout next-20170905 &&
spatch.opt ~/Projekte/Coccinelle/janitor/show_use_after_free1.cocci
sound/pci/ymfpci/ymfpci.c
…
@@ -336,8 +336,6 @@ static int snd_card_ymfpci_probe(struct
legacy_ctrl &= ~YMFPCI_LEGACY_FMEN;
pci_write_config_word(pci, PCIR_DSXG_LEGACY,
legacy_ctrl);
} else if ((err = snd_opl3_hwdep_new(opl3, 0, 1, NULL)) < 0) {
- snd_card_free(card);
- dev_err(card->dev, "cannot create opl3 hwdep\n");
return err;
}
}
I have tried the SmPL script out on another source file.
elfring@Sonne:~/Projekte/Linux/next-patched> spatch.opt
~/Projekte/Coccinelle/janitor/show_use_after_free1.cocci
sound/core/seq/seq_queue.c
…
@@ -246,9 +246,7 @@ struct snd_seq_queue *snd_seq_queue_find
for (i = 0; i < SNDRV_SEQ_MAX_QUEUES; i++) {
if ((q = queueptr(i)) != NULL) {
- if (strncmp(q->name, name, sizeof(q->name)) == 0)
return q;
- queuefree(q);
}
}
return NULL;
Now I wonder why the software “Coccinelle 1.0.6-00242-g3f038a5d” finds
this place relevant when the function call sequence does not fit to the order
I tried to express for a known use case.
I would appreciate further advice.
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking statement order for patch generation with SmPL support
On Thu, 7 Sep 2017, SF Markus Elfring wrote:
> Hello,
>
> I have constructed another small script for the semantic patch language.
>
> @usage@
> identifier action, member, release=~"^.+free$";
> expression context;
> @@
> *release(context);
> <+...
> *action(..., (context)->member, ...)
> ...+>
>
>
> The following source code place can be found by such a simple approach
> for further software development considerations.
> https://lkml.org/lkml/2017/9/6/669
>
> elfring@Sonne:~/Projekte/Linux/next-patched> git checkout next-20170905 &&
> spatch.opt ~/Projekte/Coccinelle/janitor/show_use_after_free1.cocci
> sound/pci/ymfpci/ymfpci.c
> …
> @@ -336,8 +336,6 @@ static int snd_card_ymfpci_probe(struct
> legacy_ctrl &= ~YMFPCI_LEGACY_FMEN;
> pci_write_config_word(pci, PCIR_DSXG_LEGACY,
> legacy_ctrl);
> } else if ((err = snd_opl3_hwdep_new(opl3, 0, 1, NULL)) < 0) {
> - snd_card_free(card);
> - dev_err(card->dev, "cannot create opl3 hwdep\n");
> return err;
> }
> }
>
>
> I have tried the SmPL script out on another source file.
>
> elfring@Sonne:~/Projekte/Linux/next-patched> spatch.opt
> ~/Projekte/Coccinelle/janitor/show_use_after_free1.cocci
> sound/core/seq/seq_queue.c
> …
> @@ -246,9 +246,7 @@ struct snd_seq_queue *snd_seq_queue_find
>
> for (i = 0; i < SNDRV_SEQ_MAX_QUEUES; i++) {
> if ((q = queueptr(i)) != NULL) {
> - if (strncmp(q->name, name, sizeof(q->name)) == 0)
> return q;
> - queuefree(q);
> }
> }
> return NULL;
>
>
> Now I wonder why the software “Coccinelle 1.0.6-00242-g3f038a5d” finds
> this place relevant when the function call sequence does not fit to the order
> I tried to express for a known use case.
> I would appreciate further advice.
Because there is a loop, and you did nothing to prevent an update to q
because the free and the dereference.
The rule would be just as well as:
@usage@
identifier action, member, release=~"^.+free$";
expression context,e;
@@
*release(context);
... when != context = e // to get the first result
*action(..., (context)->member, ...)
or
@usage@
identifier action, member, release=~"^.+free$";
expression context,e;
@@
*release(context);
... when != context = e
when any // to get all results
*action(..., (context)->member, ...)
julia___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
