From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1726) id E16FC3857829; Mon, 14 Mar 2022 14:08:26 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E16FC3857829 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Andrew Burgess To: gdb-cvs@sourceware.org Subject: [binutils-gdb] gdb/gdbarch: compare some fields against 0 verify_gdbarch X-Act-Checkin: binutils-gdb X-Git-Author: Andrew Burgess X-Git-Refname: refs/heads/master X-Git-Oldrev: 23bade95de322dead7fbd33368dce271c2911773 X-Git-Newrev: a5118a18db47c8ccaf4995fbb62e2a1eb377fa3e Message-Id: <20220314140826.E16FC3857829@sourceware.org> Date: Mon, 14 Mar 2022 14:08:26 +0000 (GMT) X-BeenThere: gdb-cvs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 Mar 2022 14:08:27 -0000 https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3Da5118a18db47= c8ccaf4995fbb62e2a1eb377fa3e commit a5118a18db47c8ccaf4995fbb62e2a1eb377fa3e Author: Andrew Burgess Date: Thu Mar 10 11:18:18 2022 +0000 gdb/gdbarch: compare some fields against 0 verify_gdbarch =20 After the previous commit, which removes the predicate function gdbarch_register_type_p, I noticed that the gdbarch->register_type field was not checked at in the verify_gdbarch function. =20 More than not being checked, the field wasn't mentioned at all. =20 I find this strange, I would expect that every field would at least be mentioned - we already generate comments for some fields saying that this field is _not_ being checked, so the fact that this field isn't being checked looks (to me), like this field is somehow slipping through the cracks. =20 The comment at the top of gdbarch-components.py tries to explain how the validation is done. I didn't understand this comment completely, but, I think this final sentence: =20 "Otherwise, the check is done against 0 (really NULL for function pointers, but same idea)." =20 Means that, if non of the other cases apply, then the field should be checked against 0, with 0 indicating that the field is invalid (was not set by the tdep code). However, this is clearly not being done. =20 Looking in gdbarch.py at the code to generate verify_gdbarch we do find that there is a case that is not handled, the case where the 'invalid' field is set true True, but non of the other cases apply. =20 In this commit I propose two changes: =20 1. Handle the case where the 'invalid' field of a property is set to True, this should perform a check for the field of gdbarch still being set to 0, and =20 2. If the if/else series that generates verify_gdbarch doesn't handle a property then we should raise an exception. This means that if a property is added which isn't handled, we should no longer silently ignore it. =20 After doing this, I re-generated the gdbarch files and saw that the following gdbarch fields now had new validation checks: =20 register_type believe_pcc_promotion register_to_value value_to_register frame_red_zone_size displaced_step_restore_all_in_ptid solib_symbols_extension =20 Looking at how these are currently set in the various -tdep.c files, I believe the only one of these that is required to be set for all architectures is the register_type field. =20 And so, for all of the other fields, I've changed the property definition on gdbarch-components.py, setting the 'invalid' field to False. =20 Now, after re-generation, the register_type field is checked against 0, thus an architecture that doesn't set_gdbarch_register_type will now fail during validation. For all the other fields we skip the validation, in which case, it is find for an architecture to not set this field. =20 My expectation is that there should be no user visible changes after this commit. Certainly for all fields except register_type, all I've really done is cause some extra comments to be generated, so I think that's clearly fine. =20 For the register_type field, my claim is that any architecture that didn't provide this would fail when creating its register cache, and I couldn't spot an architecture that doesn't provide this hook. As such, I think this change should be fine too. Diff: --- gdb/gdbarch-components.py | 12 ++++++------ gdb/gdbarch.c | 11 +++++++++++ gdb/gdbarch.py | 10 ++++++++++ 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py index aa0b3682593..c820ddae764 100644 --- a/gdb/gdbarch-components.py +++ b/gdb/gdbarch-components.py @@ -740,7 +740,7 @@ FRAME corresponds to the longjmp frame. Value( type=3D"int", name=3D"believe_pcc_promotion", - invalid=3DTrue, + invalid=3DFalse, ) =20 Method( @@ -762,7 +762,7 @@ Function( ("int *", "optimizedp"), ("int *", "unavailablep"), ], - invalid=3DTrue, + invalid=3DFalse, ) =20 Function( @@ -774,7 +774,7 @@ Function( ("struct type *", "type"), ("const gdb_byte *", "buf"), ], - invalid=3DTrue, + invalid=3DFalse, ) =20 Method( @@ -1086,7 +1086,7 @@ Method( Value( type=3D"int", name=3D"frame_red_zone_size", - invalid=3DTrue, + invalid=3DFalse, ) =20 Method( @@ -1767,7 +1767,7 @@ contents of all displaced step buffers in the child's= address space. type=3D"void", name=3D"displaced_step_restore_all_in_ptid", params=3D[("inferior *", "parent_inf"), ("ptid_t", "child_ptid")], - invalid=3DTrue, + invalid=3DFalse, ) =20 Method( @@ -2298,7 +2298,7 @@ compared to the names of the files GDB should load fo= r debug info. """, type=3D"const char *", name=3D"solib_symbols_extension", - invalid=3DTrue, + invalid=3DFalse, printer=3D"pstring (gdbarch->solib_symbols_extension)", ) =20 diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c index 28e1fbc2c71..9fdcf1505fe 100644 --- a/gdb/gdbarch.c +++ b/gdb/gdbarch.c @@ -443,6 +443,8 @@ verify_gdbarch (struct gdbarch *gdbarch) /* Skip verify of dwarf2_reg_to_regnum, invalid_p =3D=3D 0 */ if (gdbarch->register_name =3D=3D 0) log.puts ("\n\tregister_name"); + if (gdbarch->register_type =3D=3D 0) + log.puts ("\n\tregister_type"); /* Skip verify of dummy_id, invalid_p =3D=3D 0 */ /* Skip verify of deprecated_fp_regnum, invalid_p =3D=3D 0 */ /* Skip verify of push_dummy_call, has predicate. */ @@ -456,7 +458,10 @@ verify_gdbarch (struct gdbarch *gdbarch) /* Skip verify of cannot_fetch_register, invalid_p =3D=3D 0 */ /* Skip verify of cannot_store_register, invalid_p =3D=3D 0 */ /* Skip verify of get_longjmp_target, has predicate. */ + /* Skip verify of believe_pcc_promotion, invalid_p =3D=3D 0 */ /* Skip verify of convert_register_p, invalid_p =3D=3D 0 */ + /* Skip verify of register_to_value, invalid_p =3D=3D 0 */ + /* Skip verify of value_to_register, invalid_p =3D=3D 0 */ /* Skip verify of value_from_register, invalid_p =3D=3D 0 */ /* Skip verify of pointer_to_address, invalid_p =3D=3D 0 */ /* Skip verify of address_to_pointer, invalid_p =3D=3D 0 */ @@ -488,6 +493,7 @@ verify_gdbarch (struct gdbarch *gdbarch) /* Skip verify of frame_num_args, has predicate. */ /* Skip verify of frame_align, has predicate. */ /* Skip verify of stabs_argument_has_addr, invalid_p =3D=3D 0 */ + /* Skip verify of frame_red_zone_size, invalid_p =3D=3D 0 */ /* Skip verify of convert_from_func_ptr_addr, invalid_p =3D=3D 0 */ /* Skip verify of addr_bits_remove, invalid_p =3D=3D 0 */ /* Skip verify of significant_addr_bit, invalid_p =3D=3D 0 */ @@ -538,6 +544,7 @@ verify_gdbarch (struct gdbarch *gdbarch) if ((! gdbarch->displaced_step_finish) !=3D (! gdbarch->displaced_step_p= repare)) log.puts ("\n\tdisplaced_step_finish"); /* Skip verify of displaced_step_copy_insn_closure_by_addr, has predicat= e. */ + /* Skip verify of displaced_step_restore_all_in_ptid, invalid_p =3D=3D 0= */ /* Skip verify of relocate_instruction, has predicate. */ /* Skip verify of overlay_update, has predicate. */ /* Skip verify of core_read_description, has predicate. */ @@ -573,6 +580,7 @@ verify_gdbarch (struct gdbarch *gdbarch) /* Skip verify of guess_tracepoint_registers, invalid_p =3D=3D 0 */ /* Skip verify of auto_charset, invalid_p =3D=3D 0 */ /* Skip verify of auto_wide_charset, invalid_p =3D=3D 0 */ + /* Skip verify of solib_symbols_extension, invalid_p =3D=3D 0 */ /* Skip verify of has_dos_based_file_system, invalid_p =3D=3D 0 */ /* Skip verify of gen_return_address, invalid_p =3D=3D 0 */ /* Skip verify of info_proc, has predicate. */ @@ -2485,6 +2493,7 @@ int gdbarch_believe_pcc_promotion (struct gdbarch *gdbarch) { gdb_assert (gdbarch !=3D NULL); + /* Skip verify of believe_pcc_promotion, invalid_p =3D=3D 0 */ if (gdbarch_debug >=3D 2) fprintf_unfiltered (gdb_stdlog, "gdbarch_believe_pcc_promotion called\= n"); return gdbarch->believe_pcc_promotion; @@ -3091,6 +3100,7 @@ int gdbarch_frame_red_zone_size (struct gdbarch *gdbarch) { gdb_assert (gdbarch !=3D NULL); + /* Skip verify of frame_red_zone_size, invalid_p =3D=3D 0 */ if (gdbarch_debug >=3D 2) fprintf_unfiltered (gdb_stdlog, "gdbarch_frame_red_zone_size called\n"= ); return gdbarch->frame_red_zone_size; @@ -4822,6 +4832,7 @@ const char * gdbarch_solib_symbols_extension (struct gdbarch *gdbarch) { gdb_assert (gdbarch !=3D NULL); + /* Skip verify of solib_symbols_extension, invalid_p =3D=3D 0 */ if (gdbarch_debug >=3D 2) fprintf_unfiltered (gdb_stdlog, "gdbarch_solib_symbols_extension calle= d\n"); return gdbarch->solib_symbols_extension; diff --git a/gdb/gdbarch.py b/gdb/gdbarch.py index 3bd6400355e..c89d19ccbcf 100755 --- a/gdb/gdbarch.py +++ b/gdb/gdbarch.py @@ -360,6 +360,16 @@ with open("gdbarch.c", "w") as f: elif c.predefault is not None: print(f" if (gdbarch->{c.name} =3D=3D {c.predefault})", file= =3Df) print(f""" log.puts ("\\n\\t{c.name}");""", file=3Df) + elif c.invalid is True: + print(f" if (gdbarch->{c.name} =3D=3D 0)", file=3Df) + print(f""" log.puts ("\\n\\t{c.name}");""", file=3Df) + else: + # We should not allow ourselves to simply do nothing here + # because no other case applies. If we end up here then + # either the input data needs adjusting so one of the + # above cases matches, or we need additional cases adding + # here. + raise Exception("unhandled case when generating gdbarch valida= tion") print(" if (!log.empty ())", file=3Df) print(" internal_error (__FILE__, __LINE__,", file=3Df) print(""" _("verify_gdbarch: the following are invalid ...%s"),""= ", file=3Df)