[tor-commits] [tor/master] Add a coccinelle script to look for {inc, dec}rements in log_debug

[asn at torproject.org]

commit 387cfccee47394adeba8cbf49c130cc9b332b025
Author: Nick Mathewson <nickm at torproject.org>
Date:   Wed Sep 11 18:53:16 2019 -0400

    Add a coccinelle script to look for {inc,dec}rements in log_debug
    
    We want to forbid this pattern since, unlike the other log_*()
    macros, log_debug() conditionally evaluates its arguments only if
    debug-level logging is enabled.  Thus, a call to
        log_debug("%d", x++);
    will only increment x if debugging logs are enabled, which is
    probably not what the programmer intended.
    
    One bug caused by this pattern was #30628.
    
    This script detects log_debug( ) calls with any of E++, E--, ++E,
    or --E in their arguments, where E is an arbitrary expression.
    
    Closes ticket 30743.
---
 changes/ticket30743              |  7 +++++++
 scripts/coccinelle/debugmm.cocci | 29 +++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/changes/ticket30743 b/changes/ticket30743
new file mode 100644
index 000000000..4f029717d
--- /dev/null
+++ b/changes/ticket30743
@@ -0,0 +1,7 @@
+  o Minor features (maintenance scripts):
+    - Add a coccinelle script to detect bugs caused by incrementing or
+      decrementing a variable inside a call to log_debug().  Since
+      log_debug() is a macro whose arguments are conditionally evaluated, it
+      is usually an error to do this. One such bug was 30628, in which SENDME
+      cells were miscounted by a decrement operator inside a log_debug()
+      call.  Closes ticket 30743.
diff --git a/scripts/coccinelle/debugmm.cocci b/scripts/coccinelle/debugmm.cocci
new file mode 100644
index 000000000..dbd308df3
--- /dev/null
+++ b/scripts/coccinelle/debugmm.cocci
@@ -0,0 +1,29 @@
+// Look for use of expressions with side-effects inside of debug logs.
+//
+// This script detects expressions like ++E, --E, E++, and E-- inside of
+// calls to log_debug().
+//
+// The log_debug() macro exits early if debug logging is not enabled,
+// potentially causing problems if its arguments have side-effects.
+
+@@
+expression E;
+@@
+*log_debug(... , <+...  --E ...+>, ... );
+
+
+@@
+expression E;
+@@
+*log_debug(... , <+...  ++E ...+>, ... );
+
+@@
+expression E;
+@@
+*log_debug(... , <+...  E-- ...+>, ... );
+
+
+@@
+expression E;
+@@
+*log_debug(... , <+...  E++ ...+>, ... );