[v2] Make gdb.arch/i386-biarch-core.exp more robust

Message ID 1486462287-14715-1-git-send-email-lgustavo@codesourcery.com
State New, archived
Headers

Commit Message

Luis Machado Feb. 7, 2017, 10:11 a.m. UTC
  This test attempts to load a x86 core file no matter what target
architectures the tested GDB supports. If GDB doesn't know how to handle
a i386 target, it is very likely the core file will not be recognized.

In this case we should still attempt to load a core file to make sure GDB
doesn't crash or throws an internal error.  But we should not proceed to
try to read memory unconditionally.

This patch makes the test check for proper i386 arch support in GDB and bails
out if i386 is not supported and the core file format is not recognized.

This addresses the spurious aarch64-elf failures i'm seeing for this test.

gdb/testsuite/ChangeLog:

YYYY-MM-DD  Luis Machado  <lgustavo@codesourcery.com>

	* gdb.arch/i386-biarch-core.exp: Check for i386 arch support and
	return if core file is not recognized
---
 gdb/testsuite/gdb.arch/i386-biarch-core.exp | 30 ++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)
  

Comments

Pedro Alves Feb. 7, 2017, 10:51 a.m. UTC | #1
On 02/07/2017 10:11 AM, Luis Machado wrote:
> This test attempts to load a x86 core file no matter what target
> architectures the tested GDB supports. If GDB doesn't know how to handle
> a i386 target, it is very likely the core file will not be recognized.
> 
> In this case we should still attempt to load a core file to make sure GDB
> doesn't crash or throws an internal error.  But we should not proceed to
> try to read memory unconditionally.
> 
> This patch makes the test check for proper i386 arch support in GDB and bails
> out if i386 is not supported and the core file format is not recognized.
> 
> This addresses the spurious aarch64-elf failures i'm seeing for this test.
> 
> gdb/testsuite/ChangeLog:
> 
> YYYY-MM-DD  Luis Machado  <lgustavo@codesourcery.com>
> 
> 	* gdb.arch/i386-biarch-core.exp: Check for i386 arch support and
> 	return if core file is not recognized
> ---
>  gdb/testsuite/gdb.arch/i386-biarch-core.exp | 30 ++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/testsuite/gdb.arch/i386-biarch-core.exp b/gdb/testsuite/gdb.arch/i386-biarch-core.exp
> index 4c5f450..a05096c 100644
> --- a/gdb/testsuite/gdb.arch/i386-biarch-core.exp
> +++ b/gdb/testsuite/gdb.arch/i386-biarch-core.exp
> @@ -54,6 +54,18 @@ if {$corestat(size) != 102400} {
>      return -1
>  }
>  
> +# First check if this particular GDB supports i386, otherwise we should not
> +# expect the i386 core file to be loaded successfully.
> +set supports_arch_i386 1
> +set test "complete set architecture i386"
> +gdb_test_multiple $test $test {
> +    -re ".*\r\nset architecture i386\r\n(.*\r\n)?$gdb_prompt $" {

".*" at the start of a -re is implicit/redundant.

> +    }
> +    -re "\r\n$gdb_prompt $" {
> +        set supports_arch_i386 0
> +    }
> +}
> +
>  # Wrongly built GDB complains by:
>  # "..." is not a core dump: File format not recognized
>  # As the provided test core has 64bit PRSTATUS i386 built GDB cannot parse it.
> @@ -62,7 +74,23 @@ if {$corestat(size) != 102400} {
>  # objcopy as it corrupts the core file beyond all recognition.
>  # The output therefore does not matter much, just we should not get GDB
>  # internal error.
> -gdb_test "core-file ${corefile}" ".*" "core-file"
> +#
> +# If this particular GDB does not support i386, it is expected GDB will not
> +# recognize the core file.  If it does anyway, it should not crash.
> +set test "load core file"
> +gdb_test_multiple "core-file ${corefile}" $test {
> +    -re ".* no core file handler recognizes format(.*\r\n)?$gdb_prompt $" {

Ditto.  Also, why the "(....)?" in "(.*\r\n)?" ?

> +	if { $supports_arch_i386 } {
> +	    fail $test
> +	} else {
> +	    untested $test

The "load core file" test was a pass, this "else" outcome is expected.
What's untested is the next test.  Either write something like:

            pass $test
            untested ".text is readable (core file unrecognized)"
            return

or here write only:

            pass $test

and put the untested after the gdb_test_multiple, close to
the following gdb_test.

> +	    return
> +	}
> +    }
> +    -re "\r\n$gdb_prompt $" {
> +	pass $test
> +    }
> +}
>  

Thanks,
Pedro Alves
  
Luis Machado Feb. 7, 2017, 11:18 a.m. UTC | #2
On 02/07/2017 04:51 AM, Pedro Alves wrote:
> On 02/07/2017 10:11 AM, Luis Machado wrote:
>> This test attempts to load a x86 core file no matter what target
>> architectures the tested GDB supports. If GDB doesn't know how to handle
>> a i386 target, it is very likely the core file will not be recognized.
>>
>> In this case we should still attempt to load a core file to make sure GDB
>> doesn't crash or throws an internal error.  But we should not proceed to
>> try to read memory unconditionally.
>>
>> This patch makes the test check for proper i386 arch support in GDB and bails
>> out if i386 is not supported and the core file format is not recognized.
>>
>> This addresses the spurious aarch64-elf failures i'm seeing for this test.
>>
>> gdb/testsuite/ChangeLog:
>>
>> YYYY-MM-DD  Luis Machado  <lgustavo@codesourcery.com>
>>
>> 	* gdb.arch/i386-biarch-core.exp: Check for i386 arch support and
>> 	return if core file is not recognized
>> ---
>>  gdb/testsuite/gdb.arch/i386-biarch-core.exp | 30 ++++++++++++++++++++++++++++-
>>  1 file changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/gdb/testsuite/gdb.arch/i386-biarch-core.exp b/gdb/testsuite/gdb.arch/i386-biarch-core.exp
>> index 4c5f450..a05096c 100644
>> --- a/gdb/testsuite/gdb.arch/i386-biarch-core.exp
>> +++ b/gdb/testsuite/gdb.arch/i386-biarch-core.exp
>> @@ -54,6 +54,18 @@ if {$corestat(size) != 102400} {
>>      return -1
>>  }
>>
>> +# First check if this particular GDB supports i386, otherwise we should not
>> +# expect the i386 core file to be loaded successfully.
>> +set supports_arch_i386 1
>> +set test "complete set architecture i386"
>> +gdb_test_multiple $test $test {
>> +    -re ".*\r\nset architecture i386\r\n(.*\r\n)?$gdb_prompt $" {
>
> ".*" at the start of a -re is implicit/redundant.
>

Thanks. Good to know.

>> +    }
>> +    -re "\r\n$gdb_prompt $" {
>> +        set supports_arch_i386 0
>> +    }
>> +}
>> +
>>  # Wrongly built GDB complains by:
>>  # "..." is not a core dump: File format not recognized
>>  # As the provided test core has 64bit PRSTATUS i386 built GDB cannot parse it.
>> @@ -62,7 +74,23 @@ if {$corestat(size) != 102400} {
>>  # objcopy as it corrupts the core file beyond all recognition.
>>  # The output therefore does not matter much, just we should not get GDB
>>  # internal error.
>> -gdb_test "core-file ${corefile}" ".*" "core-file"
>> +#
>> +# If this particular GDB does not support i386, it is expected GDB will not
>> +# recognize the core file.  If it does anyway, it should not crash.
>> +set test "load core file"
>> +gdb_test_multiple "core-file ${corefile}" $test {
>> +    -re ".* no core file handler recognizes format(.*\r\n)?$gdb_prompt $" {
>
> Ditto.  Also, why the "(....)?" in "(.*\r\n)?" ?
>

I think this was from a copy/paste.

>> +	if { $supports_arch_i386 } {
>> +	    fail $test
>> +	} else {
>> +	    untested $test
>
> The "load core file" test was a pass, this "else" outcome is expected.
> What's untested is the next test.  Either write something like:
>
>             pass $test
>             untested ".text is readable (core file unrecognized)"
>             return
>
> or here write only:
>
>             pass $test
>
> and put the untested after the gdb_test_multiple, close to
> the following gdb_test.
>
>> +	    return
>> +	}
>> +    }
>> +    -re "\r\n$gdb_prompt $" {
>> +	pass $test
>> +    }
>> +}
>>

I'll address the above in a new version.

Thanks,
Luis
  

Patch

diff --git a/gdb/testsuite/gdb.arch/i386-biarch-core.exp b/gdb/testsuite/gdb.arch/i386-biarch-core.exp
index 4c5f450..a05096c 100644
--- a/gdb/testsuite/gdb.arch/i386-biarch-core.exp
+++ b/gdb/testsuite/gdb.arch/i386-biarch-core.exp
@@ -54,6 +54,18 @@  if {$corestat(size) != 102400} {
     return -1
 }
 
+# First check if this particular GDB supports i386, otherwise we should not
+# expect the i386 core file to be loaded successfully.
+set supports_arch_i386 1
+set test "complete set architecture i386"
+gdb_test_multiple $test $test {
+    -re ".*\r\nset architecture i386\r\n(.*\r\n)?$gdb_prompt $" {
+    }
+    -re "\r\n$gdb_prompt $" {
+        set supports_arch_i386 0
+    }
+}
+
 # Wrongly built GDB complains by:
 # "..." is not a core dump: File format not recognized
 # As the provided test core has 64bit PRSTATUS i386 built GDB cannot parse it.
@@ -62,7 +74,23 @@  if {$corestat(size) != 102400} {
 # objcopy as it corrupts the core file beyond all recognition.
 # The output therefore does not matter much, just we should not get GDB
 # internal error.
-gdb_test "core-file ${corefile}" ".*" "core-file"
+#
+# If this particular GDB does not support i386, it is expected GDB will not
+# recognize the core file.  If it does anyway, it should not crash.
+set test "load core file"
+gdb_test_multiple "core-file ${corefile}" $test {
+    -re ".* no core file handler recognizes format(.*\r\n)?$gdb_prompt $" {
+	if { $supports_arch_i386 } {
+	    fail $test
+	} else {
+	    untested $test
+	    return
+	}
+    }
+    -re "\r\n$gdb_prompt $" {
+	pass $test
+    }
+}
 
 # Test if at least the core file segments memory has been loaded.
 # https://bugzilla.redhat.com/show_bug.cgi?id=457187