diff mbox

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

Message ID 1485870927-12623-1-git-send-email-lgustavo@codesourcery.com
State New
Headers show

Commit Message

Luis Machado Jan. 31, 2017, 1:55 p.m. UTC
This test attempts to load a x86 core file no matter what the target
architecture is. If the architecture is not x86, GDB will not recognize
the core file and therefore won't have any memory to inspect. All we will
have is a memory read error, resulting in a FAIL.

The following patch addresses this by checking if we successfully loaded
the core file. If not, just return. Otherwise it keeps testing.

gdb/testsuite/ChangeLog:

2017-01-31  Luis Machado  <lgustavo@codesourcery.com>

	* gdb.arch/i386-biarch-core.exp: Return if core file was not
	recognized.
---
 gdb/testsuite/gdb.arch/i386-biarch-core.exp | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Pedro Alves Feb. 6, 2017, 5:28 p.m. UTC | #1
On 01/31/2017 01:55 PM, Luis Machado wrote:
> This test attempts to load a x86 core file no matter what the target
> architecture is. If the architecture is not x86, GDB will not recognize
> the core file and therefore won't have any memory to inspect. All we will
> have is a memory read error, resulting in a FAIL.
> 
> The following patch addresses this by checking if we successfully loaded
> the core file. If not, just return. Otherwise it keeps testing.
> 
> gdb/testsuite/ChangeLog:
> 
> 2017-01-31  Luis Machado  <lgustavo@codesourcery.com>
> 
> 	* gdb.arch/i386-biarch-core.exp: Return if core file was not
> 	recognized.

This seems to contradict a bit the point of the test, which says:

> # Test if at least the core file segments memory has been loaded.

when:

> # Wrongly built GDB complains by:
> # "..." is not a core dump: File format not recognized

I.e., would we be potentially losing coverage?  Assurance
that we're not I think should be part of the rationale for
this change.

Does the test pass for you if your non --target=x86 build
ends up including x86 support, with e.g., --enable-targets=all?

If it does, then maybe we should check whether the GDB
build supports x86, by e.g., doing:

  gdb_test_multiple "complete set architecture i386" ...

and seeing if something comes out.

then only do "unsupported" if x86 is not supported, otherwise,
the memory read failure is really a fail.

Thanks,
Pedro Alves
Luis Machado Feb. 6, 2017, 5:58 p.m. UTC | #2
On 02/06/2017 11:28 AM, Pedro Alves wrote:
> On 01/31/2017 01:55 PM, Luis Machado wrote:
>> This test attempts to load a x86 core file no matter what the target
>> architecture is. If the architecture is not x86, GDB will not recognize
>> the core file and therefore won't have any memory to inspect. All we will
>> have is a memory read error, resulting in a FAIL.
>>
>> The following patch addresses this by checking if we successfully loaded
>> the core file. If not, just return. Otherwise it keeps testing.
>>
>> gdb/testsuite/ChangeLog:
>>
>> 2017-01-31  Luis Machado  <lgustavo@codesourcery.com>
>>
>> 	* gdb.arch/i386-biarch-core.exp: Return if core file was not
>> 	recognized.
>
> This seems to contradict a bit the point of the test, which says:
>
>> # Test if at least the core file segments memory has been loaded.
>
> when:
>
>> # Wrongly built GDB complains by:
>> # "..." is not a core dump: File format not recognized

You're right. The same message would've come out in a broken gdb trying 
to load the test core file.

>
> I.e., would we be potentially losing coverage?  Assurance
> that we're not I think should be part of the rationale for
> this change.
>
> Does the test pass for you if your non --target=x86 build
> ends up including x86 support, with e.g., --enable-targets=all?
>

It should, because then gdb will recognize it.

> If it does, then maybe we should check whether the GDB
> build supports x86, by e.g., doing:
>
>   gdb_test_multiple "complete set architecture i386" ...
>

Yes, indeed. This test should never try to force a gdb that doesn't 
understand i386/x86-64 core files to load one.

> and seeing if something comes out.
>
> then only do "unsupported" if x86 is not supported, otherwise,
> the memory read failure is really a fail.

Right. In this case it was a fail because gdb didn't even load the core 
file. As opposed to failing to read from a core file memory segment.

I'll adjust this.
Pedro Alves Feb. 6, 2017, 6:02 p.m. UTC | #3
On 02/06/2017 05:58 PM, Luis Machado wrote:

> Yes, indeed. This test should never try to force a gdb that doesn't
> understand i386/x86-64 core files to load one.

TBC, I think it should, to make sure gdb doesn't crash.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/testsuite/gdb.arch/i386-biarch-core.exp b/gdb/testsuite/gdb.arch/i386-biarch-core.exp
index 4c5f450..2776ac3 100644
--- a/gdb/testsuite/gdb.arch/i386-biarch-core.exp
+++ b/gdb/testsuite/gdb.arch/i386-biarch-core.exp
@@ -62,7 +62,16 @@  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"
+set test "load core file"
+gdb_test_multiple "core-file ${corefile}" $test {
+    -re ".* no core file handler recognizes format(.*\r\n)?$gdb_prompt $" {
+	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