[RFC 3/8] static-analyzer: Enforce coroutine_fn restrictions for direct calls

[Alberto Faria]

Add a static-analyzer.py check ensuring 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.py       | 207 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 220 insertions(+)

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.py b/static-analyzer.py
index 010cc92212..8abc552b82 100755
--- a/static-analyzer.py
+++ b/static-analyzer.py
@@ -440,6 +440,81 @@ def function_occurrence_might_use_return_value(
             log(cursor, f"{cursor.spelling}() return value is never used")
 
 
+@check("coroutine-annotation-validity")
+def check_coroutine_annotation_validity(
+    translation_unit: TranslationUnit,
+    translation_unit_path: str,
+    log: Callable[[Cursor, str], None],
+) -> None:
+
+    for [*ancestors, node] in all_paths(translation_unit.cursor):
+
+        # validate annotation usage
+
+        if is_annotation(node, "coroutine_fn") and (
+            ancestors[-1] is None
+            or not is_valid_coroutine_fn_usage(ancestors[-1])
+        ):
+            log(node, "invalid coroutine_fn usage")
+
+        if is_comma_wrapper(
+            node, "__allow_coroutine_fn_call"
+        ) and not is_valid_allow_coroutine_fn_call_usage(node):
+            log(node, "invalid __allow_coroutine_fn_call usage")
+
+        # reject re-declarations with inconsistent annotations
+
+        if node.kind == CursorKind.FUNCTION_DECL:
+
+            def log_annotation_disagreement(annotation: str) -> None:
+                log(
+                    node,
+                    f"{annotation} annotation disagreement with"
+                    f" {format_location(node.canonical)}",
+                )
+
+            if is_coroutine_fn(node) != is_coroutine_fn(node.canonical):
+                log_annotation_disagreement("coroutine_fn")
+
+
+@check("coroutine-calls")
+def check_coroutine_calls(
+    translation_unit: TranslationUnit,
+    translation_unit_path: str,
+    log: Callable[[Cursor, str], None],
+) -> None:
+
+    for caller in subtrees_matching(
+        translation_unit.cursor,
+        lambda n: n.kind == CursorKind.FUNCTION_DECL and n.is_definition(),
+    ):
+
+        caller_is_coroutine = is_coroutine_fn(caller)
+
+        for [*_, call_parent, call] in filter(
+            lambda path: path[-1].kind == CursorKind.CALL_EXPR,
+            all_paths(caller),
+        ):
+
+            # We can get some "calls" that are actually things like top-level
+            # macro invocations.
+            if call.referenced is None:
+                continue
+
+            callee = call.referenced.canonical
+
+            # reject calls from non-coroutine_fn to coroutine_fn
+
+            if (
+                not caller_is_coroutine
+                and is_coroutine_fn(callee)
+                and not is_comma_wrapper(
+                    call_parent, "__allow_coroutine_fn_call"
+                )
+            ):
+                log(call, "non-coroutine_fn function calls coroutine_fn")
+
+
 # ---------------------------------------------------------------------------- 
#
 # Traversal
 
@@ -464,6 +539,138 @@ def aux(node: Cursor) -> Iterable[Sequence[Cursor]]:
     return aux(root)
 
 
+# Doesn't traverse yielded subtrees.
+def subtrees_matching(
+    root: Cursor, predicate: Callable[[Cursor], bool]
+) -> Iterable[Cursor]:
+
+    if predicate(root):
+        yield root
+    else:
+        for child in root.get_children():
+            yield from subtrees_matching(child, predicate)
+
+
+# ---------------------------------------------------------------------------- 
#
+# Node predicates
+
+
+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()
+
+    if call.kind != CursorKind.CALL_EXPR:
+        return False
+
+    return 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"
+    )
+
+
+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
+
+
+# A "comma wrapper" is the pattern `((void)string_literal, expr)`. The `expr` 
is
+# said to be "comma wrapped".
+def is_comma_wrapper(node: Cursor, literal: str) -> bool:
+
+    # 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}"'
+    )
+
+
 # ---------------------------------------------------------------------------- 
#
 # Utilities handy for development
 
-- 
2.36.1