* [PATCH] Make gdb.arch/i386-biarch-core.exp more robust
@ 2017-01-31 13:55 Luis Machado
2017-02-06 17:28 ` Pedro Alves
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Luis Machado @ 2017-01-31 13:55 UTC (permalink / raw)
To: gdb-patches
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(-)
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
--
2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Make gdb.arch/i386-biarch-core.exp more robust
2017-01-31 13:55 [PATCH] Make gdb.arch/i386-biarch-core.exp more robust Luis Machado
@ 2017-02-06 17:28 ` Pedro Alves
2017-02-06 17:59 ` Luis Machado
2017-02-07 10:11 ` [PATCH,v2] " Luis Machado
2017-02-07 11:26 ` [PATCH, v3] " Luis Machado
2 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2017-02-06 17:28 UTC (permalink / raw)
To: Luis Machado, gdb-patches
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Make gdb.arch/i386-biarch-core.exp more robust
2017-02-06 17:28 ` Pedro Alves
@ 2017-02-06 17:59 ` Luis Machado
2017-02-06 18:03 ` Pedro Alves
0 siblings, 1 reply; 10+ messages in thread
From: Luis Machado @ 2017-02-06 17:59 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH,v2] Make gdb.arch/i386-biarch-core.exp more robust
2017-01-31 13:55 [PATCH] Make gdb.arch/i386-biarch-core.exp more robust Luis Machado
2017-02-06 17:28 ` Pedro Alves
@ 2017-02-07 10:11 ` Luis Machado
2017-02-07 10:51 ` Pedro Alves
2017-02-07 11:26 ` [PATCH, v3] " Luis Machado
2 siblings, 1 reply; 10+ messages in thread
From: Luis Machado @ 2017-02-07 10:11 UTC (permalink / raw)
To: gdb-patches; +Cc: palves
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 $" {
+ }
+ -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
--
2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH,v2] Make gdb.arch/i386-biarch-core.exp more robust
2017-02-07 10:11 ` [PATCH,v2] " Luis Machado
@ 2017-02-07 10:51 ` Pedro Alves
2017-02-07 11:18 ` Luis Machado
0 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2017-02-07 10:51 UTC (permalink / raw)
To: Luis Machado, gdb-patches
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH,v2] Make gdb.arch/i386-biarch-core.exp more robust
2017-02-07 10:51 ` Pedro Alves
@ 2017-02-07 11:18 ` Luis Machado
0 siblings, 0 replies; 10+ messages in thread
From: Luis Machado @ 2017-02-07 11:18 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH, v3] Make gdb.arch/i386-biarch-core.exp more robust
2017-01-31 13:55 [PATCH] Make gdb.arch/i386-biarch-core.exp more robust Luis Machado
2017-02-06 17:28 ` Pedro Alves
2017-02-07 10:11 ` [PATCH,v2] " Luis Machado
@ 2017-02-07 11:26 ` Luis Machado
2017-02-07 11:29 ` Pedro Alves
2 siblings, 1 reply; 10+ messages in thread
From: Luis Machado @ 2017-02-07 11:26 UTC (permalink / raw)
To: gdb-patches; +Cc: palves
Changes in v3:
- Addressed Pedro's comments.
- Removed useless bits in the regular expressions.
- Fixed pass/untested calls.
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 | 31 ++++++++++++++++++++++++++++-
1 file changed, 30 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..fd76369 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,24 @@ 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 {
+ pass $test
+ untested ".text is readable (core file unrecognized)"
+ 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
--
2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-02-13 13:15 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-31 13:55 [PATCH] Make gdb.arch/i386-biarch-core.exp more robust Luis Machado
2017-02-06 17:28 ` Pedro Alves
2017-02-06 17:59 ` Luis Machado
2017-02-06 18:03 ` Pedro Alves
2017-02-07 10:11 ` [PATCH,v2] " Luis Machado
2017-02-07 10:51 ` Pedro Alves
2017-02-07 11:18 ` Luis Machado
2017-02-07 11:26 ` [PATCH, v3] " Luis Machado
2017-02-07 11:29 ` Pedro Alves
2017-02-13 13:15 ` Luis Machado
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).