public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Alberto Faria <afaria@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Stefano Garzarella" <sgarzare@redhat.com>,
	"Hannes Reinecke" <hare@suse.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	"Vladimir Sementsov-Ogievskiy" <vsementsov@yandex-team.ru>,
	"Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>,
	"Peter Lieven" <pl@kamp.de>,
	kvm@vger.kernel.org, "Xie Yongji" <xieyongji@bytedance.com>,
	"Eric Auger" <eric.auger@redhat.com>,
	"Hanna Reitz" <hreitz@redhat.com>,
	"Jeff Cody" <codyprime@gmail.com>,
	"Eric Blake" <eblake@redhat.com>,
	"Denis V. Lunev" <den@openvz.org>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Christian Schoenebeck" <qemu_oss@crudebyte.com>,
	"Stefan Weil" <sw@weilnetz.de>,
	"Klaus Jensen" <its@irrelevant.dk>,
	"Laurent Vivier" <lvivier@redhat.com>,
	"Alberto Garcia" <berto@igalia.com>,
	"Michael Roth" <michael.roth@amd.com>,
	"Juan Quintela" <quintela@redhat.com>,
	"David Hildenbrand" <david@redhat.com>,
	qemu-block@nongnu.org, "Konstantin Kostiuk" <kkostiuk@redhat.com>,
	"Kevin Wolf" <kwolf@redhat.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Marcelo Tosatti" <mtosatti@redhat.com>,
	"Greg Kurz" <groug@kaod.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Amit Shah" <amit@kernel.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Alex Williamson" <alex.williamson@redhat.com>,
	"Peter Xu" <peterx@redhat.com>,
	"Raphael Norwitz" <raphael.norwitz@nutanix.com>,
	"Ronnie Sahlberg" <ronniesahlberg@gmail.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Emanuele Giuseppe Esposito" <eesposit@redhat.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Marcel Apfelbaum" <marcel.apfelbaum@gmail.com>,
	"Dmitry Fleytman" <dmitry.fleytman@gmail.com>,
	"Eduardo Habkost" <eduardo@habkost.net>,
	"Fam Zheng" <fam@euphon.net>, "Thomas Huth" <thuth@redhat.com>,
	"Keith Busch" <kbusch@kernel.org>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Richard W.M. Jones" <rjones@redhat.com>,
	"John Snow" <jsnow@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Alberto Faria" <afaria@redhat.com>
Subject: [RFC v2 07/10] static-analyzer: Enforce coroutine_fn restrictions on function pointers
Date: Fri, 29 Jul 2022 14:00:36 +0100	[thread overview]
Message-ID: <20220729130040.1428779-8-afaria@redhat.com> (raw)
In-Reply-To: <20220729130040.1428779-1-afaria@redhat.com>

Extend static-analyzer.py's "coroutine_fn" check 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/__init__.py     |  27 ++++++++
 static_analyzer/coroutine_fn.py | 115 ++++++++++++++++++++++++++++++--
 2 files changed, 138 insertions(+), 4 deletions(-)

diff --git a/static_analyzer/__init__.py b/static_analyzer/__init__.py
index 5abdbd21a3..90992d3500 100644
--- a/static_analyzer/__init__.py
+++ b/static_analyzer/__init__.py
@@ -24,6 +24,8 @@
     Cursor,
     CursorKind,
     SourceLocation,
+    SourceRange,
+    TokenGroup,
     TranslationUnit,
     TypeKind,
     conf,
