| Message ID | 20240418201039.236867-8-gustavo.romero@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Add another way to check tagged addresses on remote targets | expand |
On 4/18/24 21:10, 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 3d034bb1ef8..cfb54de157d 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -15682,6 +15682,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); > @@ -15748,6 +15750,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 malformed reply of incorrect length. */ > + reply = "0104A590001234006"; > + strcpy (packet.data (), reply.c_str ()); > + /* Because this is a malformed reply 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 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 This is OK. Thanks for the series. Let us know if you need us to push it for you. Approved-By: Luis Machado <luis.machado@arm.com> Tested-By: Luis Machado <luis.machado@arm.com>
Hi Luis, Thiago, Eli, and Tom, On 4/19/24 4:53 AM, Luis Machado wrote: > On 4/18/24 21:10, 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 3d034bb1ef8..cfb54de157d 100644 >> --- a/gdb/remote.c >> +++ b/gdb/remote.c >> @@ -15682,6 +15682,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); >> @@ -15748,6 +15750,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 malformed reply of incorrect length. */ >> + reply = "0104A590001234006"; >> + strcpy (packet.data (), reply.c_str ()); >> + /* Because this is a malformed reply 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 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 > > This is OK. Thanks for the series. > > Let us know if you need us to push it for you. > > Approved-By: Luis Machado <luis.machado@arm.com> > Tested-By: Luis Machado <luis.machado@arm.com> Thanks a lot for all the reviews! Yip, I need somebody to push the patchset. :-) Cheers, Gustavo
On 4/19/24 15:00, Gustavo Romero wrote: > Hi Luis, Thiago, Eli, and Tom, > > On 4/19/24 4:53 AM, Luis Machado wrote: >> On 4/18/24 21:10, 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 3d034bb1ef8..cfb54de157d 100644 >>> --- a/gdb/remote.c >>> +++ b/gdb/remote.c >>> @@ -15682,6 +15682,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); >>> @@ -15748,6 +15750,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 malformed reply of incorrect length. */ >>> + reply = "0104A590001234006"; >>> + strcpy (packet.data (), reply.c_str ()); >>> + /* Because this is a malformed reply 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 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 >> >> This is OK. Thanks for the series. >> >> Let us know if you need us to push it for you. >> >> Approved-By: Luis Machado <luis.machado@arm.com> >> Tested-By: Luis Machado <luis.machado@arm.com> > > Thanks a lot for all the reviews! > > Yip, I need somebody to push the patchset. :-) > > > Cheers, > Gustavo Pushed now.
diff --git a/gdb/remote.c b/gdb/remote.c index 3d034bb1ef8..cfb54de157d 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -15682,6 +15682,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); @@ -15748,6 +15750,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 malformed reply of incorrect length. */ + reply = "0104A590001234006"; + strcpy (packet.data (), reply.c_str ()); + /* Because this is a malformed reply 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 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
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(+)