qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Alberto Faria <afaria@redhat.com>
To: qemu-devel@nongnu.org
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	qemu-block@nongnu.org, "Denis V. Lunev" <den@openvz.org>,
	Emanuele Giuseppe Esposito <eesposit@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Ronnie Sahlberg <ronniesahlberg@gmail.com>,
	Hanna Reitz <hreitz@redhat.com>,
	Stefano Garzarella <sgarzare@redhat.com>,
	Kevin Wolf <kwolf@redhat.com>, Peter Xu <peterx@redhat.com>,
	Alberto Garcia <berto@igalia.com>, John Snow <jsnow@redhat.com>,
	Eric Blake <eblake@redhat.com>, Fam Zheng <fam@euphon.net>,
	Markus Armbruster <armbru@redhat.com>,
	Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>,
	Peter Lieven <pl@kamp.de>, Alberto Faria <afaria@redhat.com>
Subject: [RFC 5/8] static-analyzer: Enforce coroutine_fn restrictions on function pointers
Date: Sat,  2 Jul 2022 12:33:28 +0100	[thread overview]
Message-ID: <20220702113331.2003820-6-afaria@redhat.com> (raw)
In-Reply-To: <20220702113331.2003820-1-afaria@redhat.com>

Extend static-analyzer.py to enforce coroutine_fn restrictions on
function pointer operations.

Invalid operations include assigning a coroutine_fn value to a
non-coroutine_fn function pointer, and invoking a coroutine_fn function
pointer from a non-coroutine_fn function.

Signed-off-by: Alberto Faria <afaria@redhat.com>
---
 static-analyzer.py | 147 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 143 insertions(+), 4 deletions(-)

diff --git a/static-analyzer.py b/static-analyzer.py
index 8abc552b82..485d054052 100755
--- a/static-analyzer.py
+++ b/static-analyzer.py
@@ -36,6 +36,8 @@
     TranslationUnit,
     TranslationUnitLoadError,
     TypeKind,
+    SourceRange,
+    TokenGroup,
 )
 
 Cursor.__hash__ = lambda self: self.hash  # so `Cursor`s can be dict keys
