gdb/gdbarch: compare some fields against 0 verify_gdbarch
authorAndrew Burgess <aburgess@redhat.com>
Thu, 10 Mar 2022 11:18:18 +0000 (11:18 +0000)
committerAndrew Burgess <aburgess@redhat.com>
Mon, 14 Mar 2022 14:08:05 +0000 (14:08 +0000)
commita5118a18db47c8ccaf4995fbb62e2a1eb377fa3e
tree639e2747cc99ba81309d392cc73cff872d0793d9
parent23bade95de322dead7fbd33368dce271c2911773
gdb/gdbarch: compare some fields against 0 verify_gdbarch

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.

More than not being checked, the field wasn't mentioned at all.

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.

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:

  "Otherwise, the check is done against 0 (really NULL for function
  pointers, but same idea)."

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.

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.

In this commit I propose two changes:

 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

 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.

After doing this, I re-generated the gdbarch files and saw that the
following gdbarch fields now had new validation checks:

  register_type
  believe_pcc_promotion
  register_to_value
  value_to_register
  frame_red_zone_size
  displaced_step_restore_all_in_ptid
  solib_symbols_extension

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.

And so, for all of the other fields, I've changed the property
definition on gdbarch-components.py, setting the 'invalid' field to
False.

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.

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.

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.
gdb/gdbarch-components.py
gdb/gdbarch.c
gdb/gdbarch.py