[v2] Improve supports_memtag proc

Message ID 20210329150521.799524-1-luis.machado@linaro.org
State New
Headers show
Series
  • [v2] Improve supports_memtag proc
Related show

Commit Message

Luis Machado via Gdb-patches March 29, 2021, 3:05 p.m.
After understanding a bit more about the caching procs in GDB's testsuite,
I noticed the current implementation of the supports_memtag check is
error-prone.

It has to be executed when GDB has already been started, which may or may
not be true.

The following patch turns the proc into a standalone check that starts/exits
a GDB instance, not relying on any existing state.

I've updated the proc's documentation to state that it *has* to be called
first thing in the testcase, in order to prevent future uses in the middle
of testcases.

If called in the middle of a testcase, and if it is not cached yet, it will
trash the current state of the GDB session.

gdb/testsuite/ChangeLog:

YYYY-MM-DD  Luis Machado  <luis.machado@linaro.org>

	* lib/gdb.exp (supports_memtag): Make more robust.
	* gdb.base/memtag.exp: Move memtag support check.
	* gdb.arch/aarch64-mte.exp: Likewise.
---
 gdb/testsuite/gdb.arch/aarch64-mte.exp | 14 ++++++------
 gdb/testsuite/gdb.base/memtag.exp      | 14 ++++++------
 gdb/testsuite/lib/gdb.exp              | 30 ++++++++++++++++++++++----
 3 files changed, 40 insertions(+), 18 deletions(-)

-- 
2.25.1

Comments

Andrew Burgess March 29, 2021, 3:39 p.m. | #1
* Luis Machado via Gdb-patches <gdb-patches@sourceware.org> [2021-03-29 12:05:21 -0300]:

> After understanding a bit more about the caching procs in GDB's testsuite,

> I noticed the current implementation of the supports_memtag check is

> error-prone.

> 

> It has to be executed when GDB has already been started, which may or may

> not be true.

> 

> The following patch turns the proc into a standalone check that starts/exits

> a GDB instance, not relying on any existing state.

> 

> I've updated the proc's documentation to state that it *has* to be called

> first thing in the testcase, in order to prevent future uses in the middle

> of testcases.

> 

> If called in the middle of a testcase, and if it is not cached yet, it will

> trash the current state of the GDB session.

> 

> gdb/testsuite/ChangeLog:

> 

> YYYY-MM-DD  Luis Machado  <luis.machado@linaro.org>

> 

> 	* lib/gdb.exp (supports_memtag): Make more robust.

> 	* gdb.base/memtag.exp: Move memtag support check.

> 	* gdb.arch/aarch64-mte.exp: Likewise.


LGTM.

Thanks,
Andrew


> ---

>  gdb/testsuite/gdb.arch/aarch64-mte.exp | 14 ++++++------

>  gdb/testsuite/gdb.base/memtag.exp      | 14 ++++++------

>  gdb/testsuite/lib/gdb.exp              | 30 ++++++++++++++++++++++----

>  3 files changed, 40 insertions(+), 18 deletions(-)

> 

> diff --git a/gdb/testsuite/gdb.arch/aarch64-mte.exp b/gdb/testsuite/gdb.arch/aarch64-mte.exp

> index 62dfc863748..88a4001a8e6 100644

> --- a/gdb/testsuite/gdb.arch/aarch64-mte.exp

> +++ b/gdb/testsuite/gdb.arch/aarch64-mte.exp

> @@ -48,6 +48,13 @@ if {![is_aarch64_target]} {

>      return

>  }

>  

> +# Targets that don't support memory tagging should not execute the

> +# runtime memory tagging tests.

> +if {![supports_memtag]} {

> +    unsupported "memory tagging unsupported"

> +    return -1

> +}

> +

>  standard_testfile

