diff mbox

[3/3] Remove HP-UX references fom testsuite

Message ID 5677F519.2010000@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves Dec. 21, 2015, 12:48 p.m. UTC 
  I looked this one over too.  A few minor comments below, but
otherwise looks good to me.  Thanks for doing this!

On 12/19/2015 11:30 PM, Simon Marchi wrote:

> 	* gdb.multi/bkpt-multi-exec.ex: Likewise.p

Typo: "ex: Likewise.p" -> "exp: Likewise."


> +gdb_test_multiple "catch vfork" "$name" {
> +    -re "Catchpoint \[0-9\]* .vfork..*$gdb_prompt $" {
> +	pass $name
> +    }
> +    -re "Catch of vfork not yet implemented.*$gdb_prompt $" {

This case can be removed.  GDB doesn't ever output this.

> +	pass $name
>      }
>  }
>



> --- a/gdb/testsuite/gdb.base/display.exp
> +++ b/gdb/testsuite/gdb.base/display.exp
> @@ -194,15 +194,9 @@ gdb_test "print/r j" " = 0x0\[\\r\\n\]+" "debug test output 2a"
>  gdb_test "print j"   " = 0\[\\r\\n\]+"   "debug test output 3"
>
>  # x/0 j doesn't produce any output and terminates PA64 process when testing
> -if [istarget "hppa2.0w-hp-hpux11*"] {
> -    xfail "'x/0 j' terminates PA64 process - skipped test point"
> -} else {
> -    gdb_test_no_output "x/0 j"
> -}
> -if [istarget "hppa*-hp-hpux*"] {
> -    # on HP-UX you could access the first page without getting an error
> -    gdb_test "x/rx j" ".*(Cannot access|Error accessing) memory.*|.*0xa:\[ \t\]*\[0-9\]+.*"
> -}
> +gdb_test_no_output "x/0 j"
> +
> +gdb_test "x/rx j" ".*(Cannot access|Error accessing) memory.*|.*0xa:\[ \t\]*\[0-9\]+.*"

This last one used to only run on hpux.  It seems like it'll now fail
on is_address_zero_readable targets -- the non-error alternative assumes
the variable's value is "0xa" ?

You could guard it with "if [is_address_zero_readable]", but, that
proc does "x 0" internally, so it sounds like it'd be pointless?

>  gdb_test "print/0 j" ".*Item count other than 1 is meaningless.*" "print/0 j"
>  gdb_test "print/s sum" " = 1000" "ignored s"









> --- a/gdb/testsuite/gdb.dwarf2/pr10770.c
> +++ b/gdb/testsuite/gdb.dwarf2/pr10770.c
> @@ -4,7 +4,6 @@
>  /* HP-UX libunwind.so doesn't provide _UA_END_OF_STACK */
>  /* { dg-do run } */
>  /* { dg-options "-fexceptions" } */
> -/* { dg-skip-if "" { "ia64-*-hpux11.*" }  { "*" } { "" } } */
>  /* Verify DW_OP_* handling in the unwinder.  */
>  
>  #include <unwind.h>

You should remove the HP-UX reference at the top too then.
But, GDB doesn't use the dg-* markers.  Note the comment at the top:

/* This file comes from GCC.  If you are tempted to change it,
   consider also changing the copy there.  */

I'd either leave it be, or, remove the whole dg-* comment block.

Comparing our copy to the gcc copy, that's the only bit that diverged so far:

$ diff -up ~/gdb/mygit/src/gdb/testsuite/gdb.dwarf2/pr10770.c  src/gcc/testsuite/gcc.dg/cleanup-13.c

Thanks,
Pedro Alves

Comments

Simon Marchi Dec. 21, 2015, 4:57 p.m. UTC | #1 
On 21 December 2015 at 07:48, Pedro Alves <palves@redhat.com> wrote:
> I looked this one over too.  A few minor comments below, but
> otherwise looks good to me.  Thanks for doing this!
>
> On 12/19/2015 11:30 PM, Simon Marchi wrote:
>
>>       * gdb.multi/bkpt-multi-exec.ex: Likewise.p
>
> Typo: "ex: Likewise.p" -> "exp: Likewise."

Fixed.

