[PATCH v3 16/28] Change DWARF stack to use new dwarf_entry classes

Zoran Zaric zoran.zaric@amd.com
Thu Nov 4 12:43:37 GMT 2021


>> +
>> +  dwarf_location *location = dynamic_cast<dwarf_location *> (entry.get ());
>> +  gdb_assert (location != nullptr);
>> +
>> +  return location->to_value (address_type);
>> +}
>> +
>> +/* Set of functions that perform different arithmetic operations
>> +   on a given DWARF value arguments.
> 
> Maybe use 'on dwarf_value arguments.' (otherwise the 'a' and 'arguments'
> seems strange to me).
> 

I see what you mean. Thanks.

>> +
>> +   Currently the existing struct value operations are used under the
>> +   hood to avoid the code duplication.  Vector types are planned to be
>> +   promoted to base types in the future anyway which means that the
>> +   operations subset needed is just going to grow anyway.  */
>> +
>> +/* Compare two DWARF value's ARG1 and ARG2 for equality in a context
>> +   of a value entry comparison.  */
>> +
>> +static bool
>> +dwarf_value_equal_op (dwarf_value &arg1, dwarf_value &arg2)
>> +{
>> +  struct value *arg1_value = arg1.to_gdb_value (arg1.type ());
>> +  struct value *arg2_value = arg2.to_gdb_value (arg2.type ());
>> +  return value_equal (arg1_value, arg2_value);
>> +}
>> +
>> +/* Compare if DWARF value ARG1 is lesser then DWARF value ARG2 in a
> 
> is lesser then -> is less than
> 

The funny thing is that this was there in the original code and nobody
noticed until now. Thanks.


>> diff --git a/gdb/dwarf2/expr.h b/gdb/dwarf2/expr.h
>> index 16c5e710ff5..aaae381f4fe 100644
>> --- a/gdb/dwarf2/expr.h
>> +++ b/gdb/dwarf2/expr.h
>> @@ -25,6 +25,22 @@
>>   #include "leb128.h"
>>   #include "gdbtypes.h"
>>
>> +/* Base class that describes entries found on a DWARF expression
>> +   evaluation stack.  */
>> +
>> +class dwarf_entry
>> +{
>> +public:
>> +  /* Not expected to be called on it's own.  */
>> +  dwarf_entry () = default;
> 
> This was mentioned in a earlier patch but I re-state it here because it
> is easy to miss it when reabasing since this part is temporarily moved
> from another file: this ctor could be turned protected.
> 

Thank you for tracking it through the series for me :).

Like discussed on some previous patches (PATCH v3 6/28) comment threads, 
I will also add a comment at the beginning of the expr.c file in this 
patch to explain that the evaluator is using the HOST_CHAR_SIZE constant
instead the hard coded number 8 until there is a proper support for 
different sizes bytes.

I was advised to use this constant by a maintainer Pedro Alves, because 
the value of that constant is apparently always guaranteed to be 8.

Much appreciate it,
Zoran


More information about the Gdb-patches mailing list