>  if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {

>      return -1

> @@ -58,13 +65,6 @@ if ![runto_main] {

>      return -1

>  }

>  

> -# Targets that don't support memory tagging should not execute the

> -# runtime memory tagging tests.

> -if {![supports_memtag]} {

> -    unsupported "memory tagging unsupported"

> -    return -1

> -}

> -

>  gdb_breakpoint "access_memory"

>  

>  if [gdb_continue "access_memory"] {

> diff --git a/gdb/testsuite/gdb.base/memtag.exp b/gdb/testsuite/gdb.base/memtag.exp

> index aa21f7f3654..4b19d9f1488 100644

> --- a/gdb/testsuite/gdb.base/memtag.exp

> +++ b/gdb/testsuite/gdb.base/memtag.exp

> @@ -15,6 +15,13 @@

>  

>  # Smoke testing for the various memory tagging commands in GDB.

>  

> +# Targets that don't support memory tagging should not execute the

> +# runtime memory tagging tests.

> +if {![supports_memtag]} {

> +    unsupported "memory tagging unsupported"

> +    return -1

> +}

> +

>  set u_msg "Memory tagging not supported or disabled by the current architecture\."

>  

>  standard_testfile

> @@ -44,13 +51,6 @@ if ![runto_main] {

>      return -1

>  }

>  

> -# Targets that don't support memory tagging should not execute the

> -# runtime memory tagging tests.

> -if {![supports_memtag]} {

> -    unsupported "memory tagging unsupported"

> -    return -1

> -}

> -

>  # With the program running, try to use the memory tagging commands.

>  with_test_prefix "during program execution" {

>      set msg "Argument required \\(address or pointer\\)\."

> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp

> index 73fea3a104d..7ae84b397b1 100644

> --- a/gdb/testsuite/lib/gdb.exp

> +++ b/gdb/testsuite/lib/gdb.exp

> @@ -2749,19 +2749,41 @@ proc supports_get_siginfo_type {} {

>  }

>  

>  # Return 1 if memory tagging is supported at runtime, otherwise return 0.

> +# Since this test starts and exits GDB, this check must be executed at the

> +# beginning of a testcase.

>  

>  gdb_caching_proc supports_memtag {

> -    global gdb_prompt

> +    global srcdir subdir gdb_prompt

> +

> +    set msg "supports_memtag"

> +

> +    # Compile a test program.

> +    set src { int main() { return 0; } }

> +    if {![gdb_simple_compile $msg $src executable]} {

> +        return 1

> +    }

> +

> +    # No error message, compilation succeeded so now run it via gdb.

> +    clean_restart $obj

>  

> +    if ![runto_main] {

> +        return 0

> +    }

> +

> +    # Assume memory tagging is not supported.

> +    set retval 0

>      gdb_test_multiple "memory-tag check" "" {

>  	-re "Memory tagging not supported or disabled by the current architecture\..*$gdb_prompt $" {

> -	  return 0

>  	}

>  	-re "Argument required \\(address or pointer\\).*$gdb_prompt $" {

> -	    return 1

> +	    set retval 1

>  	}

>      }

> -    return 0

> +    gdb_exit

> +    remote_file build delete $obj

> +

> +    verbose "$msg:  returning $retval" 2

> +    return $retval

>  }

>  

>  # Return 1 if the target supports hardware single stepping.

> -- 

> 2.25.1

>

Patch

diff --git a/gdb/testsuite/gdb.arch/aarch64-mte.exp b/gdb/testsuite/gdb.arch/aarch64-mte.exp
index 62dfc863748..88a4001a8e6 100644
--- a/gdb/testsuite/gdb.arch/aarch64-mte.exp
+++ b/gdb/testsuite/gdb.arch/aarch64-mte.exp
@@ -48,6 +48,13 @@  if {![is_aarch64_target]} {
     return
 }
 
+# Targets that don't support memory tagging should not execute the
+# runtime memory tagging tests.
+if {![supports_memtag]} {
+    unsupported "memory tagging unsupported"
+    return -1
+}
+
 standard_testfile
 if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
     return -1
@@ -58,13 +65,6 @@  if ![runto_main] {
     return -1
 }
 
-# Targets that don't support memory tagging should not execute the
-# runtime memory tagging tests.
-if {![supports_memtag]} {
-    unsupported "memory tagging unsupported"
-    return -1
-}
-
 gdb_breakpoint "access_memory"
 
 if [gdb_continue "access_memory"] {
diff --git a/gdb/testsuite/gdb.base/memtag.exp b/gdb/testsuite/gdb.base/memtag.exp
index aa21f7f3654..4b19d9f1488 100644
--- a/gdb/testsuite/gdb.base/memtag.exp
+++ b/gdb/testsuite/gdb.base/memtag.exp
@@ -15,6 +15,13 @@ 
 
 # Smoke testing for the various memory tagging commands in GDB.
 
+# Targets that don't support memory tagging should not execute the
+# runtime memory tagging tests.
+if {![supports_memtag]} {
+    unsupported "memory tagging unsupported"
+    return -1
+}
+
 set u_msg "Memory tagging not supported or disabled by the current architecture\."
 
 standard_testfile
@@ -44,13 +51,6 @@  if ![runto_main] {
     return -1
 }
 
-# Targets that don't support memory tagging should not execute the
-# runtime memory tagging tests.
-if {![supports_memtag]} {
-    unsupported "memory tagging unsupported"
-    return -1
-}
-
 # With the program running, try to use the memory tagging commands.
 with_test_prefix "during program execution" {
     set msg "Argument required \\(address or pointer\\)\."
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 73fea3a104d..7ae84b397b1 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2749,19 +2749,41 @@  proc supports_get_siginfo_type {} {
 }
 
 # Return 1 if memory tagging is supported at runtime, otherwise return 0.
+# Since this test starts and exits GDB, this check must be executed at the
+# beginning of a testcase.
 
 gdb_caching_proc supports_memtag {
-    global gdb_prompt
+    global srcdir subdir gdb_prompt
+
+    set msg "supports_memtag"
+
+    # Compile a test program.
+    set src { int main() { return 0; } }
+    if {![gdb_simple_compile $msg $src executable]} {
+        return 1
+    }
+
+    # No error message, compilation succeeded so now run it via gdb.
+    clean_restart $obj
 
+    if ![runto_main] {
+        return 0
+    }
+
+    # Assume memory tagging is not supported.
+    set retval 0
     gdb_test_multiple "memory-tag check" "" {
 	-re "Memory tagging not supported or disabled by the current architecture\..*$gdb_prompt $" {
-	  return 0
 	}
 	-re "Argument required \\(address or pointer\\).*$gdb_prompt $" {
-	    return 1
+	    set retval 1
 	}
     }
-    return 0
+    gdb_exit
+    remote_file build delete $obj
+
+    verbose "$msg:  returning $retval" 2
+    return $retval
 }
 
 # Return 1 if the target supports hardware single stepping.