[RFC v2 05/10] static-analyzer: Enforce coroutine_fn restrictions for direct calls

[Alberto Faria]

Add a "coroutine_fn" check to static-analyzer.py that ensures that
non-coroutine_fn functions don't perform direct calls to coroutine_fn
functions.

For the few cases where this must happen, introduce an
__allow_coroutine_fn_call() macro that wraps offending calls and
overrides the static analyzer.

Signed-off-by: Alberto Faria <afaria@redhat.com>
---
 include/qemu/coroutine.h        |  13 +++
 static_analyzer/__init__.py     |  46 ++++++++-
 static_analyzer/coroutine_fn.py | 173 ++++++++++++++++++++++++++++++++
 3 files changed, 231 insertions(+), 1 deletion(-)
 create mode 100644 static_analyzer/coroutine_fn.py

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index 08c5bb3c76..40a4037525 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -42,7 +42,20 @@
  *       ....
  *   }
  */
+#ifdef __clang__
+#define coroutine_fn __attribute__((__annotate__("coroutine_fn")))
+#else
 #define 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.
+ *
+ * You don't want to use this.
+ */
+#define __allow_coroutine_fn_call(call) \
+    ((void)"__allow_coroutine_fn_call", call)
 
 typedef struct Coroutine Coroutine;
 
diff --git a/static_analyzer/__init__.py b/static_analyzer/__init__.py
index 36028724b1..5abdbd21a3 100644
--- a/static_analyzer/__init__.py
+++ b/static_analyzer/__init__.py
@@ -23,8 +23,9 @@
 from clang.cindex import (  # type: ignore
     Cursor,
     CursorKind,
-    TranslationUnit,
     SourceLocation,
+    TranslationUnit,
+    TypeKind,
     conf,
 )
 
@@ -146,6 +147,49 @@ def matcher(n: Cursor) -> bool:
     return any(map(matcher, node.get_children()))
 
 
+def is_annotated_with(node: Cursor, annotation: str) -> bool:
+    return any(is_annotation(c, annotation) for c in node.get_children())
+
+
+def is_annotation(node: Cursor, annotation: str) -> bool:
+    return node.kind == CursorKind.ANNOTATE_ATTR and node.spelling == 
annotation
+
+
+def is_comma_wrapper(node: Cursor, literal: str) -> bool:
+    """
+    Check if `node` is a "comma-wrapper" with the given string literal.
+
+    A "comma-wrapper" is the pattern `((void)string_literal, expr)`. The `expr`
+    is said to be "comma-wrapped".
+    """
+
+    # TODO: Do we need to check that the operator is `,`? Is there another
+    # operator that can combine void and an expr?
+
+    if node.kind != CursorKind.BINARY_OPERATOR:
+        return False
+
+    [left, _right] = node.get_children()
+
+    if (
+        left.kind != CursorKind.CSTYLE_CAST_EXPR
+        or left.type.kind != TypeKind.VOID
+    ):
+        return False
+
+    [unexposed_expr] = left.get_children()
+
+    if unexposed_expr.kind != CursorKind.UNEXPOSED_EXPR:
+        return False
+
+    [string_literal] = unexposed_expr.get_children()
+
+    return (
+        string_literal.kind == CursorKind.STRING_LITERAL
+        and string_literal.spelling == f'"{literal}"'
+    )
+
+
 # ---------------------------------------------------------------------------- 
#
 # Checks
 
