Re: [PATCH v3 06/24] Unit tests for gdbserver memory tagging remote packets

[Luis Machado via Gdb-patches <gdb-patches@sourceware.org>]

On 12/25/20 5:13 PM, Simon Marchi wrote:
> 
> 
> On 2020-11-09 12:04 p.m., Luis Machado via Gdb-patches wrote:
>> Updates on v2:
>>
>> - Update unit tests to cope with additional tag type field in the remote
>> packets.
>>
>> --
>>
>> Add some unit testing to exercise the functions handling the qMemTags and
>> QMemTags packets as well as feature support.
>>
>> gdbserver/ChangeLog:
>>
>> YYYY-MM-DD  Luis Machado  <luis.machado@linaro.org>
>>
>> 	* server.cc (test_memory_tagging_functions): New function.
>> 	(captured_main): Register test_memory_tagging_functions.
>> ---
>>   gdbserver/server.cc | 100 ++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 100 insertions(+)
>>
>> diff --git a/gdbserver/server.cc b/gdbserver/server.cc
>> index 7eddd0e01f..6812b624f8 100644
>> --- a/gdbserver/server.cc
>> +++ b/gdbserver/server.cc
>> @@ -3647,6 +3647,103 @@ detach_or_kill_for_exit_cleanup ()
>>       }
>>   }
>>   
>> +#if GDB_SELF_TEST
>> +
>> +namespace selftests {
>> +
>> +static void
>> +test_memory_tagging_functions (void)
>> +{
>> +  /* Setup testing.  */
>> +  gdb::char_vector packet;
>> +  gdb::byte_vector tags, bv;
>> +  std::string expected;
>> +  packet.resize (32000);
>> +  CORE_ADDR addr;
>> +  size_t len;
>> +  int type;
>> +
>> +  /* Test parsing a qMemTags request.  */
>> +
>> +  /* Invalid request, addr len and type unchanged.  */
>> +  addr = 0xff;
>> +  len = 255;
>> +  type = 255;
>> +  strcpy (packet.data (), "qMemTags_wrong:0,0:0");
>> +  SELF_CHECK (parse_fmemtags_request (packet.data (), &addr, &len, &type) != 0);
>> +  SELF_CHECK (addr == 0xff && len == 255 && type == 255);
> 
> As I said earlier, I would have made parse_fmemtags_request assert that the
> packet begins with "qMemTags:" (instead of returning an error value), but it's
> really a nit and I can live with both.

Fixed now and removed the tests that cause an assertion.

> 
> Otherwise this patch LGTM.
> 
> Simon
>