public inbox for gdb-cvs@sourceware.org
help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@sourceware.org>
To: gdb-cvs@sourceware.org
Subject: [binutils-gdb] gdb: add asserts to gdbarch_register_name
Date: Sun,  2 Oct 2022 16:28:01 +0000 (GMT)	[thread overview]
Message-ID: <20221002162801.2A9403858D38@sourceware.org> (raw)

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=7df4240040277fb752fd010f5a947cf2d32bc3a5

commit 7df4240040277fb752fd010f5a947cf2d32bc3a5
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Sep 1 14:01:11 2022 +0100

    gdb: add asserts to gdbarch_register_name
    
    This commit adds asserts to gdbarch_register_name that validate the
    parameters, and the return value.
    
    The interesting thing here is that gdbarch_register_name is generated
    by gdbarch.py, and so, to add these asserts, I need to update the
    generation script.
    
    I've added two new arguments for Functions and Methods (as declared in
    gdbarch-components.py), these arguments are 'param_checks' and
    'result_checks'.  Each of these new arguments can be used to list some
    expressions that are then used within gdb_assert calls in the
    generated code.
    
    The asserts that validate the API as described in the comment I added
    to gdbarch_register_name a few commits back; the register number
    passed in needs to be a valid cooked register number, and the result
    being returned should not be nullptr.

Diff:
---
 gdb/gdbarch-components.py | 16 ++++++++++++++++
 gdb/gdbarch.c             |  6 +++++-
 gdb/gdbarch.py            | 21 ++++++++++++++++++++-
 3 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py
index 9e0a3e99417..b8d7648d5ec 100644
--- a/gdb/gdbarch-components.py
+++ b/gdb/gdbarch-components.py
@@ -99,6 +99,20 @@
 # argument, and NAME is the name.  Note that while the names could be
 # auto-generated, this approach lets the "comment" field refer to
 # arguments in a nicer way.  It is also just nicer for users.
+#
+# * "param_checks" - optional, a list of strings.  Each string is an
+# expression that is placed within a gdb_assert before the call is
+# made to the Function/Method implementation.  Each expression is
+# something that should be true, and it is expected that the
+# expression will make use of the parameters named in 'params' (though
+# this is not required).
+#
+# * "result_checks" - optional, a list of strings.  Each string is an
+# expression that is placed within a gdb_assert after the call to the
+# Function/Method implementation.  Within each expression the variable
+# 'result' can be used to reference the result of the function/method
+# implementation.  The 'result_checks' can only be used if the 'type'
+# of this Function/Method is not 'void'.
 
 Info(
     type="const struct bfd_arch_info *",
@@ -568,6 +582,8 @@ should never return nullptr.
     type="const char *",
     name="register_name",
     params=[("int", "regnr")],
+    param_checks=["regnr >= 0", "regnr < gdbarch_num_cooked_regs (gdbarch)"],
+    result_checks=["result != nullptr"],
     predefault="0",
     invalid=True,
 )
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index be73d399069..4b0c10be402 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -2235,9 +2235,13 @@ gdbarch_register_name (struct gdbarch *gdbarch, int regnr)
 {
   gdb_assert (gdbarch != NULL);
   gdb_assert (gdbarch->register_name != NULL);
+  gdb_assert (regnr >= 0);
+  gdb_assert (regnr < gdbarch_num_cooked_regs (gdbarch));
   if (gdbarch_debug >= 2)
     gdb_printf (gdb_stdlog, "gdbarch_register_name called\n");
-  return gdbarch->register_name (gdbarch, regnr);
+  auto result = gdbarch->register_name (gdbarch, regnr);
+  gdb_assert (result != nullptr);
+  return result;
 }
 
 void
diff --git a/gdb/gdbarch.py b/gdb/gdbarch.py
index 696b3028e6f..da848feae84 100755
--- a/gdb/gdbarch.py
+++ b/gdb/gdbarch.py
@@ -49,6 +49,11 @@ class _Component:
             setattr(self, key, kwargs[key])
         components.append(self)
 
+        # It doesn't make sense to have a check of the result value
+        # for a function or method with void return type.
+        if self.type == "void" and self.result_checks:
+            raise Exception("can't have result checks with a void return type")
+
     def get_predicate(self):
         "Return the expression used for validity checking."
         assert self.predicate and not isinstance(self.invalid, str)
@@ -112,6 +117,8 @@ class Function(_Component):
         postdefault=None,
         invalid=None,
         printer=None,
+        param_checks=None,
+        result_checks=None,
     ):
         super().__init__(
             comment=comment,
@@ -123,6 +130,8 @@ class Function(_Component):
             invalid=invalid,
             printer=printer,
             params=params,
+            param_checks=param_checks,
+            result_checks=result_checks,
         )
 
     def ftype(self):
@@ -444,6 +453,9 @@ with open("gdbarch.c", "w") as f:
                     f"  /* Do not check predicate: {c.get_predicate()}, allow call.  */",
                     file=f,
                 )
+            if c.param_checks:
+                for rule in c.param_checks:
+                    print(f"  gdb_assert ({rule});", file=f)
             print("  if (gdbarch_debug >= 2)", file=f)
             print(
                 f"""    gdb_printf (gdb_stdlog, "gdbarch_{c.name} called\\n");""",
@@ -451,8 +463,15 @@ with open("gdbarch.c", "w") as f:
             )
             print("  ", file=f, end="")
             if c.type != "void":
-                print("return ", file=f, end="")
+                if c.result_checks:
+                    print("auto result = ", file=f, end="")
+                else:
+                    print("return ", file=f, end="")
             print(f"gdbarch->{c.name} ({c.actuals()});", file=f)
+            if c.type != "void" and c.result_checks:
+                for rule in c.result_checks:
+                    print(f"  gdb_assert ({rule});", file=f)
+                print("  return result;", file=f)
             print("}", file=f)
             print(file=f)
             print("void", file=f)

                 reply	other threads:[~2022-10-02 16:28 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20221002162801.2A9403858D38@sourceware.org \
    --to=aburgess@sourceware.org \
    --cc=gdb-cvs@sourceware.org \
    /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 read-only IMAP folder(s) and NNTP newsgroup(s).