Re: [PATCH] gdb: error out if architecture does not implement any "return_value" hook

Simon Marchi via Gdb-patches
2023-02-28T19:53:33Z

>> diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
>> index caa65c334eca..7ceecbf5d223 100644
>> --- a/gdb/gdbarch_components.py
>> +++ b/gdb/gdbarch_components.py
>> @@ -902,11 +902,11 @@ for instance).
>> ("struct value **", "read_value"),
>> ("const gdb_byte *", "writebuf"),
>> ],
>> - predefault="default_gdbarch_return_value",
>> # If we're using the default, then the other method must be set;
>> # but if we aren't using the default here then the other method
>> # must not be set.
>
> I don't think this comment aligns with the postdefault code. It says
> "If we're using the default, ..." but "we" here is
> 'return_value_as_value', but we're actually checking 'return_value'.

Oops, I should have removed it probably.

>> - invalid="(gdbarch->return_value_as_value == default_gdbarch_return_value) == (gdbarch->return_value == nullptr)",
>> + postdefault="gdbarch->return_value != nullptr ? default_gdbarch_return_value : nullptr",
>
> If you search for the postdefault string you'll notice this code is not
> actually generated anywhere! This is a consequence of also having
> defined a predicate.

Wow, that's bad! I really thought it was generated, maybe it was when
trying some other combination of attributes.

> As I say above, the gdbarch.py algorithm could do with some updating in
> a few cases.
>
> The good news is, I already have a patch that fixes this problem. I was
> going to include a link to it here, but turns out I never actually
> posted it! I'm going to try to get that post on the list today, I've
> tried it locally, and with my patch your postdefault code is generated
> correctly.

Thanks a lot. I agree with pretty much all you said above. I think a
GDB port could live without a "return value" hook, but again there might
be no point for us to support that extra complexity. I'll go look at
your series.

Simon

Simon