Re: [PATCH v2] Add SVR4 psABI specific parser for AUXV entries

[Kamil Rytarowski <n54@gmx.com>]

On 09.04.2020 02:04, Simon Marchi wrote:
>> diff --git a/gdb/auxv.c b/gdb/auxv.c
>> index c13d7a22eb9..28e8d2f704d 100644
>> --- a/gdb/auxv.c
>> +++ b/gdb/auxv.c
>> @@ -248,34 +248,65 @@ memory_xfer_auxv (struct target_ops *ops,
>>    return procfs_xfer_auxv (readbuf, writebuf, offset, len, xfered_len);
>>  }
>>
>> -/* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
>> -   Return 0 if *READPTR is already at the end of the buffer.
>> -   Return -1 if there is insufficient buffer for a whole entry.
>> -   Return 1 if an entry was read into *TYPEP and *VALP.  */
>> -int
>> -default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
>> -		   gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
>> +/* This function compared to other auxv_parse functions: it takes the size of
>> +   the auxv type field as a parameter.  */
>> +
>> +static int
>> +generic_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
>> +		    gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp,
>> +		    int sizeof_auxv_type)
>>  {
>> -  const int sizeof_auxv_field = gdbarch_ptr_bit (target_gdbarch ())
>> -				/ TARGET_CHAR_BIT;
>> -  const enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
>> +  struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
>> +  const int sizeof_auxv_val = TYPE_LENGTH (ptr_type);
>> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>>    gdb_byte *ptr = *readptr;
>>
>>    if (endptr == ptr)
>>      return 0;
>>
>> -  if (endptr - ptr < sizeof_auxv_field * 2)
>> +  if (endptr - ptr < 2 * sizeof_auxv_val)
>>      return -1;
>>
>> -  *typep = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
>> -  ptr += sizeof_auxv_field;
>> -  *valp = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
>> -  ptr += sizeof_auxv_field;
>> +  *typep = extract_unsigned_integer (ptr, sizeof_auxv_type, byte_order);
>> +  /* Even if the auxv type takes less space than an auxv value, there is
>> +     padding after the type such that the value is aligned on a multiple of
>> +     its size (and this is why we advance by `sizeof_auxv_val` and not
>> +     `sizeof_auxv_type`.  */
>
> The parenthesis in the comment is not closed.
>

Fixed.

>> diff --git a/gdb/auxv.h b/gdb/auxv.h
>> index a5a932ec80e..c1d11ea2c7b 100644
>> --- a/gdb/auxv.h
>> +++ b/gdb/auxv.h
>> @@ -25,12 +25,27 @@
>>  /* See "include/elf/common.h" for the definition of valid AT_* values.  */
>>
>>  /* The default implementation of to_auxv_parse, used by the target
>> -   stack.  */
>> +   stack.
>>
>> +   Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
>> +   Return 0 if *READPTR is already at the end of the buffer.
>> +   Return -1 if there is insufficient buffer for a whole entry.
>> +   Return 1 if an entry was read into *TYPEP and *VALP.  */
>>  extern int default_auxv_parse (struct target_ops *ops, gdb_byte **readptr,
>>  			       gdb_byte *endptr, CORE_ADDR *typep,
>>  			       CORE_ADDR *valp);
>>
>> +/* The SVR4 psABI implementation of to_auxv_parse, that uses an int to
>> +   store the type rather than long as assumed by the default parser.
>> +
>> +   Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
>> +   Return 0 if *READPTR is already at the end of the buffer.
>> +   Return -1 if there is insufficient buffer for a whole entry.
>> +   Return 1 if an entry was read into *TYPEP and *VALP.  */
>> +extern int svr4_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
>> +			       gdb_byte *endptr, CORE_ADDR *typep,
>> +			       CORE_ADDR *valp);
>
> The last two lines are not properly aligned on the parenthesis.
>

Fixed.

> Simon
>

Please see v3.