@@ -117,6 +119,31 @@ def actual_visitor(node: Cursor, parent: Cursor, client_data: None) -> int:
 # 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 might_have_attribute(node: Cursor, attr: Union[CursorKind, str]) -> bool:
     """
     Check whether any of `node`'s children are an attribute of the given kind,
diff --git a/static_analyzer/coroutine_fn.py b/static_analyzer/coroutine_fn.py
index f70a3167eb..a16dcbeb52 100644
--- a/static_analyzer/coroutine_fn.py
+++ b/static_analyzer/coroutine_fn.py
@@ -8,6 +8,7 @@
     check,
     is_annotated_with,
     is_annotation,
+    is_binary_operator,
     is_comma_wrapper,
     visit,
 )
@@ -22,6 +23,7 @@ def check_coroutine_fn(context: CheckContext) -> None:
     def visitor(node: Cursor) -> VisitorResult:
 
         validate_annotations(context, node)
+        check_function_pointers(context, node)
 
         if node.kind == CursorKind.FUNCTION_DECL and node.is_definition():
             check_direct_calls(context, node)
@@ -91,6 +93,83 @@ def visitor(node: Cursor) -> VisitorResult:
         visit(caller, visitor)
 
 
+def check_function_pointers(context: CheckContext, node: Cursor) -> 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.
+
+    # 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])
+        ):
+            context.report(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])
+            ):
+                context.report(
+                    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)
+        ):
+            context.report(node, "assigning coroutine_fn to non-coroutine_fn")
+
+
 # ---------------------------------------------------------------------------- #
 
 
@@ -138,7 +217,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.
@@ -165,9 +250,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
 
 
 # ---------------------------------------------------------------------------- #
-- 
2.37.1


  parent reply	other threads:[~2022-07-29 13:02 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-29 13:00 [RFC v2 00/10] Introduce an extensible static analyzer Alberto Faria
2022-07-29 13:00 ` [RFC v2 01/10] Add " Alberto Faria
2022-07-29 13:00 ` [RFC v2 02/10] Drop unused static function return values Alberto Faria
2022-08-03 10:46   ` Dr. David Alan Gilbert
2022-08-03 11:07     ` Alberto Faria
2022-08-03 11:15       ` Richard W.M. Jones
2022-08-03 11:37         ` Daniel P. Berrangé
2022-08-03 12:25           ` Peter Maydell
2022-08-03 12:47             ` Richard W.M. Jones
2022-08-12 16:29         ` Alberto Faria
2022-08-03 11:12     ` Daniel P. Berrangé
2022-08-03 12:30   ` Peter Maydell
2022-08-12 16:01     ` Alberto Faria
2022-07-29 13:00 ` [RFC v2 03/10] static-analyzer: Support adding tests to checks Alberto Faria
2022-07-29 13:00 ` [RFC v2 04/10] static-analyzer: Avoid reanalyzing unmodified translation units Alberto Faria
2022-07-29 13:00 ` [RFC v2 05/10] static-analyzer: Enforce coroutine_fn restrictions for direct calls Alberto Faria
2022-07-29 13:00 ` [RFC v2 06/10] Fix some direct calls from non-coroutine_fn to coroutine_fn Alberto Faria
2022-07-29 13:00 ` Alberto Faria [this message]
2022-07-29 13:00 ` [RFC v2 08/10] Fix some bad coroutine_fn indirect calls and pointer assignments Alberto Faria
2022-07-29 13:00 ` [RFC v2 09/10] block: Add no_coroutine_fn marker Alberto Faria
2022-07-29 13:00 ` [RFC v2 10/10] Fix some calls from coroutine_fn to no_coroutine_fn Alberto Faria
2022-08-04 11:44 ` [RFC v2 00/10] Introduce an extensible static analyzer Marc-André Lureau
2022-08-12 15:48   ` Alberto Faria
2022-08-16  7:17     ` Marc-André Lureau
2022-10-13 22:00 ` Paolo Bonzini
2022-10-15 13:14   ` Christian Schoenebeck

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=20220729130040.1428779-8-afaria@redhat.com \
    --to=afaria@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=alex.williamson@redhat.com \
    --cc=amit@kernel.org \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=berto@igalia.com \
    --cc=codyprime@gmail.com \
    --cc=david@redhat.com \
    --cc=den@openvz.org \
    --cc=dgilbert@redhat.com \
    --cc=dmitry.fleytman@gmail.com \
    --cc=eblake@redhat.com \
    --cc=eduardo@habkost.net \
    --cc=eesposit@redhat.com \
    --cc=eric.auger@redhat.com \
    --cc=f4bug@amsat.org \
    --cc=fam@euphon.net \
    --cc=groug@kaod.org \
    --cc=hare@suse.com \
    --cc=hreitz@redhat.com \
    --cc=its@irrelevant.dk \
    --cc=jasowang@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kbusch@kernel.org \
    --cc=kkostiuk@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=kwolf@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=maciej.szmigiero@oracle.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=michael.roth@amd.com \
    --cc=mst@redhat.com \
    --cc=mtosatti@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=qemu_oss@crudebyte.com \
    --cc=quintela@redhat.com \
    --cc=raphael.norwitz@nutanix.com \
    --cc=richard.henderson@linaro.org \
    --cc=rjones@redhat.com \
    --cc=ronniesahlberg@gmail.com \
    --cc=sgarzare@redhat.com \
    --cc=stefanha@redhat.com \
    --cc=sw@weilnetz.de \
    --cc=thuth@redhat.com \
    --cc=vsementsov@yandex-team.ru \
    --cc=xieyongji@bytedance.com \
    /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