Re: [PATCH v5 7/8] gdb/testsuite: Add unit tests for qIsAddressTagged packet

[Luis Machado <luis.machado@arm.com>]

Based on what I mentioned in 06/08, we need to clearly define if garbage replies
starting with 00 and 01 are valid or not, and document that. We also need to adjust
these tests.

On 4/17/24 22:04, Gustavo Romero wrote:
> Add unit tests for testing qIsAddressTagged packet request creation and
> reply checks.
> 
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> ---
>  gdb/remote.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
> 
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 2bb962955b5..bc2cfed2595 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -15681,6 +15681,8 @@ test_memory_tagging_functions ()
>    scoped_restore restore_memtag_support_
>      = make_scoped_restore (&config->support);
>  
> +  struct gdbarch *gdbarch = current_inferior ()->arch ();
> +
>    /* Test memory tagging packet support.  */
>    config->support = PACKET_SUPPORT_UNKNOWN;
>    SELF_CHECK (remote.supports_memory_tagging () == false);
> @@ -15747,6 +15749,71 @@ test_memory_tagging_functions ()
>    create_store_memtags_request (packet, 0xdeadbeef, 255, 1, tags);
>    SELF_CHECK (memcmp (packet.data (), expected.c_str (),
>  		      expected.length ()) == 0);
> +
> +  /* Test creating a qIsAddressTagged request.  */
> +  expected = "qIsAddressTagged:deadbeef";
> +  create_is_address_tagged_request (gdbarch, packet, 0xdeadbeef);
> +  SELF_CHECK (strcmp (packet.data (), expected.c_str ()) == 0);
> +
> +  /* Test error reply on qIsAddressTagged request.  */
> +  reply = "E00";
> +  strcpy (packet.data (), reply.c_str ());
> +  /* is_tagged must not change, hence it's tested too.  */
> +  bool is_tagged = false;
> +  SELF_CHECK (check_is_address_tagged_reply (&remote, packet, is_tagged) ==
> +	      false);
> +  SELF_CHECK (is_tagged == false);
> +
> +  /* Test 'tagged' as reply.  */
> +  reply = "01";
> +  strcpy (packet.data (), reply.c_str ());
> +  /* Because the byte is 01, is_tagged should be set to true.  */
> +  is_tagged = false;
> +  SELF_CHECK (check_is_address_tagged_reply (&remote, packet, is_tagged) ==
> +	      true);
> +  SELF_CHECK (is_tagged == true);
> +
> +  /* Test 'not tagged' as reply.  */
> +  reply = "00";
> +  strcpy (packet.data (), reply.c_str ());
> +  /* Because the byte is 00, is_tagged should be set to false.  */
> +  is_tagged = true;
> +  SELF_CHECK (check_is_address_tagged_reply (&remote, packet, is_tagged) ==
> +	      true);
> +  SELF_CHECK (is_tagged == false);
> +
> +  /* Test an invalid reply (neither 00 nor 01).  */
> +  reply = "04";
> +  strcpy (packet.data (), reply.c_str ());
> +  /* Because the byte is invalid is_tagged must not change.  */
> +  is_tagged = false;
> +  SELF_CHECK (check_is_address_tagged_reply (&remote, packet, is_tagged) ==
> +	      false);
> +  SELF_CHECK (is_tagged == false);
> +
> +  /* Test proper first byte truncation.  */
> +  reply = "0104A590001234006";
> +  strcpy (packet.data (), reply.c_str ());
> +  /* Because the first byte is 01, is_tagged should be set to true.  */
> +  is_tagged = false;
> +  SELF_CHECK (check_is_address_tagged_reply (&remote, packet, is_tagged) ==
> +	      true);
> +  SELF_CHECK (is_tagged == true);
> +
> +  /* Test empty reply.  */
> +  reply = "";
> +  strcpy (packet.data (), reply.c_str ());
> +  /* is_tagged must not change, hence it's tested too.  */
> +  is_tagged = true;
> +  /* On the previous tests, qIsAddressTagged packet was auto detected and set
> +     as supported.  But an empty reply means the packet is unsupported, so for
> +     testing the empty reply the support is reset to unknown state, otherwise
> +     packet_ok will complain.   */
> +  remote.m_features.m_protocol_packets[PACKET_qIsAddressTagged].support =
> +    PACKET_SUPPORT_UNKNOWN;
> +  SELF_CHECK (check_is_address_tagged_reply (&remote, packet, is_tagged) ==
> +	      false);
> +  SELF_CHECK (is_tagged == true);
>  }
>  
>  static void

Otherwise this is OK to me, pending matching documentation updates.