>> +gdb_test_multiple "catch vfork" "$name" {
>> +    -re "Catchpoint \[0-9\]* .vfork..*$gdb_prompt $" {
>> +     pass $name
>> +    }
>> +    -re "Catch of vfork not yet implemented.*$gdb_prompt $" {
>
> This case can be removed.  GDB doesn't ever output this.

Actually, is it true for all "Catch of * not yet implemented" cases?

testsuite/gdb.base/break.exp
482:    -re "Catch of fork not yet implemented.*$gdb_prompt $" {
493:    -re "Catch of vfork not yet implemented.*$gdb_prompt $" {
503:    -re "Catch of exec not yet implemented.*$gdb_prompt $" {

testsuite/gdb.base/sepdebug.exp
291:    -re "Catch of fork not yet implemented.*$gdb_prompt $" {
302:    -re "Catch of vfork events not supported on HP-UX 10.20.*" {
308:    -re "Catch of vfork not yet implemented.*$gdb_prompt $" {
318:    -re "Catch of exec not yet implemented.*$gdb_prompt $" {

Oh damn, that just found another HP-UX reference.  I'll remove the
"Catch of vfork events not supported on HP-UX 10.20.*" as part of this
patch.

Grepping for "Catch of" in the source doesn't return anything, so I
guess they could all be removed from the testsuite. If that is right,
I think I would do it in a separate patch.

>> +     pass $name
>>      }
>>  }
>>
>
>
>
>> --- a/gdb/testsuite/gdb.base/display.exp
>> +++ b/gdb/testsuite/gdb.base/display.exp
>> @@ -194,15 +194,9 @@ gdb_test "print/r j" " = 0x0\[\\r\\n\]+" "debug test output 2a"
>>  gdb_test "print j"   " = 0\[\\r\\n\]+"   "debug test output 3"
>>
>>  # x/0 j doesn't produce any output and terminates PA64 process when testing
>> -if [istarget "hppa2.0w-hp-hpux11*"] {
>> -    xfail "'x/0 j' terminates PA64 process - skipped test point"
>> -} else {
>> -    gdb_test_no_output "x/0 j"
>> -}
>> -if [istarget "hppa*-hp-hpux*"] {
>> -    # on HP-UX you could access the first page without getting an error
>> -    gdb_test "x/rx j" ".*(Cannot access|Error accessing) memory.*|.*0xa:\[ \t\]*\[0-9\]+.*"
>> -}
>> +gdb_test_no_output "x/0 j"
>> +
>> +gdb_test "x/rx j" ".*(Cannot access|Error accessing) memory.*|.*0xa:\[ \t\]*\[0-9\]+.*"
>
> This last one used to only run on hpux.  It seems like it'll now fail
> on is_address_zero_readable targets -- the non-error alternative assumes
> the variable's value is "0xa" ?
>
> You could guard it with "if [is_address_zero_readable]", but, that
> proc does "x 0" internally, so it sounds like it'd be pointless?

Right it's pointless now, I'll just remove it.

>> --- a/gdb/testsuite/gdb.dwarf2/pr10770.c
>> +++ b/gdb/testsuite/gdb.dwarf2/pr10770.c
>> @@ -4,7 +4,6 @@
>>  /* HP-UX libunwind.so doesn't provide _UA_END_OF_STACK */
>>  /* { dg-do run } */
>>  /* { dg-options "-fexceptions" } */
>> -/* { dg-skip-if "" { "ia64-*-hpux11.*" }  { "*" } { "" } } */
>>  /* Verify DW_OP_* handling in the unwinder.  */
>>
>>  #include <unwind.h>
>
> You should remove the HP-UX reference at the top too then.
> But, GDB doesn't use the dg-* markers.  Note the comment at the top:
>
> /* This file comes from GCC.  If you are tempted to change it,
>    consider also changing the copy there.  */
>
> I'd either leave it be, or, remove the whole dg-* comment block.
>
> Comparing our copy to the gcc copy, that's the only bit that diverged so far:
>
> $ diff -up ~/gdb/mygit/src/gdb/testsuite/gdb.dwarf2/pr10770.c  src/gcc/testsuite/gcc.dg/cleanup-13.c
> --- /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.dwarf2/pr10770.c        2015-05-08 17:35:43.878768249 +0100
> +++ src/gcc/testsuite/gcc.dg/cleanup-13.c       2015-08-22 16:16:35.514064275 +0100
> @@ -1,10 +1,8 @@
> -/* This file comes from GCC.  If you are tempted to change it,
> -   consider also changing the copy there.  */
> -
>  /* HP-UX libunwind.so doesn't provide _UA_END_OF_STACK */
>  /* { dg-do run } */
>  /* { dg-options "-fexceptions" } */
>  /* { dg-skip-if "" { "ia64-*-hpux11.*" }  { "*" } { "" } } */
> +/* { dg-skip-if "" { ! nonlocal_goto } { "*" } { "" } } */
>  /* Verify DW_OP_* handling in the unwinder.  */
>
>  #include <unwind.h>

Ok, I'll leave the file as-is.

Another thing, the gdb.base/environ.exp is guarded by a

 23 if ![istarget "hppa*-*-hpux*"] then {
 24   return
 25 }

but it doesn't test hp-ux specific things.  It overlaps
gdb.base/testenv.exp in what it tests, but it does test a few more
things (like having an equal sign in the value when setting an env
var).  Removing the guard, it seems like the test runs fine on Linux
native.  It does not run fine with
native-gdbserver/native-extended-gdbserver, however.  So I could
replace it with the appropriate "if not remote" check.
gdb.base/testenv.exp uses "if { [is_remote target] }", but it's not
right, because it doesn't catch when running with
native-extended-gdbserver.

So for now I think I'll just leave it as-is, and we can merge the two
tests and clean this up after.
Pedro Alves Dec. 21, 2015, 5:07 p.m. UTC | #2 
On 12/21/2015 04:57 PM, Simon Marchi wrote:
> On 21 December 2015 at 07:48, Pedro Alves <palves@redhat.com> wrote:
>> I looked this one over too.  A few minor comments below, but
>> otherwise looks good to me.  Thanks for doing this!
>>
>> On 12/19/2015 11:30 PM, Simon Marchi wrote:
>>
>>>       * gdb.multi/bkpt-multi-exec.ex: Likewise.p
>>
>> Typo: "ex: Likewise.p" -> "exp: Likewise."
> 
> Fixed.
> 
>>> +gdb_test_multiple "catch vfork" "$name" {
>>> +    -re "Catchpoint \[0-9\]* .vfork..*$gdb_prompt $" {
>>> +     pass $name
>>> +    }
>>> +    -re "Catch of vfork not yet implemented.*$gdb_prompt $" {
>>
>> This case can be removed.  GDB doesn't ever output this.
> 
> Actually, is it true for all "Catch of * not yet implemented" cases?
> 

Yes.  I did a google search now for "Catch of fork not yet implemented"
and found this:

  https://www.sourceware.org/ml/gdb-patches/2004-01/msg00679.html

> testsuite/gdb.base/break.exp
> 482:    -re "Catch of fork not yet implemented.*$gdb_prompt $" {
> 493:    -re "Catch of vfork not yet implemented.*$gdb_prompt $" {
> 503:    -re "Catch of exec not yet implemented.*$gdb_prompt $" {
> 
> testsuite/gdb.base/sepdebug.exp
> 291:    -re "Catch of fork not yet implemented.*$gdb_prompt $" {
> 302:    -re "Catch of vfork events not supported on HP-UX 10.20.*" {
> 308:    -re "Catch of vfork not yet implemented.*$gdb_prompt $" {
> 318:    -re "Catch of exec not yet implemented.*$gdb_prompt $" {
> 
> Oh damn, that just found another HP-UX reference.  I'll remove the
> "Catch of vfork events not supported on HP-UX 10.20.*" as part of this
> patch.
> 
> Grepping for "Catch of" in the source doesn't return anything, so I
> guess they could all be removed from the testsuite. If that is right,
> I think I would do it in a separate patch.

That'd be great!

> 
> Another thing, the gdb.base/environ.exp is guarded by a
> 
>  23 if ![istarget "hppa*-*-hpux*"] then {
>  24   return
>  25 }
> 
> but it doesn't test hp-ux specific things.  

Right, that's old PR8595 - environ.exp could run on more platforms:
  https://sourceware.org/bugzilla/show_bug.cgi?id=8595

> It overlaps
> gdb.base/testenv.exp in what it tests, but it does test a few more
> things (like having an equal sign in the value when setting an env
> var).  Removing the guard, it seems like the test runs fine on Linux
> native.  It does not run fine with
> native-gdbserver/native-extended-gdbserver, however.  So I could
> replace it with the appropriate "if not remote" check.
> gdb.base/testenv.exp uses "if { [is_remote target] }", but it's not
> right, because it doesn't catch when running with
> native-extended-gdbserver.

Right.  I think most is_remote checks are wrong.  This is really
a protocol limitation, a bit orthogonal to protocol used or whether
the host and target machines are the same.   Probably the right
check is:

  [target_info gdb_protocol] == "remote" || [target_info gdb_protocol] == "extended-remote"

Better yet, add a new supports_target_env or some such to lib/gdb.exp
that encapsulates this.

> 
> So for now I think I'll just leave it as-is, and we can merge the two
> tests and clean this up after.
> 

That's fine.  It waited over 12 years already, it can wait a
little while longer.  :-)

Thanks,
Pedro Alves
Simon Marchi Dec. 21, 2015, 5:52 p.m. UTC | #3 
On 21 December 2015 at 12:07, Pedro Alves <palves@redhat.com> wrote:
> On 12/21/2015 04:57 PM, Simon Marchi wrote:
>> On 21 December 2015 at 07:48, Pedro Alves <palves@redhat.com> wrote:
>>> I looked this one over too.  A few minor comments below, but
>>> otherwise looks good to me.  Thanks for doing this!
>>>
>>> On 12/19/2015 11:30 PM, Simon Marchi wrote:
>>>
>>>>       * gdb.multi/bkpt-multi-exec.ex: Likewise.p
>>>
>>> Typo: "ex: Likewise.p" -> "exp: Likewise."
>>
>> Fixed.
>>
>>>> +gdb_test_multiple "catch vfork" "$name" {
>>>> +    -re "Catchpoint \[0-9\]* .vfork..*$gdb_prompt $" {
>>>> +     pass $name
>>>> +    }
>>>> +    -re "Catch of vfork not yet implemented.*$gdb_prompt $" {
>>>
>>> This case can be removed.  GDB doesn't ever output this.
>>
>> Actually, is it true for all "Catch of * not yet implemented" cases?
>>
>
> Yes.  I did a google search now for "Catch of fork not yet implemented"
> and found this:
>
>   https://www.sourceware.org/ml/gdb-patches/2004-01/msg00679.html
>
>> testsuite/gdb.base/break.exp
>> 482:    -re "Catch of fork not yet implemented.*$gdb_prompt $" {
>> 493:    -re "Catch of vfork not yet implemented.*$gdb_prompt $" {
>> 503:    -re "Catch of exec not yet implemented.*$gdb_prompt $" {
>>
>> testsuite/gdb.base/sepdebug.exp
>> 291:    -re "Catch of fork not yet implemented.*$gdb_prompt $" {
>> 302:    -re "Catch of vfork events not supported on HP-UX 10.20.*" {
>> 308:    -re "Catch of vfork not yet implemented.*$gdb_prompt $" {
>> 318:    -re "Catch of exec not yet implemented.*$gdb_prompt $" {
>>
>> Oh damn, that just found another HP-UX reference.  I'll remove the
>> "Catch of vfork events not supported on HP-UX 10.20.*" as part of this
>> patch.
>>
>> Grepping for "Catch of" in the source doesn't return anything, so I
>> guess they could all be removed from the testsuite. If that is right,
>> I think I would do it in a separate patch.
>
> That'd be great!
>
>>
>> Another thing, the gdb.base/environ.exp is guarded by a
>>
>>  23 if ![istarget "hppa*-*-hpux*"] then {
>>  24   return
>>  25 }
>>
>> but it doesn't test hp-ux specific things.
>
> Right, that's old PR8595 - environ.exp could run on more platforms:
>   https://sourceware.org/bugzilla/show_bug.cgi?id=8595
>
>> It overlaps
>> gdb.base/testenv.exp in what it tests, but it does test a few more
>> things (like having an equal sign in the value when setting an env
>> var).  Removing the guard, it seems like the test runs fine on Linux
>> native.  It does not run fine with
>> native-gdbserver/native-extended-gdbserver, however.  So I could
>> replace it with the appropriate "if not remote" check.
>> gdb.base/testenv.exp uses "if { [is_remote target] }", but it's not
>> right, because it doesn't catch when running with
>> native-extended-gdbserver.
>
> Right.  I think most is_remote checks are wrong.  This is really
> a protocol limitation, a bit orthogonal to protocol used or whether
> the host and target machines are the same.   Probably the right
> check is:
>
>   [target_info gdb_protocol] == "remote" || [target_info gdb_protocol] == "extended-remote"
>
> Better yet, add a new supports_target_env or some such to lib/gdb.exp
> that encapsulates this.
>
>>
>> So for now I think I'll just leave it as-is, and we can merge the two
>> tests and clean this up after.
>>
>
> That's fine.  It waited over 12 years already, it can wait a
> little while longer.  :-)
>
> Thanks,
> Pedro Alves


Ok, I pushed this one in.  Thanks!
diff mbox

Patch

--- /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.dwarf2/pr10770.c        2015-05-08 17:35:43.878768249 +0100
+++ src/gcc/testsuite/gcc.dg/cleanup-13.c       2015-08-22 16:16:35.514064275 +0100
@@ -1,10 +1,8 @@ 
-/* This file comes from GCC.  If you are tempted to change it,
-   consider also changing the copy there.  */
-
 /* HP-UX libunwind.so doesn't provide _UA_END_OF_STACK */
 /* { dg-do run } */
 /* { dg-options "-fexceptions" } */
 /* { dg-skip-if "" { "ia64-*-hpux11.*" }  { "*" } { "" } } */
+/* { dg-skip-if "" { ! nonlocal_goto } { "*" } { "" } } */
 /* Verify DW_OP_* handling in the unwinder.  */

 #include <unwind.h>