@@ -515,6 +517,90 @@ def check_coroutine_calls(
                 log(call, "non-coroutine_fn function calls coroutine_fn")
 
 
+@check("coroutine-pointers")
+def check_coroutine_pointers(
+    translation_unit: TranslationUnit,
+    translation_unit_path: str,
+    log: Callable[[Cursor, str], None],
+) -> None:
+
+    # What we would really like is to associate annotation attributes with types
+    # directly, but that doesn't seem possible. Instead, we have to look at the
+    # relevant variable/field/parameter declarations, and follow typedefs.
+
+    # This doesn't check all possible ways of assigning to a coroutine_fn
+    # field/variable/parameter. That would probably be too much work.
+
+    # TODO: Check struct/union/array initialization.
+    # TODO: Check assignment to struct/union/array fields.
+
+    for [*_, node] in all_paths(translation_unit.cursor):
+
+        # check initialization of variables using coroutine_fn values
+
+        if node.kind == CursorKind.VAR_DECL:
+
+            children = [
+                c
+                for c in node.get_children()
+                if c.kind
+                not in [
+                    CursorKind.ANNOTATE_ATTR,
+                    CursorKind.INIT_LIST_EXPR,
+                    CursorKind.TYPE_REF,
+                    CursorKind.UNEXPOSED_ATTR,
+                ]
+            ]
+
+            if (
+                len(children) == 1
+                and not is_coroutine_fn(node)
+                and is_coroutine_fn(children[0])
+            ):
+                log(node, "assigning coroutine_fn to non-coroutine_fn")
+
+        # check initialization of fields using coroutine_fn values
+
+        # TODO: This only checks designator initializers.
+
+        if node.kind == CursorKind.INIT_LIST_EXPR:
+
+            for initializer in filter(
+                lambda n: n.kind == CursorKind.UNEXPOSED_EXPR,
+                node.get_children(),
+            ):
+
+                children = list(initializer.get_children())
+
+                if (
+                    len(children) == 2
+                    and children[0].kind == CursorKind.MEMBER_REF
+                    and not is_coroutine_fn(children[0].referenced)
+                    and is_coroutine_fn(children[1])
+                ):
+                    log(
+                        initializer,
+                        "assigning coroutine_fn to non-coroutine_fn",
+                    )
+
+        # check assignments of coroutine_fn values to variables or fields
+
+        if is_binary_operator(node, "="):
+
+            [left, right] = node.get_children()
+
+            if (
+                left.kind
+                in [
+                    CursorKind.DECL_REF_EXPR,
+                    CursorKind.MEMBER_REF_EXPR,
+                ]
+                and not is_coroutine_fn(left.referenced)
+                and is_coroutine_fn(right)
+            ):
+                log(node, "assigning coroutine_fn to non-coroutine_fn")
+
+
 # ---------------------------------------------------------------------------- #
 # Traversal
 
@@ -555,6 +641,31 @@ def subtrees_matching(
 # Node predicates
 
 
+def is_binary_operator(node: Cursor, operator: str) -> bool:
+    return (
+        node.kind == CursorKind.BINARY_OPERATOR
+        and get_binary_operator_spelling(node) == operator
+    )
+
+
+def get_binary_operator_spelling(node: Cursor) -> Optional[str]:
+
+    assert node.kind == CursorKind.BINARY_OPERATOR
+
+    [left, right] = node.get_children()
+
+    op_range = SourceRange.from_locations(left.extent.end, right.extent.start)
+
+    tokens = list(TokenGroup.get_tokens(node.translation_unit, op_range))
+    if not tokens:
+        # Can occur when left and right children extents overlap due to
+        # misparsing.
+        return None
+
+    [op_token, *_] = tokens
+    return op_token.spelling
+
+
 def is_valid_coroutine_fn_usage(parent: Cursor) -> bool:
     """
     Check if an occurrence of `coroutine_fn` represented by a node with parent
@@ -599,7 +710,13 @@ def is_valid_allow_coroutine_fn_call_usage(node: Cursor) -> bool:
     `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.
+      - A function declared with the `coroutine_fn` annotation, OR
+      - A field/variable/parameter whose declaration has the `coroutine_fn`
+        annotation, and of function pointer type, OR
+      - [TODO] A field/variable/parameter of a typedef function pointer type,
+        and the typedef has the `coroutine_fn` annotation, OR
+      - [TODO] A field/variable/parameter of a pointer to typedef function type,
+        and the typedef has the `coroutine_fn` annotation.
 
     TODO: Ensure that `__allow_coroutine_fn_call()` is in the body of a
     non-`coroutine_fn` function.
@@ -627,9 +744,31 @@ def is_coroutine_fn(node: Cursor) -> bool:
         else:
             break
 
-    return node.kind == CursorKind.FUNCTION_DECL and is_annotated_with(
-        node, "coroutine_fn"
-    )
+    if node.kind in [CursorKind.DECL_REF_EXPR, CursorKind.MEMBER_REF_EXPR]:
+        node = node.referenced
+
+    # ---
+
+    if node.kind == CursorKind.FUNCTION_DECL:
+        return is_annotated_with(node, "coroutine_fn")
+
+    if node.kind in [
+        CursorKind.FIELD_DECL,
+        CursorKind.VAR_DECL,
+        CursorKind.PARM_DECL,
+    ]:
+
+        if is_annotated_with(node, "coroutine_fn"):
+            return True
+
+        # TODO: If type is typedef or pointer to typedef, follow typedef.
+
+        return False
+
+    if node.kind == CursorKind.TYPEDEF_DECL:
+        return is_annotated_with(node, "coroutine_fn")
+
+    return False
 
 
 def is_annotated_with(node: Cursor, annotation: str) -> bool:
-- 
2.36.1



  parent reply	other threads:[~2022-07-02 11:42 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-02 11:33 [RFC 0/8] Introduce an extensible static analyzer Alberto Faria
2022-07-02 11:33 ` [RFC 1/8] Add " Alberto Faria
2022-07-02 11:33 ` [RFC 2/8] Drop some unused static function return values Alberto Faria
2022-07-02 11:33 ` [RFC 3/8] static-analyzer: Enforce coroutine_fn restrictions for direct calls Alberto Faria
2022-07-02 11:33 ` [RFC 4/8] Fix some direct calls from non-coroutine_fn to coroutine_fn Alberto Faria
2022-07-02 14:13   ` Paolo Bonzini
2022-07-03 22:20     ` Alberto Faria
2022-07-02 11:33 ` Alberto Faria [this message]
2022-07-04 14:16   ` [RFC 5/8] static-analyzer: Enforce coroutine_fn restrictions on function pointers Víctor Colombo
2022-07-04 16:57     ` Alberto Faria
2022-07-04 17:46       ` Víctor Colombo
2022-07-04 18:04         ` Alberto Faria
2022-07-04 19:06           ` Víctor Colombo
2022-07-02 11:33 ` [RFC 6/8] Fix some coroutine_fn indirect calls and pointer assignments Alberto Faria
2022-07-02 11:33 ` [RFC 7/8] block: Add no_coroutine_fn marker Alberto Faria
2022-07-02 11:33 ` [RFC 8/8] Avoid calls from coroutine_fn to no_coroutine_fn Alberto Faria
2022-07-02 14:17 ` [RFC 0/8] Introduce an extensible static analyzer Paolo Bonzini
2022-07-04 16:28 ` Daniel P. Berrangé
2022-07-04 19:30   ` Alberto Faria
2022-07-05  7:16     ` Daniel P. Berrangé
2022-07-05 11:28       ` Alberto Faria
2022-07-05 16:12         ` Daniel P. Berrangé
2022-07-06  9:54           ` Alberto Faria
2022-07-06 10:15             ` Daniel P. Berrangé
2022-07-08 17:18               ` Alberto Faria
2022-07-05 16:13 ` Daniel P. Berrangé
2022-07-06  9:56   ` Alberto Faria

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220702113331.2003820-6-afaria@redhat.com \
    --to=afaria@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berto@igalia.com \
    --cc=den@openvz.org \
    --cc=eblake@redhat.com \
    --cc=eesposit@redhat.com \
    --cc=fam@euphon.net \
    --cc=hreitz@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    --cc=pl@kamp.de \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=ronniesahlberg@gmail.com \
    --cc=sgarzare@redhat.com \
    --cc=stefanha@redhat.com \
    --cc=vsementsov@yandex-team.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).