Re: [Cocci] [PATCH v2] coccinelle: locks: Add balancedlock.cocci script

2020-11-18 Thread Julia Lawall
> > +++ b/scripts/coccinelle/locks/balancedlock.cocci
> …
> > +//# False positives may be generated due to locks released within a nested
> > +//# function call or a goto block.
> > +///
> > +// Confidence: Moderate
>
> How good does such information fit together?
>

What kind of response do you expect?  There are some concerns, so it's not
High confidence.  It works pretty well so it's not low confidence.  So
it's moderate confidence.  What else is there to say?

> …
> >+ (
> > +mutex_lock@p(E);
> > +|
> > +read_lock@p(E);
> > +|
> …
>
> Why did you not reorder the elements of such a SmPL disjunctions according to
> an usage incidence (which can be determined by another SmPL script like
> “report_lock_calls.cocci”)?

I don't recall ever seeing any evidence that it has an impact for function
calls.  Furthermore, the numbers will change over time.  So why change it?

> …
> > +msg = "This code segment might have an unbalanced lock."
> > +coccilib.org.print_todo(j0[0], msg)
>
> Please pass the string literal directly.
>
> +coccilib.org.print_todo(j0[0], "This code segment might have an unbalanced 
> lock.")

The code is fine as it is.

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


Re: [Cocci] [PATCH v2] coccinelle: locks: Add balancedlock.cocci script

2020-11-18 Thread Markus Elfring
> Changes in v2(as suggested by Markus):

Thanks you picked a few suggestions up.


I would appreciate further constructive clarifications.


…
> +++ b/scripts/coccinelle/locks/balancedlock.cocci
…
> +//# False positives may be generated due to locks released within a nested
> +//# function call or a goto block.
> +///
> +// Confidence: Moderate

How good does such information fit together?


> +// Copyright: (C) 2020 Julia Lawall INRIA/LIP6
> +// Copyright: (C) 2020 Sumera Priyadarsini

Does this information indicate that the shown script for the semantic patch 
language
was a development result from another collaboration?


…
>+ (
> +mutex_lock@p(E);
> +|
> +read_lock@p(E);
> +|
…

Why did you not reorder the elements of such a SmPL disjunctions according to
an usage incidence (which can be determined by another SmPL script like
“report_lock_calls.cocci”)?


…
> +@balanced2 depends on context || org || report@
> +identifier lock, unlock = {mutex_unlock,
…

Are there any chances to avoid the repetition of the function name list
for this SmPL constraint?


…
> +msg = "This code segment might have an unbalanced lock."
> +coccilib.org.print_todo(j0[0], msg)

Please pass the string literal directly.

+coccilib.org.print_todo(j0[0], "This code segment might have an unbalanced 
lock.")


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


[Cocci] [PATCH v2] coccinelle: locks: Add balancedlock.cocci script

2020-11-18 Thread Sumera Priyadarsini
When acquiring locks under certain conditions, they must be released
under the same conditions as well. However, sometimes, there may be
missing unlocks which may lead to a potential deadlock.

Add this script to detect such code segments and avoid potential
deadlock situations.

Signed-off-by: Sumera Priyadarsini 
---
Changes in v2(as suggested by Markus):
- Modify usage of position variable
- Modify comments
- Add dependencies for rules
---
 scripts/coccinelle/locks/balancedlock.cocci | 164 
 1 file changed, 164 insertions(+)
 create mode 100644 scripts/coccinelle/locks/balancedlock.cocci

diff --git a/scripts/coccinelle/locks/balancedlock.cocci 
b/scripts/coccinelle/locks/balancedlock.cocci
new file mode 100644
index ..9684a9920f79
--- /dev/null
+++ b/scripts/coccinelle/locks/balancedlock.cocci
@@ -0,0 +1,164 @@
+/// Sometimes, locks that are acquired under certain conditions may have
+/// missing unlocks leading to a potential deadlock situation. This
+/// semantic patch detects such cases.
+//# False positives may be generated due to locks released within a nested
+//# function call or a goto block.
+///
+// Confidence: Moderate
+// Copyright: (C) 2020 Julia Lawall INRIA/LIP6
+// Copyright: (C) 2020 Sumera Priyadarsini
+
+virtual context
+virtual org
+virtual report
+
+
+@prelocked@
+expression E;
+position p;
+@@
+
+(
+mutex_lock@p(E);
+|
+read_lock@p(E);
+|
+write_lock@p(E);
+|
+spin_lock@p(E);
+|
+spin_lock_bh@p(E);
+|
+spin_lock_irqsave@p(E, ...);
+|
+read_lock_irqsave@p(E, ...);
+|
+write_lock_irqsave@p(E, ...);
+|
+raw_spin_lock@p(E);
+|
+raw_spin_lock_irq@p(E);
+|
+raw_spin_lock_bh@p(E);
+|
+local_lock@p(E);
+|
+local_lock_irq@p(E);
+|
+local_lock_irqsave@p(E, ...);
+|
+read_lock_irq@p(E);
+|
+read_lock_bh@p(E);
+|
+write_lock_bh@p(E);
+)
+
+@balanced@ depends on context || org || report@
+position prelocked.p;
+position pif;
+expression e,prelocked.E;
+statement S1,S2;
+identifier lock;
+identifier unlock={mutex_unlock,
+   spin_unlock,
+   spin_unlock_bh,
+   spin_unlock_irqrestore,
+   read_unlock_irqrestore,
+   write_unlock_irqrestore,
+   raw_spin_unlock,
+   raw_spin_unlock_irq,
+   raw_spin_unlock_bh,
+   local_unlock,
+   local_unlock_irq,
+   local_unlock_irqrestore,
+   read_unlock_irq,
+   read_unlock_bh,
+   write_unlock_bh
+   };
+@@
+
+if (e) {
+ ... when any
+lock@p(E, ...)
+ ... when != E
+ when any
+} else S1
+... when != E
+when any
+if@pif (e) {
+ ... when != E
+ when any
+ unlock(E, ...);
+ ... when any
+} else S2
+...  when != E
+ when any
+
+// 
+
+@balanced2 depends on context || org || report@
+identifier lock, unlock = {mutex_unlock,
+   spin_unlock,
+   spin_unlock_bh,
+   spin_unlock_irqrestore,
+   read_unlock_irqrestore,
+   write_unlock_irqrestore,
+   raw_spin_unlock,
+   raw_spin_unlock_irq,
+   raw_spin_unlock_bh,
+   local_unlock,
+   local_unlock_irq,
+   local_unlock_irqrestore,
+   read_unlock_irq,
+   read_unlock_bh,
+   write_unlock_bh
+   };
+expression E, f, x;
+statement S1, S2, S3, S4;
+position prelocked.p, balanced.pif;
+position j0, j1, j2, j3;
+@@
+
+* lock@j0@p(E, ...);
+... when != E;
+when != if@pif (...) S1 else S2
+when any
+x@j1 = f(...);
+* if (<+...x...+>)
+{
+  ... when != E;
+  when forall
+  when != if@pif (...) S3 else S4
+*  return@j2 ...;
+}
+... when any
+* unlock@j3(E, ...);
+
+// 
+
+@script:python balanced2_org depends on org@
+j0 << balanced2.j0;
+j1 << balanced2.j1;
+j2 << balanced2.j2;
+j3 << balanced2.j3;
+@@
+
+msg = "This code segment might have an unbalanced lock."
+coccilib.org.print_todo(j0[0], msg)
+coccilib.org.print_link(j1[0], "")
+coccilib.org.print_link(j2[0], "")
+coccilib.org.print_link(j3[0], "")
+
+// 
+
+@script:python balanced2_report depends on report@
+j0 << balanced2.j0;
+j1 << balanced2.j1;
+j2 << balanced2.j2;
+j3 << balanced2.j3;
+@@
+
+msg = "This code segment might have an unbalanced lock around lines %s,%s,%s." 
% (j1[0].line,j2[0].line,j3[0].line)
+coccilib.report.print_report(j0[0], msg)
+
-- 
2.25.1

___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/lis