diff mbox series

[RFC,v2,09/10] block: Add no_coroutine_fn marker

Message ID 20220729130040.1428779-10-afaria@redhat.com
State New
Headers show
Series Introduce an extensible static analyzer | expand

Commit Message

Alberto Faria July 29, 2022, 1 p.m. UTC
When applied to a function, it advertises that it should not be called
from coroutine_fn functions.

Make generated_co_wrapper evaluate to no_coroutine_fn, as coroutine_fn
functions should instead directly call the coroutine_fn that backs the
generated_co_wrapper.

Add a "no_coroutine_fn" check to static-analyzer.py that enforces
no_coroutine_fn rules.

Signed-off-by: Alberto Faria <afaria@redhat.com>
---
 include/block/block-common.h       |   2 +-
 include/qemu/coroutine.h           |  12 ++++
 static_analyzer/no_coroutine_fn.py | 111 +++++++++++++++++++++++++++++
 3 files changed, 124 insertions(+), 1 deletion(-)
 create mode 100644 static_analyzer/no_coroutine_fn.py
diff mbox series

Patch

diff --git a/include/block/block-common.h b/include/block/block-common.h
index fdb7306e78..4b60c8c6f2 100644
--- a/include/block/block-common.h
+++ b/include/block/block-common.h
@@ -42,7 +42,7 @@ 
  *
  * Read more in docs/devel/block-coroutine-wrapper.rst
  */
-#define generated_co_wrapper
+#define generated_co_wrapper no_coroutine_fn
 
 /* block.c */
 typedef struct BlockDriver BlockDriver;
diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index 26445b3176..c61dd2d3f7 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -48,6 +48,18 @@ 
 #define coroutine_fn
 #endif
 
+/**
+ * Mark a function that should never be called from a coroutine context
+ *
+ * This typically means that there is an analogous, coroutine_fn function that
+ * should be used instead.
+ */
+#ifdef __clang__
+#define no_coroutine_fn __attribute__((__annotate__("no_coroutine_fn")))
+#else
+#define no_coroutine_fn
+#endif
+
 /**
  * This can wrap a call to a coroutine_fn from a non-coroutine_fn function and
  * suppress the static analyzer's complaints.
diff --git a/static_analyzer/no_coroutine_fn.py b/static_analyzer/no_coroutine_fn.py
new file mode 100644
index 0000000000..1d0b93bb62
--- /dev/null
+++ b/static_analyzer/no_coroutine_fn.py
@@ -0,0 +1,111 @@ 
+# ---------------------------------------------------------------------------- #
+
+from clang.cindex import Cursor, CursorKind  # type: ignore
+
+from static_analyzer import (
+    CheckContext,
+    VisitorResult,
+    check,
+    is_annotation,
+    is_annotated_with,
+    visit,
+)
+from static_analyzer.coroutine_fn import is_coroutine_fn
+
+# ---------------------------------------------------------------------------- #
+
+
+@check("no_coroutine_fn")
+def check_no_coroutine_fn(context: CheckContext) -> None:
+    """Reports violations of no_coroutine_fn rules."""
+
+    def visitor(node: Cursor) -> VisitorResult:
+
+        validate_annotations(context, node)
+
+        if node.kind == CursorKind.FUNCTION_DECL and node.is_definition():
+            check_calls(context, node)
+            return VisitorResult.CONTINUE
+
+        return VisitorResult.RECURSE
+
+    visit(context.translation_unit.cursor, visitor)
+
+
+def validate_annotations(context: CheckContext, node: Cursor) -> None:
+
+    # validate annotation usage
+
+    if is_annotation(node, "no_coroutine_fn") and (
+        node.parent is None or not is_valid_no_coroutine_fn_usage(node.parent)
+    ):
+        context.report(node, "invalid no_coroutine_fn usage")
+
+    # reject re-declarations with inconsistent annotations
+
+    if node.kind == CursorKind.FUNCTION_DECL and is_no_coroutine_fn(
+        node
+    ) != is_no_coroutine_fn(node.canonical):
+
+        context.report(
+            node,
+            f"no_coroutine_fn annotation disagreement with"
+            f" {context.format_location(node.canonical)}",
+        )
+
+
+def check_calls(context: CheckContext, caller: Cursor) -> None:
+    """
+    Reject calls from coroutine_fn to no_coroutine_fn.
+
+    Assumes that `caller` is a function definition.
+    """
+
+    if is_coroutine_fn(caller):
+
+        def visitor(node: Cursor) -> VisitorResult:
+
+            # We can get some "calls" that are actually things like top-level
+            # macro invocations for which `node.referenced` is None.
+
+            if (
+                node.kind == CursorKind.CALL_EXPR
+                and node.referenced is not None
+                and is_no_coroutine_fn(node.referenced.canonical)
+            ):
+                context.report(
+                    node,
+                    f"coroutine_fn calls no_coroutine_fn function"
+                    f" {node.referenced.spelling}()",
+                )
+
+            return VisitorResult.RECURSE
+
+        visit(caller, visitor)
+
+
+# ---------------------------------------------------------------------------- #
+
+
+def is_valid_no_coroutine_fn_usage(parent: Cursor) -> bool:
+    """
+    Checks if an occurrence of `no_coroutine_fn` represented by a node with
+    parent `parent` appears at a valid point in the AST. This is the case if the
+    parent is a function declaration/definition.
+    """
+
+    return parent.kind == CursorKind.FUNCTION_DECL
+
+
+def is_no_coroutine_fn(node: Cursor) -> bool:
+    """
+    Checks whether the given `node` should be considered to be
+    `no_coroutine_fn`.
+
+    This assumes valid usage of `no_coroutine_fn`.
+    """
+
+    return is_annotated_with(node, "no_coroutine_fn")
+
+
+# ---------------------------------------------------------------------------- #