Patchwork [2/2] gdb.multi/multi-arch-exec.exp: Also test -m32 => -m64

login
register
mail settings
Submitter Pedro Alves
Date Oct. 9, 2017, 10:16 a.m.
Message ID <1507544177-24880-3-git-send-email-palves@redhat.com>
Download mbox | patch
Permalink /patch/23406/
State New
Headers show

Comments

Pedro Alves - Oct. 9, 2017, 10:16 a.m.
The gdb.multi/multi-arch-exec.exp testcase currently tests execing
from -m64 to -m32, but does not test the other direction.  For
thoroughness, this commit fixes that.  Without the fix in the previous
commit for example ("Multi-arch exec, more register reading
avoidance"), on x86_64 we would get different symptoms depending on
"execing direction".  Vis:

  Continuing.
  Truncated register 50 in remote 'g' packet
  Truncated register 50 in remote 'g' packet
  (gdb) FAIL: gdb.multi/multi-arch-exec.exp: first_arch=1: selected_thread=2: follow_exec_mode=same: continue across exec that changes architecture

Vs:

  Continuing.
  Remote 'g' packet reply is too long (expected 440 bytes, got 816 bytes): daffffffffffffff0000[snip]
  Remote 'g' packet reply is too long (expected 440 bytes, got 816 bytes): daffffffffffffff0000[snip]
  (gdb) FAIL: gdb.multi/multi-arch-exec.exp: first_arch=2: selected_thread=2: follow_exec_mode=same: continue across exec that changes architecture

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	Test both arch1=>arch2 and arch2=>arch1.

	* gdb.multi/multi-arch-exec.exp (exec1, srcfile1, binfile1, exec2)
	(srcfile2, binfile2, march1, march2): Remove globals.  Largely
	factored out to...
	(append_arch1_options, append_arch2_options, append_arch_options)
	(build_executables): New procedures.
	(do_test): New 'first_arch' parameter.  Use it to define 'exec1'
	local.
	(top level): Add new 'first_arch' testing axis.
---
 gdb/testsuite/gdb.multi/multi-arch-exec.exp | 166 ++++++++++++++++++++--------
 1 file changed, 121 insertions(+), 45 deletions(-)
Simon Marchi - Oct. 9, 2017, 2:02 p.m.
On 2017-10-09 06:16 AM, Pedro Alves wrote:
> The gdb.multi/multi-arch-exec.exp testcase currently tests execing
> from -m64 to -m32, but does not test the other direction.  For
> thoroughness, this commit fixes that.  Without the fix in the previous
> commit for example ("Multi-arch exec, more register reading
> avoidance"), on x86_64 we would get different symptoms depending on
> "execing direction".  Vis:
> 
>   Continuing.
>   Truncated register 50 in remote 'g' packet
>   Truncated register 50 in remote 'g' packet
>   (gdb) FAIL: gdb.multi/multi-arch-exec.exp: first_arch=1: selected_thread=2: follow_exec_mode=same: continue across exec that changes architecture
> 
> Vs:
> 
>   Continuing.
>   Remote 'g' packet reply is too long (expected 440 bytes, got 816 bytes): daffffffffffffff0000[snip]
>   Remote 'g' packet reply is too long (expected 440 bytes, got 816 bytes): daffffffffffffff0000[snip]
>   (gdb) FAIL: gdb.multi/multi-arch-exec.exp: first_arch=2: selected_thread=2: follow_exec_mode=same: continue across exec that changes architecture

Good idea.

> gdb/testsuite/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
> 
> 	Test both arch1=>arch2 and arch2=>arch1.
> 
> 	* gdb.multi/multi-arch-exec.exp (exec1, srcfile1, binfile1, exec2)
> 	(srcfile2, binfile2, march1, march2): Remove globals.  Largely
> 	factored out to...
> 	(append_arch1_options, append_arch2_options, append_arch_options)
> 	(build_executables): New procedures.
> 	(do_test): New 'first_arch' parameter.  Use it to define 'exec1'
> 	local.
> 	(top level): Add new 'first_arch' testing axis.
> ---
>  gdb/testsuite/gdb.multi/multi-arch-exec.exp | 166 ++++++++++++++++++++--------
>  1 file changed, 121 insertions(+), 45 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.multi/multi-arch-exec.exp b/gdb/testsuite/gdb.multi/multi-arch-exec.exp
> index 891eb14..bba8f54 100644
> --- a/gdb/testsuite/gdb.multi/multi-arch-exec.exp
> +++ b/gdb/testsuite/gdb.multi/multi-arch-exec.exp
> @@ -29,55 +29,124 @@ if [istarget "i?86-*linux*"] {
>      return
>  }
>  
> -# Can't use standard_testfile, we want executables with specialized
> -# names.
> -set exec1 "multi-arch-exec"
> -set srcfile1 multi-arch-exec.c
> -set binfile1 [standard_output_file ${exec1}]
> -
> -set exec2 "multi-arch-exec-hello"
> -set srcfile2 hello.c
> -set binfile2 [standard_output_file ${exec2}]
> -
> -# Build two executables, one for each arch.
> -
> -if [istarget "s390*-*-*"] {
> -    set march1 "-m64"
> -    set march2 "-m31"
> -} elseif { [istarget "aarch64*-*-*"] } {
> -    set march1 ""
> -    set march2 ""
> -} else {
> -    set march1 "-m64"
> -    set march2 "-m32"
> +# The testcase builds two programs, each of its own architecture.  For
> +# example, one built with -m64, another with -m32.  The exact compiler
> +# options depends on target triplet.  We generically refer to the
> +# architectures simply as 'architecture 1' and 'architecture 2'.  Each
> +# program is actually built twice, once for each architecture, because
> +# we test both execing from arch1 to arch2 and from arch2 to arch1.
> +# The architecture of the executable that execs is encoded in the
> +# binaries' names, like so:
> +#
> +#    $first_arch-multi-arch-exec         # execing program
> +#    $first_arch-multi-arch-exec-hello   # execed program
> +
> +# Append the options necessary to build a program for architecture 1
> +# to the OPTIONS_VAR list.
> +
> +proc append_arch1_options {options_var} {
> +    upvar 1 $options_var options
> +
> +    if { [istarget "aarch64*-*-*"] } {
> +	return 1
> +    }
> +
> +    lappend options "additional_flags=-m64"
> +    return 1
>  }
>  
> -if { [prepare_for_testing "failed to prepare" ${exec1} "${srcfile1}" \
> -	  [list debug pthreads \
> -	       additional_flags=${march1}]] } {
> -    return -1
> +# Append the options necessary to build a program for architecture 2
> +# to the OPTIONS_VAR list.
> +
> +proc append_arch2_options {options_var} {
> +    upvar 1 $options_var options
> +
> +    if { [istarget "aarch64*-*-*"] } {
> +	if {[info exists ARM_CC_FOR_TARGET]} {
> +	    lappend options "compiler=${ARM_CC_FOR_TARGET}"
> +	    return 1
> +	} else {
> +	    unsupported "ARM compiler is not known"
> +	    return 0
> +	}
> +    }
> +
> +    if [istarget "s390*-*-*"] {
> +	set march "-m31"
> +    } else {
> +	set march "-m32"
> +    }
> +    lappend options "additional_flags=${march}"
> +    return 1
>  }
>  
> -set options [list debug]
> +# Append the options necessary to build a program for architecture
> +# ARCH to the OPTIONS_VAR list.  Returns true on success.
>  
> -if { [istarget "aarch64*-*-*"] } {
> -    if {[info exists ARM_CC_FOR_TARGET]} {
> -	lappend options "compiler=${ARM_CC_FOR_TARGET}"
> +proc append_arch_options {arch options_var} {
> +    upvar 1 $options_var options
> +
> +    if {$arch == 1} {
> +	append_arch1_options options
> +    } elseif {$arch == 2} {
> +	append_arch2_options options
>      } else {
> -	unsupported "ARM compiler is not known"
> -	return -1
> +	error "unhandled architecture: $arch"
>      }
> -} else {
> -    lappend options "additional_flags=${march2}"
>  }
>  
> -if { [prepare_for_testing "failed to prepare" ${exec2} "${srcfile2}" \
> -	  $options] } {
> -    return -1
> +# Build the executables for testing with FIRST_ARCH (either 1 or 2) as
> +# the architecture before the exec.  Returns true on success.
> +
> +proc build_executables { first_arch } {
> +
> +    # Can't use standard_testfile, we want executables with specialized
> +    # names.
> +    set exec1 "$first_arch-multi-arch-exec"
> +    set srcfile1 multi-arch-exec.c
> +    set binfile1 [standard_output_file ${exec1}]
> +
> +    set exec2 "$first_arch-multi-arch-exec-hello"
> +    set srcfile2 hello.c
> +    set binfile2 [standard_output_file ${exec2}]
> +
> +    # Build two executables, one for each arch.
> +
> +    if {$first_arch == 1} {
> +	set arch1 1
> +	set arch2 2
> +    } elseif {$first_arch == 2} {
> +	set arch1 2
> +	set arch2 1
> +    } else {
> +	error "unhandled first architecture: $first_arch"
> +    }

I would rename arch1/arch2 to arch_from/arch_to.  1/2 is already used
to name the two architectures, so it's confusing if it's also used to
mean from/to.

> +
> +    set options [list debug pthreads]
> +    if {![append_arch_options $arch1 options]} {
> +	return 0
> +    }
> +
> +    if { [build_executable "failed to prepare" ${exec1} "${srcfile1}" \
> +	      $options] } {
> +	return 0
> +    }
> +
> +    set options [list debug]
> +    if {![append_arch_options $arch2 options]} {

append_arch_options doesn't explicitly return something.  Is the result of the
last command executed in the procedure returned implicitly?

Otherwise, LGTM.

Simon

Patch

diff --git a/gdb/testsuite/gdb.multi/multi-arch-exec.exp b/gdb/testsuite/gdb.multi/multi-arch-exec.exp
index 891eb14..bba8f54 100644
--- a/gdb/testsuite/gdb.multi/multi-arch-exec.exp
+++ b/gdb/testsuite/gdb.multi/multi-arch-exec.exp
@@ -29,55 +29,124 @@  if [istarget "i?86-*linux*"] {
     return
 }
 
-# Can't use standard_testfile, we want executables with specialized
-# names.
-set exec1 "multi-arch-exec"
-set srcfile1 multi-arch-exec.c
-set binfile1 [standard_output_file ${exec1}]
-
-set exec2 "multi-arch-exec-hello"
-set srcfile2 hello.c
-set binfile2 [standard_output_file ${exec2}]
-
-# Build two executables, one for each arch.
-
-if [istarget "s390*-*-*"] {
-    set march1 "-m64"
-    set march2 "-m31"
-} elseif { [istarget "aarch64*-*-*"] } {
-    set march1 ""
-    set march2 ""
-} else {
-    set march1 "-m64"
-    set march2 "-m32"
+# The testcase builds two programs, each of its own architecture.  For
+# example, one built with -m64, another with -m32.  The exact compiler
+# options depends on target triplet.  We generically refer to the
+# architectures simply as 'architecture 1' and 'architecture 2'.  Each
+# program is actually built twice, once for each architecture, because
+# we test both execing from arch1 to arch2 and from arch2 to arch1.
+# The architecture of the executable that execs is encoded in the
+# binaries' names, like so:
+#
+#    $first_arch-multi-arch-exec         # execing program
+#    $first_arch-multi-arch-exec-hello   # execed program
+
+# Append the options necessary to build a program for architecture 1
+# to the OPTIONS_VAR list.
+
+proc append_arch1_options {options_var} {
+    upvar 1 $options_var options
+
+    if { [istarget "aarch64*-*-*"] } {
+	return 1
+    }
+
+    lappend options "additional_flags=-m64"
+    return 1
 }
 
-if { [prepare_for_testing "failed to prepare" ${exec1} "${srcfile1}" \
-	  [list debug pthreads \
-	       additional_flags=${march1}]] } {
-    return -1
+# Append the options necessary to build a program for architecture 2
+# to the OPTIONS_VAR list.
+
+proc append_arch2_options {options_var} {
+    upvar 1 $options_var options
+
+    if { [istarget "aarch64*-*-*"] } {
+	if {[info exists ARM_CC_FOR_TARGET]} {
+	    lappend options "compiler=${ARM_CC_FOR_TARGET}"
+	    return 1
+	} else {
+	    unsupported "ARM compiler is not known"
+	    return 0
+	}
+    }
+
+    if [istarget "s390*-*-*"] {
+	set march "-m31"
+    } else {
+	set march "-m32"
+    }
+    lappend options "additional_flags=${march}"
+    return 1
 }
 
-set options [list debug]
+# Append the options necessary to build a program for architecture
+# ARCH to the OPTIONS_VAR list.  Returns true on success.
 
-if { [istarget "aarch64*-*-*"] } {
-    if {[info exists ARM_CC_FOR_TARGET]} {
-	lappend options "compiler=${ARM_CC_FOR_TARGET}"
+proc append_arch_options {arch options_var} {
+    upvar 1 $options_var options
+
+    if {$arch == 1} {
+	append_arch1_options options
+    } elseif {$arch == 2} {
+	append_arch2_options options
     } else {
-	unsupported "ARM compiler is not known"
-	return -1
+	error "unhandled architecture: $arch"
     }
-} else {
-    lappend options "additional_flags=${march2}"
 }
 
-if { [prepare_for_testing "failed to prepare" ${exec2} "${srcfile2}" \
-	  $options] } {
-    return -1
+# Build the executables for testing with FIRST_ARCH (either 1 or 2) as
+# the architecture before the exec.  Returns true on success.
+
+proc build_executables { first_arch } {
+
+    # Can't use standard_testfile, we want executables with specialized
+    # names.
+    set exec1 "$first_arch-multi-arch-exec"
+    set srcfile1 multi-arch-exec.c
+    set binfile1 [standard_output_file ${exec1}]
+
+    set exec2 "$first_arch-multi-arch-exec-hello"
+    set srcfile2 hello.c
+    set binfile2 [standard_output_file ${exec2}]
+
+    # Build two executables, one for each arch.
+
+    if {$first_arch == 1} {
+	set arch1 1
+	set arch2 2
+    } elseif {$first_arch == 2} {
+	set arch1 2
+	set arch2 1
+    } else {
+	error "unhandled first architecture: $first_arch"
+    }
+
+    set options [list debug pthreads]
+    if {![append_arch_options $arch1 options]} {
+	return 0
+    }
+
+    if { [build_executable "failed to prepare" ${exec1} "${srcfile1}" \
+	      $options] } {
+	return 0
+    }
+
+    set options [list debug]
+    if {![append_arch_options $arch2 options]} {
+	return 0
+    }
+
+    if { [build_executable "failed to prepare" ${exec2} "${srcfile2}" \
+	      $options] } {
+	return 0
+    }
+
+    return 1
 }
 
-proc do_test { mode selected_thread } {
-	global exec1
+proc do_test { first_arch mode selected_thread } {
+	set exec1 "$first_arch-multi-arch-exec"
 
 	clean_restart ${exec1}
 	if ![runto all_started] then {
@@ -102,12 +171,19 @@  proc do_test { mode selected_thread } {
 	gdb_test "continue" "Breakpoint 2, main.*" "continue across exec that changes architecture"
 }
 
-# Test handling the exec event with either the main thread or the
-# second thread selected.  This tries to ensure that GDB doesn't read
-# registers off of the execing thread before figuring out its
-# architecture.
-foreach_with_prefix selected_thread {1 2} {
-    foreach_with_prefix follow_exec_mode {"same" "new"} {
-	do_test $follow_exec_mode $selected_thread
+# Test both arch1=>arch2 and arch2=>arch1.
+foreach_with_prefix first_arch {1 2} {
+    if {![build_executables $first_arch]} {
+	continue
+    }
+
+    # Test handling the exec event with either the main thread or the
+    # second thread selected.  This tries to ensure that GDB doesn't read
+    # registers off of the execing thread before figuring out its
+    # architecture.
+    foreach_with_prefix selected_thread {1 2} {
+	foreach_with_prefix follow_exec_mode {"same" "new"} {
+	    do_test $first_arch $follow_exec_mode $selected_thread
+	}
     }
 }