[PATCH] Harden gdb.base/coredump-filter.exp

[PATCH] Harden gdb.base/coredump-filter.exp

[Luis Gustavo]

[-- Attachment #1: Type: text/plain, Size: 718 bytes --]

This testcase seems to assume the target is running Linux, so bare 
metal, simulators and other debugging stubs running different OS' will 
have a hard time executing some of the commands the testcase issues.

Even restricting the testcase to Linux systems (which the patch below 
does), there are still problems with, say, QEMU not providing PID 
information when "info inferior" is issued. As a consequence, the 
subsequent tests will either fail or will not make much sense.

The attached patch checks if PID information is available. If not, it 
just bails out and avoids running into a number of failures.

No regressions on x86-64. For QEMU, i see only a couple PASSes before it 
is done with the testcase.

OK?

[-- Attachment #2: coredump_filter.diff --]
[-- Type: text/x-patch, Size: 1164 bytes --]

2015-04-09  Luis Machado  <lgustavo@codesourcery.com>

	gdb/testsuite/
	* gdb.base/coredump-filter.exp: Restrict test to Linux systems only.
	Handle the case of targets that do not provide PID information.

diff --git a/gdb/testsuite/gdb.base/coredump-filter.exp b/gdb/testsuite/gdb.base/coredump-filter.exp
index f3203be..4c6c6ed 100644
--- a/gdb/testsuite/gdb.base/coredump-filter.exp
+++ b/gdb/testsuite/gdb.base/coredump-filter.exp
@@ -15,6 +15,12 @@
 
 standard_testfile
 
+# This test is Linux-only.
+if ![istarget *-*-linux*] then {
+    unsupported "coredump-filter.exp"
+    return -1
+}
+
 if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } {
     untested "could not compile test program"
     return -1
@@ -146,6 +152,11 @@ gdb_test_multiple "info inferiors" "getting inferior pid" {
     -re "process \($decimal\).*\r\n$gdb_prompt $" {
 	set infpid $expect_out(1,string)
     }
+    -re "Remote target.*$gdb_prompt $" {
+	# If the target does not provide PID information (like QEMU), just bail
+	# out as the rest of the test may rely on it, giving spurious failures.
+	return -1
+    }
 }
 
 # Get the main function's address.

Re: [PATCH] Harden gdb.base/coredump-filter.exp

[Yao Qi]

Luis Gustavo <luis_gustavo@mentor.com> writes:

> diff --git a/gdb/testsuite/gdb.base/coredump-filter.exp b/gdb/testsuite/gdb.base/coredump-filter.exp
> index f3203be..4c6c6ed 100644
> --- a/gdb/testsuite/gdb.base/coredump-filter.exp
> +++ b/gdb/testsuite/gdb.base/coredump-filter.exp
> @@ -15,6 +15,12 @@
>  
>  standard_testfile
>  
> +# This test is Linux-only.
> +if ![istarget *-*-linux*] then {
> +    unsupported "coredump-filter.exp"
> +    return -1
> +}
> +
>  if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } {
>      untested "could not compile test program"
>      return -1
> @@ -146,6 +152,11 @@ gdb_test_multiple "info inferiors" "getting inferior pid" {
>      -re "process \($decimal\).*\r\n$gdb_prompt $" {
>  	set infpid $expect_out(1,string)
>      }
> +    -re "Remote target.*$gdb_prompt $" {
> +	# If the target does not provide PID information (like QEMU), just bail

FAOD, QEMU here is QEMU user mode, isn't?

> +	# out as the rest of the test may rely on it, giving spurious failures.
> +	return -1
> +    }
>  }

We can just bail out if infpid is still "" after this gdb_test_multiple.

-- 
Yao (齐尧)

Re: [PATCH] Harden gdb.base/coredump-filter.exp

[Luis Machado]

[-- Attachment #1: Type: text/plain, Size: 1499 bytes --]

On 04/10/2015 05:06 AM, Yao Qi wrote:
> Luis Gustavo <luis_gustavo@mentor.com> writes:
>
>> diff --git a/gdb/testsuite/gdb.base/coredump-filter.exp b/gdb/testsuite/gdb.base/coredump-filter.exp
>> index f3203be..4c6c6ed 100644
>> --- a/gdb/testsuite/gdb.base/coredump-filter.exp
>> +++ b/gdb/testsuite/gdb.base/coredump-filter.exp
>> @@ -15,6 +15,12 @@
>>
>>   standard_testfile
>>
>> +# This test is Linux-only.
>> +if ![istarget *-*-linux*] then {
>> +    unsupported "coredump-filter.exp"
>> +    return -1
>> +}
>> +
>>   if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } {
>>       untested "could not compile test program"
>>       return -1
>> @@ -146,6 +152,11 @@ gdb_test_multiple "info inferiors" "getting inferior pid" {
>>       -re "process \($decimal\).*\r\n$gdb_prompt $" {
>>   	set infpid $expect_out(1,string)
>>       }
>> +    -re "Remote target.*$gdb_prompt $" {
>> +	# If the target does not provide PID information (like QEMU), just bail
>
> FAOD, QEMU here is QEMU user mode, isn't?
>

Yes, this is user mode. I suppose a system mode QEMU running gdbserver 
would work just fine. I made that information explicit in this updated 
version.

>> +	# out as the rest of the test may rely on it, giving spurious failures.
>> +	return -1
>> +    }
>>   }
>
> We can just bail out if infpid is still "" after this gdb_test_multiple.
>

That still gives me one FAIL due to not matching what was expected. Did 
you mean a check inside this gdb_test_multiple?



[-- Attachment #2: coredump-filter.diff --]
[-- Type: text/x-patch, Size: 1285 bytes --]

2015-04-10  Luis Machado  <lgustavo@codesourcery.com>

	gdb/testsuite/
	* gdb.base/coredump-filter.exp: Restrict test to Linux systems only.
	Handle the case of targets that do not provide PID information.
---
 gdb/testsuite/gdb.base/coredump-filter.exp | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/gdb/testsuite/gdb.base/coredump-filter.exp b/gdb/testsuite/gdb.base/coredump-filter.exp
index f3203be..8350981 100644
--- a/gdb/testsuite/gdb.base/coredump-filter.exp
+++ b/gdb/testsuite/gdb.base/coredump-filter.exp
@@ -15,6 +15,12 @@
 
 standard_testfile
 
+# This test is Linux-only.
+if ![istarget *-*-linux*] then {
+    untested "coredump-filter.exp"
+    return -1
+}
+
 if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } {
     untested "could not compile test program"
     return -1
@@ -146,6 +152,12 @@ gdb_test_multiple "info inferiors" "getting inferior pid" {
     -re "process \($decimal\).*\r\n$gdb_prompt $" {
 	set infpid $expect_out(1,string)
     }
+    -re "Remote target.*$gdb_prompt $" {
+	# If the target does not provide PID information (like usermode QEMU),
+	# just bail out as the rest of the test may rely on it, giving spurious
+	# failures.
+	return -1
+    }
 }
 
 # Get the main function's address.
-- 
1.9.1

Re: [PATCH] Harden gdb.base/coredump-filter.exp

[Yao Qi]

>>
>> We can just bail out if infpid is still "" after this gdb_test_multiple.
>>
>
> That still gives me one FAIL due to not matching what was expected. Did
> you mean a check inside this gdb_test_multiple?
>

No, you are right.  I thought something different.

>
> 2015-04-10  Luis Machado<lgustavo@codesourcery.com>
>
> 	gdb/testsuite/
> 	* gdb.base/coredump-filter.exp: Restrict test to Linux systems only.
> 	Handle the case of targets that do not provide PID information.

It is OK to me.

-- 
Yao (齐尧)

Re: [PATCH] Harden gdb.base/coredump-filter.exp

[Luis Machado]

On 04/10/2015 12:16 PM, Yao Qi wrote:
>>>
>>> We can just bail out if infpid is still "" after this gdb_test_multiple.
>>>
>>
>> That still gives me one FAIL due to not matching what was expected. Did
>> you mean a check inside this gdb_test_multiple?
>>
>
> No, you are right.  I thought something different.
>
>>
>> 2015-04-10  Luis Machado<lgustavo@codesourcery.com>
>>
>>     gdb/testsuite/
>>     * gdb.base/coredump-filter.exp: Restrict test to Linux systems only.
>>     Handle the case of targets that do not provide PID information.
>
> It is OK to me.
>

Thanks. I've pushed this now.

Re: [PATCH] Harden gdb.base/coredump-filter.exp

[Pedro Alves]

> +# This test is Linux-only.
> +if ![istarget *-*-linux*] then {
> +    unsupported "coredump-filter.exp"
> +    return -1
> +}

s/unsupported/untested/, but it's the same:

 https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#A.22untested.22_calls

Thanks,
Pedro Alves