diff mbox series

[RFC,7/8] block: Add no_coroutine_fn marker

Message ID 20220702113331.2003820-8-afaria@redhat.com (mailing list archive)
State New, archived
Headers show
Series Introduce an extensible static analyzer | expand

Commit Message

Alberto Faria July 2, 2022, 11:33 a.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.

Extend static-analyzer.py's "coroutine-annotation-validity" and
"coroutine-calls" checks to enforce 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.py           | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 1 deletion(-)
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 40a4037525..11eea8cc79 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.py b/static-analyzer.py
index 485d054052..ad7ee1ccf5 100755
--- a/static-analyzer.py
+++ b/static-analyzer.py
@@ -459,6 +459,12 @@  def check_coroutine_annotation_validity(
         ):
             log(node, "invalid coroutine_fn usage")
 
+        if is_annotation(node, "no_coroutine_fn") and (
+            ancestors[-1] is None
+            or not is_valid_no_coroutine_fn_usage(ancestors[-1])
+        ):
+            log(node, "invalid no_coroutine_fn usage")
+
         if is_comma_wrapper(
             node, "__allow_coroutine_fn_call"
         ) and not is_valid_allow_coroutine_fn_call_usage(node):
@@ -478,6 +484,9 @@  def log_annotation_disagreement(annotation: str) -> None:
             if is_coroutine_fn(node) != is_coroutine_fn(node.canonical):
                 log_annotation_disagreement("coroutine_fn")
 
+            if is_no_coroutine_fn(node) != is_no_coroutine_fn(node.canonical):
+                log_annotation_disagreement("no_coroutine_fn")
+
 
 @check("coroutine-calls")
 def check_coroutine_calls(
@@ -516,6 +525,11 @@  def check_coroutine_calls(
             ):
                 log(call, "non-coroutine_fn function calls coroutine_fn")
 
+            # reject calls from coroutine_fn to no_coroutine_fn
+
+            if caller_is_coroutine and is_no_coroutine_fn(callee):
+                log(call, f"coroutine_fn calls no_coroutine_fn function")
+
 
 @check("coroutine-pointers")
 def check_coroutine_pointers(
@@ -704,6 +718,16 @@  def parent_type_is_function_pointer() -> bool:
     return False
 
 
+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_valid_allow_coroutine_fn_call_usage(node: Cursor) -> bool:
     """
     Check if an occurrence of `__allow_coroutine_fn_call()` represented by node
@@ -771,6 +795,17 @@  def is_coroutine_fn(node: Cursor) -> bool:
     return False
 
 
+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")
+
+
 def is_annotated_with(node: Cursor, annotation: str) -> bool:
     return any(is_annotation(c, annotation) for c in node.get_children())