Re: [PATCH-for-5.0] gdbstub: Do not use memset() on GByteArray

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 4/14/20 12:52 PM, Peter Maydell wrote:
> On Tue, 14 Apr 2020 at 11:24, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> Introduce gdb_get_zeroes() to fill a GByteArray with zeroes.
>>
>> Fixes: a010bdbe719 ("extend GByteArray to read register helpers")
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>   include/exec/gdbstub.h  | 9 +++++++++
>>   target/arm/gdbstub.c    | 3 +--
>>   target/xtensa/gdbstub.c | 6 ++----
>>   3 files changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
>> index 30b909ebd2..b52d9933ee 100644
>> --- a/include/exec/gdbstub.h
>> +++ b/include/exec/gdbstub.h
>> @@ -125,6 +125,15 @@ static inline int gdb_get_reg128(GByteArray *buf, uint64_t val_hi,
>>       return 16;
>>   }
>>
>> +static inline int gdb_get_zeroes(GByteArray *array, size_t len)
>> +{
>> +    for (size_t i = 0; i < len; i++) {
>> +        gdb_get_reg8(array, '\0');
>> +    }
>> +
>> +    return len;
>> +}
> 
> The other implementation option here would be
> 
>       guint oldlen = array->len;
>       g_byte_array_set_size(array, oldlen + len);
>       memset(array->data + oldlen, 0, len);

I thought about it but I'd rather not access GByteArray internals.

> 
> For length values < 16 my guess is the perf difference is
> not going to be noticeable though.

On ARM it is called with size=12:

target/arm/gdbstub.c:50:        memset(mem_buf, 0, 12);

On Xtensa it is only used with unimplemented registers:

         qemu_log_mask(LOG_UNIMP, "%s from reg %d of unsupported type %d\n",
                       __func__, n, reg->type);

If you prefer I can use a static empty buffer of 16 bytes and copy from it:

static inline int gdb_get_zeroes(GByteArray *array, size_t len)
{
     static const uint8_t zeroed_buf[16] = { };

     g_byte_array_append(array, zeroed_buf, MIN(len, sizeof(zeroed_buf)));
     for (size_t i = sizeof(zeroed_buf); i < len; i++) {
         gdb_get_reg8(array, '\0');
     }

     return len;
}

> 
> thanks
> -- PMM
>