From 7df4240040277fb752fd010f5a947cf2d32bc3a5 Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Thu, 1 Sep 2022 14:01:11 +0100 Subject: [PATCH] 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. --- 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) -- 2.30.2