diff --git a/static_analyzer/coroutine_fn.py b/static_analyzer/coroutine_fn.py
new file mode 100644
index 0000000000..f70a3167eb
--- /dev/null
+++ b/static_analyzer/coroutine_fn.py
@@ -0,0 +1,173 @@
+# ---------------------------------------------------------------------------- 
#
+
+from clang.cindex import Cursor, CursorKind, TypeKind  # type: ignore
+
+from static_analyzer import (
+    CheckContext,
+    VisitorResult,
+    check,
+    is_annotated_with,
+    is_annotation,
+    is_comma_wrapper,
+    visit,
+)
+
+# ---------------------------------------------------------------------------- 
#
+
+
+@check("coroutine_fn")
+def check_coroutine_fn(context: CheckContext) -> None:
+    """Reports violations of coroutine_fn rules."""
+
+    def visitor(node: Cursor) -> VisitorResult:
+
+        validate_annotations(context, node)
+
+        if node.kind == CursorKind.FUNCTION_DECL and node.is_definition():
+            check_direct_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, "coroutine_fn") and (
+        node.parent is None or not is_valid_coroutine_fn_usage(node.parent)
+    ):
+        context.report(node, "invalid coroutine_fn usage")
+
+    if is_comma_wrapper(
+        node, "__allow_coroutine_fn_call"
+    ) and not is_valid_allow_coroutine_fn_call_usage(node):
+        context.report(node, "invalid __allow_coroutine_fn_call usage")
+
+    # reject re-declarations with inconsistent annotations
+
+    if node.kind == CursorKind.FUNCTION_DECL and is_coroutine_fn(
+        node
+    ) != is_coroutine_fn(node.canonical):
+        context.report(
+            node,
+            f"coroutine_fn annotation disagreement with"
+            f" {context.format_location(node.canonical)}",
+        )
+
+
+def check_direct_calls(context: CheckContext, caller: Cursor) -> None:
+    """
+    Reject calls from non-coroutine_fn to coroutine_fn.
+
+    Assumes that `caller` is a function definition.
+    """
+
+    if not is_coroutine_fn(caller):
+
+        def visitor(node: Cursor) -> VisitorResult:
+
+            # We can get "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_coroutine_fn(node.referenced.canonical)
+                and not is_comma_wrapper(
+                    node.parent, "__allow_coroutine_fn_call"
+                )
+            ):
+                context.report(
+                    node,
+                    f"non-coroutine_fn function calls coroutine_fn"
+                    f" {node.referenced.spelling}()",
+                )
+
+            return VisitorResult.RECURSE
+
+        visit(caller, visitor)
+
+
+# ---------------------------------------------------------------------------- 
#
+
+
+def is_valid_coroutine_fn_usage(parent: Cursor) -> bool:
+    """
+    Check if an occurrence of `coroutine_fn` represented by a node with parent
+    `parent` appears at a valid point in the AST. This is the case if `parent`
+    is:
+
+      - A function declaration/definition, OR
+      - A field/variable/parameter declaration with a function pointer type, OR
+      - A typedef of a function type or function pointer type.
+    """
+
+    if parent.kind == CursorKind.FUNCTION_DECL:
+        return True
+
+    canonical_type = parent.type.get_canonical()
+
+    def parent_type_is_function() -> bool:
+        return canonical_type.kind == TypeKind.FUNCTIONPROTO
+
+    def parent_type_is_function_pointer() -> bool:
+        return (
+            canonical_type.kind == TypeKind.POINTER
+            and canonical_type.get_pointee().kind == TypeKind.FUNCTIONPROTO
+        )
+
+    if parent.kind in [
+        CursorKind.FIELD_DECL,
+        CursorKind.VAR_DECL,
+        CursorKind.PARM_DECL,
+    ]:
+        return parent_type_is_function_pointer()
+
+    if parent.kind == CursorKind.TYPEDEF_DECL:
+        return parent_type_is_function() or parent_type_is_function_pointer()
+
+    return False
+
+
+def is_valid_allow_coroutine_fn_call_usage(node: Cursor) -> bool:
+    """
+    Check if an occurrence of `__allow_coroutine_fn_call()` represented by node
+    `node` appears at a valid point in the AST. This is the case if its right
+    operand is a call to:
+
+      - A function declared with the `coroutine_fn` annotation.
+
+    TODO: Ensure that `__allow_coroutine_fn_call()` is in the body of a
+    non-`coroutine_fn` function.
+    """
+
+    [_, call] = node.get_children()
+
+    return call.kind == CursorKind.CALL_EXPR and is_coroutine_fn(
+        call.referenced
+    )
+
+
+def is_coroutine_fn(node: Cursor) -> bool:
+    """
+    Check whether the given `node` should be considered to be `coroutine_fn`.
+
+    This assumes valid usage of `coroutine_fn`.
+    """
+
+    while node.kind in [CursorKind.PAREN_EXPR, CursorKind.UNEXPOSED_EXPR]:
+        children = list(node.get_children())
+        if len(children) == 1:
+            node = children[0]
+        else:
+            break
+
+    return node.kind == CursorKind.FUNCTION_DECL and is_annotated_with(
+        node, "coroutine_fn"
+    )
+
+
+# ---------------------------------------------------------------------------- 
#
-- 
2.37.1