public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gdb fix for catch-syscall.exp
@ 2021-11-17 23:30 Carl Love
  2021-11-18 15:23 ` Tom Tromey
  2021-11-18 18:10 ` Simon Marchi
  0 siblings, 2 replies; 18+ messages in thread
From: Carl Love @ 2021-11-17 23:30 UTC (permalink / raw)
  To: gdb-patches; +Cc: cel, Will Schmidt, Rogerio Alves

GDB maintainers:

The following patch fixes three test failures and an expect error.  The
expect error ERROR: can't read "arch1": no such variable is the result
of the if/else statement in proc test_catch_syscall_multi_arch not
matching the power platform.  The power platform, starting with Power
8, has "le" in the platform string to indicate a Little Endian system. 
The current expect string doesn't match the "le" in the name.

The other three failures are the result of the execve instruction that
starts executing the new program.  The expect script is looking for the
return from the execve command which doesn't occur since execve
replaces the current process image.

The patch was tested on a Power 10 LE system with no regressions.

Please let me know if the patch is acceptable for mainline.  Thanks.

                       Carl 


-------------------------------------------------------------------
gdb fix for catch-syscall.exp

Remove check_continue "execve" from Proc test_catch_syscall_execve.

The check_continue proceedure checs that the command, execve, starts and
checks for the return from the command.  The execve command starts a new
program and thus the return from the command causing the test to fail.

The call to proc check_continue "execve" is removed and replaced with
just the call to check_call_to_syscall "execve" to verify the command
executed.  The next test in proc test_catch_syscall_execve verifies that
the new program started and hit the break point in main.

Update the check for the PowerPC architecture.  Power Little Endian systems
include "le" in the name.  The istarget "power64-*-linux*" check fails to
match LE sytems.  The expected string is updated to capture both Big Endian
and Little Endian systems.  Power 10 LE istarget prints as:
powerpc64le-unknown-linux-gnu.

This patch fixes three failures and the error:

    ERROR: can't read "arch1": no such variable

Patch tested on Power 10 ppc64le GNU/Linux platform.
---
 gdb/testsuite/gdb.base/catch-syscall.exp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
index 811a92b0aea..8b496712df5 100644
--- a/gdb/testsuite/gdb.base/catch-syscall.exp
+++ b/gdb/testsuite/gdb.base/catch-syscall.exp
@@ -348,7 +348,9 @@ proc test_catch_syscall_execve {} {
 	# Check for entry/return across the execve, making sure that the
 	# syscall_state isn't lost when turning into a new process.
 	insert_catch_syscall_with_arg "execve"
-	check_continue "execve"
+
+	# check that the execve is called
+	check_call_to_syscall "execve"
 
 	# Continue to main so extended-remote can read files as needed.
 	# (Otherwise that "Reading" output confuses gdb_continue_to_end.)
@@ -550,7 +552,7 @@ proc test_catch_syscall_multi_arch {} {
 	set syscall2_name "write"
 	set syscall_number 1
     } elseif { [istarget "powerpc-*-linux*"] \
-		   || [istarget "powerpc64-*-linux*"] } {
+		   || [istarget "powerpc64*-linux*"] } {
 	set arch1 "powerpc:common"
 	set arch2 "powerpc:common64"
 	set syscall1_name "openat"
-- 
2.30.2



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gdb fix for catch-syscall.exp
  2021-11-17 23:30 [PATCH] gdb fix for catch-syscall.exp Carl Love
@ 2021-11-18 15:23 ` Tom Tromey
  2021-11-18 18:10 ` Simon Marchi
  1 sibling, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2021-11-18 15:23 UTC (permalink / raw)
  To: Carl Love via Gdb-patches; +Cc: Carl Love, Rogerio Alves

>>>>> "Carl" == Carl Love via Gdb-patches <gdb-patches@sourceware.org> writes:

Carl> +	# check that the execve is called
Carl> +	check_call_to_syscall "execve"

In gdb, normally a comment should start with a capital letter and end
with a ".".  This patch is ok with this tweak.  Thank you.

Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gdb fix for catch-syscall.exp
  2021-11-17 23:30 [PATCH] gdb fix for catch-syscall.exp Carl Love
  2021-11-18 15:23 ` Tom Tromey
@ 2021-11-18 18:10 ` Simon Marchi
  2021-11-20  0:27   ` Carl Love
  1 sibling, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2021-11-18 18:10 UTC (permalink / raw)
  To: Carl Love, gdb-patches; +Cc: Rogerio Alves

On 2021-11-17 6:30 p.m., Carl Love via Gdb-patches wrote:
> GDB maintainers:
> 
> The following patch fixes three test failures and an expect error.  The
> expect error ERROR: can't read "arch1": no such variable is the result
> of the if/else statement in proc test_catch_syscall_multi_arch not
> matching the power platform.  The power platform, starting with Power
> 8, has "le" in the platform string to indicate a Little Endian system. 
> The current expect string doesn't match the "le" in the name.
> 
> The other three failures are the result of the execve instruction that
> starts executing the new program.  The expect script is looking for the
> return from the execve command which doesn't occur since execve
> replaces the current process image.
> 
> The patch was tested on a Power 10 LE system with no regressions.
> 
> Please let me know if the patch is acceptable for mainline.  Thanks.
> 
>                        Carl 
> 
> 
> -------------------------------------------------------------------
> gdb fix for catch-syscall.exp
> 
> Remove check_continue "execve" from Proc test_catch_syscall_execve.
> 
> The check_continue proceedure checs that the command, execve, starts and
> checks for the return from the command.  The execve command starts a new
> program and thus the return from the command causing the test to fail.
> 
> The call to proc check_continue "execve" is removed and replaced with
> just the call to check_call_to_syscall "execve" to verify the command
> executed.  The next test in proc test_catch_syscall_execve verifies that
> the new program started and hit the break point in main.
> 
> Update the check for the PowerPC architecture.  Power Little Endian systems
> include "le" in the name.  The istarget "power64-*-linux*" check fails to
> match LE sytems.  The expected string is updated to capture both Big Endian
> and Little Endian systems.  Power 10 LE istarget prints as:
> powerpc64le-unknown-linux-gnu.
> 
> This patch fixes three failures and the error:
> 
>     ERROR: can't read "arch1": no such variable
> 
> Patch tested on Power 10 ppc64le GNU/Linux platform.
> ---
>  gdb/testsuite/gdb.base/catch-syscall.exp | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
> index 811a92b0aea..8b496712df5 100644
> --- a/gdb/testsuite/gdb.base/catch-syscall.exp
> +++ b/gdb/testsuite/gdb.base/catch-syscall.exp
> @@ -348,7 +348,9 @@ proc test_catch_syscall_execve {} {
>  	# Check for entry/return across the execve, making sure that the
>  	# syscall_state isn't lost when turning into a new process.
>  	insert_catch_syscall_with_arg "execve"
> -	check_continue "execve"
> +
> +	# check that the execve is called
> +	check_call_to_syscall "execve"
>  
>  	# Continue to main so extended-remote can read files as needed.
>  	# (Otherwise that "Reading" output confuses gdb_continue_to_end.)
> @@ -550,7 +552,7 @@ proc test_catch_syscall_multi_arch {} {
>  	set syscall2_name "write"
>  	set syscall_number 1
>      } elseif { [istarget "powerpc-*-linux*"] \
> -		   || [istarget "powerpc64-*-linux*"] } {
> +		   || [istarget "powerpc64*-linux*"] } {
>  	set arch1 "powerpc:common"
>  	set arch2 "powerpc:common64"
>  	set syscall1_name "openat"
> -- 
> 2.30.2
> 
> 

Hi Carl,

This patch causes this failure regression on Ubuntu 20.04, amd64:

FAIL: gdb.base/catch-syscall.exp: execve: continue to main

From gdb.log:

 615 continue^M
 616 Continuing.^M
 617 process 2022222 is executing new program: /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/catch-syscall/catch-syscall^M
 618 ^M
 619 Catchpoint 18 (returned from syscall execve), 0x00007ffff7fd0100 in _start () from /lib64/ld-linux-x86-64.so.2^M
 620 (gdb) FAIL: gdb.base/catch-syscall.exp: execve: continue to main

Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH] gdb fix for catch-syscall.exp
  2021-11-18 18:10 ` Simon Marchi
@ 2021-11-20  0:27   ` Carl Love
  2021-11-22  1:07     ` Simon Marchi
  2021-11-22 17:01     ` John Baldwin
  0 siblings, 2 replies; 18+ messages in thread
From: Carl Love @ 2021-11-20  0:27 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches; +Cc: Rogerio Alves

Simon:

On Thu, 2021-11-18 at 13:10 -0500, Simon Marchi wrote:
> 
> Hi Carl,
> 
> This patch causes this failure regression on Ubuntu 20.04, amd64:
> 
> FAIL: gdb.base/catch-syscall.exp: execve: continue to main
> 
> From gdb.log:
> 
>  615 continue^M
>  616 Continuing.^M
>  617 process 2022222 is executing new program:
> /home/smarchi/build/binutils-
> gdb/gdb/testsuite/outputs/gdb.base/catch-syscall/catch-syscall^M
>  618 ^M
>  619 Catchpoint 18 (returned from syscall execve), 0x00007ffff7fd0100
> in _start () from /lib64/ld-linux-x86-64.so.2^M
>  620 (gdb) FAIL: gdb.base/catch-syscall.exp: execve: continue to main
> 

Interesting.  I understood that the execve call would not return to the
caller except in the case of an error.  Not sure why it behaves
differently on amd64 versus Power and Intel?  Let me take a look and
see if I can create a fix with a test multiple to cover both cases.

Thanks for letting me know.

                   Carl 


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gdb fix for catch-syscall.exp
  2021-11-20  0:27   ` Carl Love
@ 2021-11-22  1:07     ` Simon Marchi
  2021-11-22 18:16       ` Carl Love
  2021-11-22 17:01     ` John Baldwin
  1 sibling, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2021-11-22  1:07 UTC (permalink / raw)
  To: Carl Love, Simon Marchi, gdb-patches; +Cc: Rogerio Alves



On 2021-11-19 19:27, Carl Love wrote:
> Simon:
> 
> On Thu, 2021-11-18 at 13:10 -0500, Simon Marchi wrote:
>>
>> Hi Carl,
>>
>> This patch causes this failure regression on Ubuntu 20.04, amd64:
>>
>> FAIL: gdb.base/catch-syscall.exp: execve: continue to main
>>
>> From gdb.log:
>>
>>  615 continue^M
>>  616 Continuing.^M
>>  617 process 2022222 is executing new program:
>> /home/smarchi/build/binutils-
>> gdb/gdb/testsuite/outputs/gdb.base/catch-syscall/catch-syscall^M
>>  618 ^M
>>  619 Catchpoint 18 (returned from syscall execve), 0x00007ffff7fd0100
>> in _start () from /lib64/ld-linux-x86-64.so.2^M
>>  620 (gdb) FAIL: gdb.base/catch-syscall.exp: execve: continue to main
>>
> 
> Interesting.  I understood that the execve call would not return to the
> caller except in the case of an error.  Not sure why it behaves
> differently on amd64 versus Power and Intel?  Let me take a look and
> see if I can create a fix with a test multiple to cover both cases.
> 
> Thanks for letting me know.
> 
>                    Carl 
> 

Did you manage to reproduce, or do you need more data from me?

Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gdb fix for catch-syscall.exp
  2021-11-20  0:27   ` Carl Love
  2021-11-22  1:07     ` Simon Marchi
@ 2021-11-22 17:01     ` John Baldwin
  2021-11-23 20:34       ` Simon Marchi
  1 sibling, 1 reply; 18+ messages in thread
From: John Baldwin @ 2021-11-22 17:01 UTC (permalink / raw)
  To: Carl Love, Simon Marchi, gdb-patches; +Cc: Rogerio Alves

On 11/19/21 4:27 PM, Carl Love via Gdb-patches wrote:
> Simon:
> 
> On Thu, 2021-11-18 at 13:10 -0500, Simon Marchi wrote:
>>
>> Hi Carl,
>>
>> This patch causes this failure regression on Ubuntu 20.04, amd64:
>>
>> FAIL: gdb.base/catch-syscall.exp: execve: continue to main
>>
>>  From gdb.log:
>>
>>   615 continue^M
>>   616 Continuing.^M
>>   617 process 2022222 is executing new program:
>> /home/smarchi/build/binutils-
>> gdb/gdb/testsuite/outputs/gdb.base/catch-syscall/catch-syscall^M
>>   618 ^M
>>   619 Catchpoint 18 (returned from syscall execve), 0x00007ffff7fd0100
>> in _start () from /lib64/ld-linux-x86-64.so.2^M
>>   620 (gdb) FAIL: gdb.base/catch-syscall.exp: execve: continue to main
>>
> 
> Interesting.  I understood that the execve call would not return to the
> caller except in the case of an error.  Not sure why it behaves
> differently on amd64 versus Power and Intel?  Let me take a look and
> see if I can create a fix with a test multiple to cover both cases.
> 
> Thanks for letting me know.

Note that the thread/process that calls execve() does still "return"
from the kernel, it just returns to the entry point of the executable
with the contents of the user address space wiped.  I'm not quite sure
how Linux treats this, but on FreeBSD at least this "return" is treated
as a successful return from the execve system call.  In fact, I think
Linux on at least amd64 reports two events: one for an "execve" event and
one for the system call return event judging by a commit made in FreeBSD
to emulate ptrace() for Linux binaries:

https://cgit.freebsd.org/src/commit/?id=6e66030c4c05331f9b0adf87c31f2f233dd3ae1f

-- 
John Baldwin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH] gdb fix for catch-syscall.exp
  2021-11-22  1:07     ` Simon Marchi
@ 2021-11-22 18:16       ` Carl Love
  0 siblings, 0 replies; 18+ messages in thread
From: Carl Love @ 2021-11-22 18:16 UTC (permalink / raw)
  To: Simon Marchi, Simon Marchi, gdb-patches; +Cc: Rogerio Alves

Simon:

On Sun, 2021-11-21 at 20:07 -0500, Simon Marchi wrote:
>          Carl 
> > 
> 
> Did you manage to reproduce, or do you need more data from me?
> 
I can't reproduce what you saw.  But I am hoping to put together a
patch today that will use the multiple test to look for the response as
seen on amd64 but also handle not seeing the response as seen on Power.
The plan will be then for you and I to test the patch to make sure it
works on both.  Thanks.

           Carl 


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gdb fix for catch-syscall.exp
  2021-11-22 17:01     ` John Baldwin
@ 2021-11-23 20:34       ` Simon Marchi
  2021-11-23 22:34         ` John Baldwin
  2021-11-24  1:15         ` Carl Love
  0 siblings, 2 replies; 18+ messages in thread
From: Simon Marchi @ 2021-11-23 20:34 UTC (permalink / raw)
  To: John Baldwin, Carl Love, gdb-patches; +Cc: Rogerio Alves

> Note that the thread/process that calls execve() does still "return"
> from the kernel, it just returns to the entry point of the executable
> with the contents of the user address space wiped.  I'm not quite sure
> how Linux treats this, but on FreeBSD at least this "return" is treated
> as a successful return from the execve system call.  In fact, I think
> Linux on at least amd64 reports two events: one for an "execve" event and
> one for the system call return event judging by a commit made in FreeBSD
> to emulate ptrace() for Linux binaries:
>
> https://cgit.freebsd.org/src/commit/?id=6e66030c4c05331f9b0adf87c31f2f233dd3ae1f

Yes, that's what I see on Linux too.  With an x86-64 binary:


    $ ./gdb -q --data-directory=data-directory m64
    Reading symbols from m64...
    (gdb) catch syscall execve
    Catchpoint 1 (syscall 'execve' [59])
    (gdb) r
    Starting program: /home/smarchi/build/binutils-gdb-all-targets/gdb/m64

    Catchpoint 1 (call to syscall execve), 0x00007ffff7ea92fb in execve () at ../sysdeps/unix/syscall-template.S:78
    78      ../sysdeps/unix/syscall-template.S: No such file or directory.
    (gdb) c
    Continuing.
    process 3592054 is executing new program: /home/smarchi/build/binutils-gdb-all-targets/gdb/m64

    Catchpoint 1 (returned from syscall execve), 0x00007ffff7fd0100 in _start () from /lib64/ld-linux-x86-64.so.2

and an x86 binary (on an actual x86 kernel, not on top of an x86-64
kernel):


    $ ./gdb -q -nx --data-directory=data-directory ./a.out
    Reading symbols from ./a.out...
    (gdb) catch syscall execve
    Catchpoint 1 (syscall 'execve' [11])
    (gdb) r
    Starting program: /home/simark/build/binutils-gdb/gdb/a.out

    Catchpoint 1 (call to syscall execve), 0xb7fddcb0 in __kernel_vsyscall ()
    (gdb) c
    Continuing.
    process 3734 is executing new program: /home/simark/build/binutils-gdb/gdb/a.out

    Catchpoint 1 (returned from syscall execve), 0xb7fdec60 in ?? () from /lib/ld-linux.so.2

In both cases, we see the execve syscall return at the entry point of
the dynamic linker/loader.  I tested on aarch64 too, same thing.

I indeed see that on powerpc we don't get the return:

    $ ./gdb -q -nx --data-directory=data-directory ./a.out
    Reading symbols from ./a.out...
    (gdb) catch syscall execve
    Catchpoint 1 (syscall 'execve' [11])
    (gdb) r
    Starting program: /home/simark/build/binutils-gdb/gdb/a.out

    Catchpoint 1 (call to syscall execve), 0x00007ffff7e6f18c in execve () from /lib64/libc.so.6
    (gdb) c
    Continuing.
    process 98693 is executing new program: /home/simark/build/binutils-gdb/gdb/a.out

    Catchpoint 1 (call to syscall execve), 0x00007ffff7e6f18c in execve () from /lib64/libc.so.6

The behavior on powerpc sounds like a bug to me, and adjusting the test
like this patch did just covers it up.  It doesn't make sense for that
behavior to be different per arch, for the same OS.

If you all agree that it's a bug, I would suggest reverting this patch
and making a patch that kfails the test when on powerpc.  And ideally,
someone should dig to understand why we don't see the return on powerpc
(and fix it), but I'm not here to tell what other people should work on
:).

Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gdb fix for catch-syscall.exp
  2021-11-23 20:34       ` Simon Marchi
@ 2021-11-23 22:34         ` John Baldwin
  2021-11-24 17:46           ` Carl Love
  2021-11-24  1:15         ` Carl Love
  1 sibling, 1 reply; 18+ messages in thread
From: John Baldwin @ 2021-11-23 22:34 UTC (permalink / raw)
  To: Simon Marchi, Carl Love, gdb-patches; +Cc: Rogerio Alves

On 11/23/21 12:34 PM, Simon Marchi wrote:
>> Note that the thread/process that calls execve() does still "return"
>> from the kernel, it just returns to the entry point of the executable
>> with the contents of the user address space wiped.  I'm not quite sure
>> how Linux treats this, but on FreeBSD at least this "return" is treated
>> as a successful return from the execve system call.  In fact, I think
>> Linux on at least amd64 reports two events: one for an "execve" event and
>> one for the system call return event judging by a commit made in FreeBSD
>> to emulate ptrace() for Linux binaries:
>>
>> https://cgit.freebsd.org/src/commit/?id=6e66030c4c05331f9b0adf87c31f2f233dd3ae1f
> 
> Yes, that's what I see on Linux too.  With an x86-64 binary:
> 
> 
>      $ ./gdb -q --data-directory=data-directory m64
>      Reading symbols from m64...
>      (gdb) catch syscall execve
>      Catchpoint 1 (syscall 'execve' [59])
>      (gdb) r
>      Starting program: /home/smarchi/build/binutils-gdb-all-targets/gdb/m64
> 
>      Catchpoint 1 (call to syscall execve), 0x00007ffff7ea92fb in execve () at ../sysdeps/unix/syscall-template.S:78
>      78      ../sysdeps/unix/syscall-template.S: No such file or directory.
>      (gdb) c
>      Continuing.
>      process 3592054 is executing new program: /home/smarchi/build/binutils-gdb-all-targets/gdb/m64
> 
>      Catchpoint 1 (returned from syscall execve), 0x00007ffff7fd0100 in _start () from /lib64/ld-linux-x86-64.so.2
> 
> and an x86 binary (on an actual x86 kernel, not on top of an x86-64
> kernel):
> 
> 
>      $ ./gdb -q -nx --data-directory=data-directory ./a.out
>      Reading symbols from ./a.out...
>      (gdb) catch syscall execve
>      Catchpoint 1 (syscall 'execve' [11])
>      (gdb) r
>      Starting program: /home/simark/build/binutils-gdb/gdb/a.out
> 
>      Catchpoint 1 (call to syscall execve), 0xb7fddcb0 in __kernel_vsyscall ()
>      (gdb) c
>      Continuing.
>      process 3734 is executing new program: /home/simark/build/binutils-gdb/gdb/a.out
> 
>      Catchpoint 1 (returned from syscall execve), 0xb7fdec60 in ?? () from /lib/ld-linux.so.2
> 
> In both cases, we see the execve syscall return at the entry point of
> the dynamic linker/loader.  I tested on aarch64 too, same thing.
> 
> I indeed see that on powerpc we don't get the return:
> 
>      $ ./gdb -q -nx --data-directory=data-directory ./a.out
>      Reading symbols from ./a.out...
>      (gdb) catch syscall execve
>      Catchpoint 1 (syscall 'execve' [11])
>      (gdb) r
>      Starting program: /home/simark/build/binutils-gdb/gdb/a.out
> 
>      Catchpoint 1 (call to syscall execve), 0x00007ffff7e6f18c in execve () from /lib64/libc.so.6
>      (gdb) c
>      Continuing.
>      process 98693 is executing new program: /home/simark/build/binutils-gdb/gdb/a.out
> 
>      Catchpoint 1 (call to syscall execve), 0x00007ffff7e6f18c in execve () from /lib64/libc.so.6
> 
> The behavior on powerpc sounds like a bug to me, and adjusting the test
> like this patch did just covers it up.  It doesn't make sense for that
> behavior to be different per arch, for the same OS.
> 
> If you all agree that it's a bug, I would suggest reverting this patch
> and making a patch that kfails the test when on powerpc.  And ideally,
> someone should dig to understand why we don't see the return on powerpc
> (and fix it), but I'm not here to tell what other people should work on
> :).

I do think this is likely a kernel bug.  In FreeBSD's case we report a single
event for both the syscall exit and exec event (but set flags to indicate that
both events are present.. in practice for GDB I think this means that on
FreeBSD we only report the exec event.  Not sure if it would be more correct
to report two events to the core in that case.)  I do think that a given OS
should probably be consistent here across architectures.

-- 
John Baldwin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH] gdb fix for catch-syscall.exp
  2021-11-23 20:34       ` Simon Marchi
  2021-11-23 22:34         ` John Baldwin
@ 2021-11-24  1:15         ` Carl Love
  2021-11-24 19:29           ` Simon Marchi
  1 sibling, 1 reply; 18+ messages in thread
From: Carl Love @ 2021-11-24  1:15 UTC (permalink / raw)
  To: Simon Marchi, John Baldwin, gdb-patches; +Cc: Rogerio Alves, cel

Simon:

On Tue, 2021-11-23 at 15:34 -0500, Simon Marchi wrote:
> If you all agree that it's a bug, I would suggest reverting this
> patch
> and making a patch that kfails the test when on powerpc.  And
> ideally,
> someone should dig to understand why we don't see the return on
> powerpc
> (and fix it), but I'm not here to tell what other people should work
> on :).

OK, I think for powerpc the test should be a xfail.  Looking at the
README, kfail is for gdb known issues, xfail is for issues in the
environment including the OS.

I see xfail takes two arguments, the first one is the gdb bug number. 
So, I will need to file a bug before I finish this patch.

I am hoping someone has a suggestion on how to improve my patch to add
the xfail.  The issue is since Powerpc doesn't print the "Catchpoint 1
(returned from syscall execve)" but rather stops at main, the test
should not issue the continue command after the xfail.  In the case of
the xfail, I need to pass 0 backup so the if statement in proc
test_catch_syscall_execve can decide if the continue command should be
issued or not.  The change is a bit messy having to pass the return
value up.  Wondering if anyone has a better idea how to add the xfail?

Note, so far I have only tested this on Powerpc.  Patch is not ready
for committing.

                   Carl 


-------------------------------------------------
gdb Revert change to gdb.base/catch-syscall.exp

The previous commit:
   commit ab198279120fe7937c0970a8bb881922726678f9
   Author: Carl Love <cel@us.ibm.com>
   Date:   Wed Nov 17 22:29:33 2021 +0000

       gdb fix for catch-syscall.exp

Fixed the test failure on PowerPC but broke the test on amd64.  The issue is
both messages:

    Catchpoint 1 (call to syscall execve), ....
    Catchpoint 1 (returned from syscall execve),  ....

are seen on amd64 whereas on PowerPC only the "call to syscall execve" is
seen.  Based on the discussion on the mailing list, it is felt that this
is really an issue with the PowerPC kernel support not reporting both events.
Thus, the change is being reverted and will be marked as a kfail for Powerpc.
---
 gdb/testsuite/gdb.base/catch-syscall.exp | 38 +++++++++++++++++++-----
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
index 016d0a698a6..04fa2bf9687 100644
--- a/gdb/testsuite/gdb.base/catch-syscall.exp
+++ b/gdb/testsuite/gdb.base/catch-syscall.exp
@@ -127,7 +127,24 @@ proc check_return_from_syscall { syscall { pattern "" } } {
     }
 
     set thistest "syscall $syscall has returned"
-    gdb_test "continue" "Catchpoint $decimal \\(returned from syscall ${pattern}\\).*" $thistest
+    if { $pattern eq "execve" } {
+	gdb_test_multiple "continue" $thistest {
+	    -re -wrap "Catchpoint $decimal \\(returned from syscall ${pattern}\\).*" {
+		pass $thistest
+		return 1
+	    }
+	    -re -wrap ".*Breakpoint $decimal, main .*" {
+		# On Powerpc kernel does not report the returned from syscall
+		# as expected by the test.
+		xfail $thistest
+		return 0
+	    }
+	}
+
+    } else {
+	gdb_test "continue" "Catchpoint $decimal \\(returned from syscall ${pattern}\\).*" $thistest
+	return 1
+    }
 }
 
 # Internal procedure that performs two 'continue' commands and checks if
@@ -142,7 +159,11 @@ proc check_continue { syscall { pattern "" } } {
     # Testing if the inferior has called the syscall.
     check_call_to_syscall $syscall $pattern
     # And now, that the syscall has returned.
-    check_return_from_syscall $syscall $pattern
+    if [check_return_from_syscall $syscall $pattern] {
+	return 1
+    } else {
+	return 0
+    }
 }
 
 # Inserts a syscall catchpoint with an argument.
@@ -348,13 +369,14 @@ proc test_catch_syscall_execve {} {
 	# Check for entry/return across the execve, making sure that the
 	# syscall_state isn't lost when turning into a new process.
 	insert_catch_syscall_with_arg "execve"
+	if [check_continue "execve"] {
+	    # The check_continue test generates an XFAIL on Powerpc.  In
+	    # that case, gdb is already at main so don't do the continue.
 
-	# Check that the execve is called.
-	check_call_to_syscall "execve"
-
-	# Continue to main so extended-remote can read files as needed.
-	# (Otherwise that "Reading" output confuses gdb_continue_to_end.)
-	gdb_continue "main"
+	    # Continue to main so extended-remote can read files as needed.
+	    # (Otherwise that "Reading" output confuses gdb_continue_to_end.)
+	    gdb_continue "main"
+	}
 
 	# Now can we finish?
 	check_for_program_end
-- 
2.30.2



^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH] gdb fix for catch-syscall.exp
  2021-11-23 22:34         ` John Baldwin
@ 2021-11-24 17:46           ` Carl Love
  2021-11-24 17:51             ` John Baldwin
  0 siblings, 1 reply; 18+ messages in thread
From: Carl Love @ 2021-11-24 17:46 UTC (permalink / raw)
  To: John Baldwin, Simon Marchi, gdb-patches; +Cc: Rogerio Alves

On Tue, 2021-11-23 at 14:34 -0800, John Baldwin wrote:
> > > 

<snip>

> On 11/23/21 12:34 PM, Simon Marchi wrote:
> > 
> > The behavior on powerpc sounds like a bug to me, and adjusting the
> > test
> > like this patch did just covers it up.  It doesn't make sense for
> > that
> > behavior to be different per arch, for the same OS.
> > 
> > If you all agree that it's a bug, I would suggest reverting this
> > patch
> > and making a patch that kfails the test when on powerpc.  And
> > ideally,
> > someone should dig to understand why we don't see the return on
> > powerpc
> > (and fix it), but I'm not here to tell what other people should
> > work on
> > :).
> 
> I do think this is likely a kernel bug.  In FreeBSD's case we report
> a single
> event for both the syscall exit and exec event (but set flags to
> indicate that
> both events are present.. in practice for GDB I think this means that
> on
> FreeBSD we only report the exec event.  Not sure if it would be more
> correct
> to report two events to the core in that case.)  I do think that a
> given OS
> should probably be consistent here across architectures.
> 

So I sent out a preliminary patch for comments to revert the change and
make it an xfail.  

I was looking to see where I should file a bug.  The discussion has
said it is a kernel bug.  The Freebsd commit you pointed to is a distro
fix from what I can see?  So, is it really a linux kernel issue or
something that the distros handle?  I believe there is a ptrace code in
glib see that interacts with the result from the kernel, is that where
the fix should go to fix all arch?

Anyway, it isn't clear to me at the moment which bugzilla a bug should
be filed with.  Thoughts?

                        Carl


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gdb fix for catch-syscall.exp
  2021-11-24 17:46           ` Carl Love
@ 2021-11-24 17:51             ` John Baldwin
  0 siblings, 0 replies; 18+ messages in thread
From: John Baldwin @ 2021-11-24 17:51 UTC (permalink / raw)
  To: Carl Love, Simon Marchi, gdb-patches; +Cc: Rogerio Alves

On 11/24/21 9:46 AM, Carl Love wrote:
> On Tue, 2021-11-23 at 14:34 -0800, John Baldwin wrote:
>>>>
> 
> <snip>
> 
>> On 11/23/21 12:34 PM, Simon Marchi wrote:
>>>
>>> The behavior on powerpc sounds like a bug to me, and adjusting the
>>> test
>>> like this patch did just covers it up.  It doesn't make sense for
>>> that
>>> behavior to be different per arch, for the same OS.
>>>
>>> If you all agree that it's a bug, I would suggest reverting this
>>> patch
>>> and making a patch that kfails the test when on powerpc.  And
>>> ideally,
>>> someone should dig to understand why we don't see the return on
>>> powerpc
>>> (and fix it), but I'm not here to tell what other people should
>>> work on
>>> :).
>>
>> I do think this is likely a kernel bug.  In FreeBSD's case we report
>> a single
>> event for both the syscall exit and exec event (but set flags to
>> indicate that
>> both events are present.. in practice for GDB I think this means that
>> on
>> FreeBSD we only report the exec event.  Not sure if it would be more
>> correct
>> to report two events to the core in that case.)  I do think that a
>> given OS
>> should probably be consistent here across architectures.
>>
> 
> So I sent out a preliminary patch for comments to revert the change and
> make it an xfail.

For GDB I think you have to make it an xfail for powerpc.

> I was looking to see where I should file a bug.  The discussion has
> said it is a kernel bug.  The Freebsd commit you pointed to is a distro
> fix from what I can see?  So, is it really a linux kernel issue or
> something that the distros handle?  I believe there is a ptrace code in
> glib see that interacts with the result from the kernel, is that where
> the fix should go to fix all arch?

The FreeBSD commit isn't for a distro, it's for a different OS kernel
that supports (some) Linux binaries as an alternate userspace ABI.  For
the kernel bug you would need to file that with the Linux kernel maintainers
following the directions at
https://www.kernel.org/doc/html/latest/admin-guide/reporting-issues.html

-- 
John Baldwin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gdb fix for catch-syscall.exp
  2021-11-24  1:15         ` Carl Love
@ 2021-11-24 19:29           ` Simon Marchi
  2021-11-29 16:46             ` Carl Love
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2021-11-24 19:29 UTC (permalink / raw)
  To: Carl Love, John Baldwin, gdb-patches; +Cc: Rogerio Alves

On 2021-11-23 8:15 p.m., Carl Love wrote:
> Simon:
>
> On Tue, 2021-11-23 at 15:34 -0500, Simon Marchi wrote:
>> If you all agree that it's a bug, I would suggest reverting this
>> patch
>> and making a patch that kfails the test when on powerpc.  And
>> ideally,
>> someone should dig to understand why we don't see the return on
>> powerpc
>> (and fix it), but I'm not here to tell what other people should work
>> on :).
>
> OK, I think for powerpc the test should be a xfail.  Looking at the
> README, kfail is for gdb known issues, xfail is for issues in the
> environment including the OS.

If we can already determine that it's the kernel fault, then xfail is
appropriate.  But I haven't actually dug to find out who is at fault.

> I see xfail takes two arguments, the first one is the gdb bug number.
> So, I will need to file a bug before I finish this patch.

I just filed it here: https://sourceware.org/bugzilla/show_bug.cgi?id=28623

> I am hoping someone has a suggestion on how to improve my patch to add
> the xfail.  The issue is since Powerpc doesn't print the "Catchpoint 1
> (returned from syscall execve)" but rather stops at main, the test
> should not issue the continue command after the xfail.  In the case of
> the xfail, I need to pass 0 backup so the if statement in proc
> test_catch_syscall_execve can decide if the continue command should be
> issued or not.  The change is a bit messy having to pass the return
> value up.  Wondering if anyone has a better idea how to add the xfail?
>
> Note, so far I have only tested this on Powerpc.  Patch is not ready
> for committing.

I try to look at this later.  For now I will push a revert patch, since
we agree on the fact that this is a bug (and the test should not expect
the buggy behavior).

Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH] gdb fix for catch-syscall.exp
  2021-11-24 19:29           ` Simon Marchi
@ 2021-11-29 16:46             ` Carl Love
  2021-12-02 16:32               ` Ping " Carl Love
  2021-12-10 18:36               ` Simon Marchi
  0 siblings, 2 replies; 18+ messages in thread
From: Carl Love @ 2021-11-29 16:46 UTC (permalink / raw)
  To: Simon Marchi, John Baldwin, gdb-patches; +Cc: Rogerio Alves, cel

Simon:

I split the latest patch so there is a separate revert patch and a
patch to make the Powerepc test a xfail.  Note, the revert patch only
reverts the part of the original patch that causes the regression test
on amd64.  

Let me know if the first patch looks OK to commit to mainline.

We can continue to discuss the second patch to decide if there is a
better implementation to add the xfail and if it really is a kernel
failure.

                     Carl 
--------------------------------------------------------------------
[PATCH 1/2] gdb: Revert change to gdb.base/catch-syscall.exp

The previous commit:
   commit ab198279120fe7937c0970a8bb881922726678f9
   Author: Carl Love <cel@us.ibm.com>
   Date:   Wed Nov 17 22:29:33 2021 +0000

       gdb fix for catch-syscall.exp

Fixed the test failure on PowerPC but broke the test on amd64.  The issue is
both messages:

    Catchpoint 1 (call to syscall execve), ....
    Catchpoint 1 (returned from syscall execve),  ....

are seen on amd64 whereas on PowerPC only the "call to syscall execve" is
seen.  Based on the discussion on the mailing list, it is felt that this
is really an issue with the PowerPC kernel support not reporting both events.

This patch just reverts the part of the commit that caused the regression
failure on amd64.  The change for the istarget powerpc64-*-linux* is not
being reverted.
---
 gdb/testsuite/gdb.base/catch-syscall.exp | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
index 016d0a698a6..cdd5e2aec47 100644
--- a/gdb/testsuite/gdb.base/catch-syscall.exp
+++ b/gdb/testsuite/gdb.base/catch-syscall.exp
@@ -348,9 +348,7 @@ proc test_catch_syscall_execve {} {
 	# Check for entry/return across the execve, making sure that the
 	# syscall_state isn't lost when turning into a new process.
 	insert_catch_syscall_with_arg "execve"
-
-	# Check that the execve is called.
-	check_call_to_syscall "execve"
+	check_continue "execve"
 
 	# Continue to main so extended-remote can read files as needed.
 	# (Otherwise that "Reading" output confuses gdb_continue_to_end.)
-- 
2.30.2

-----------------------------------------------------------------------
[PATCH 2/2] gdb: Powerpc mark xfail in gdb.base/catch-syscall.exp

Powerpc is not reporting the

  Catchpoint 1 (returned from syscall execve),  ....

as expected.  The issue appears to be with the kernel not returning the
expected result.  This patch marks the test failure as an xfail.

See gdb bugzilla https://sourceware.org/bugzilla/show_bug.cgi?id=28623
---
 gdb/testsuite/gdb.base/catch-syscall.exp | 37 ++++++++++++++++++++----
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
index cdd5e2aec47..0693f7afe27 100644
--- a/gdb/testsuite/gdb.base/catch-syscall.exp
+++ b/gdb/testsuite/gdb.base/catch-syscall.exp
@@ -127,7 +127,24 @@ proc check_return_from_syscall { syscall { pattern "" } } {
     }
 
     set thistest "syscall $syscall has returned"
-    gdb_test "continue" "Catchpoint $decimal \\(returned from syscall ${pattern}\\).*" $thistest
+    if { $pattern eq "execve" } {
+	gdb_test_multiple "continue" $thistest {
+	    -re -wrap "Catchpoint $decimal \\(returned from syscall ${pattern}\\).*" {
+		pass $thistest
+		return 1
+	    }
+	    -re -wrap ".*Breakpoint $decimal, main .*" {
+		# On Powerpc kernel does not report the returned from syscall
+		# as expected by the test.  GDB bugzilla 28623.
+		xfail $thistest
+		return 0
+	    }
+	}
+
+    } else {
+	gdb_test "continue" "Catchpoint $decimal \\(returned from syscall ${pattern}\\).*" $thistest
+	return 1
+    }
 }
 
 # Internal procedure that performs two 'continue' commands and checks if
@@ -142,7 +159,11 @@ proc check_continue { syscall { pattern "" } } {
     # Testing if the inferior has called the syscall.
     check_call_to_syscall $syscall $pattern
     # And now, that the syscall has returned.
-    check_return_from_syscall $syscall $pattern
+    if [check_return_from_syscall $syscall $pattern] {
+	return 1
+    } else {
+	return 0
+    }
 }
 
 # Inserts a syscall catchpoint with an argument.
@@ -348,11 +369,15 @@ proc test_catch_syscall_execve {} {
 	# Check for entry/return across the execve, making sure that the
 	# syscall_state isn't lost when turning into a new process.
 	insert_catch_syscall_with_arg "execve"
-	check_continue "execve"
+	if [check_continue "execve"] {
+	    # The check_continue test generates an XFAIL on Powerpc.  In
+	    # that case, gdb is already at main so don't do the continue.
 
-	# Continue to main so extended-remote can read files as needed.
-	# (Otherwise that "Reading" output confuses gdb_continue_to_end.)
-	gdb_continue "main"
+
+	    # Continue to main so extended-remote can read files as needed.
+	    # (Otherwise that "Reading" output confuses gdb_continue_to_end.)
+	    gdb_continue "main"
+	}
 
 	# Now can we finish?
 	check_for_program_end
-- 
2.30.2



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Ping  Re: [PATCH] gdb fix for catch-syscall.exp
  2021-11-29 16:46             ` Carl Love
@ 2021-12-02 16:32               ` Carl Love
  2021-12-10 18:36               ` Simon Marchi
  1 sibling, 0 replies; 18+ messages in thread
From: Carl Love @ 2021-12-02 16:32 UTC (permalink / raw)
  To: Simon Marchi, John Baldwin, gdb-patches; +Cc: Rogerio Alves

Simon:

I sent the following two patches, the first to revert the change, the
second to make the failures xfail.   I haven't heard anything.  Please
let me know if the first patch to revert the test change is OK and I
will commit that.  We can then work more on deciding how to handle the
failure, i.e. patch 2.

                 Carl 
---------------------------------------------------

On Mon, 2021-11-29 at 08:46 -0800, Carl Love wrote:
> Simon:
> 
> I split the latest patch so there is a separate revert patch and a
> patch to make the Powerepc test a xfail.  Note, the revert patch only
> reverts the part of the original patch that causes the regression
> test
> on amd64.  
> 
> Let me know if the first patch looks OK to commit to mainline.
> 
> We can continue to discuss the second patch to decide if there is a
> better implementation to add the xfail and if it really is a kernel
> failure.
> 
>                      Carl 
> --------------------------------------------------------------------
> [PATCH 1/2] gdb: Revert change to gdb.base/catch-syscall.exp
> 
> The previous commit:
>    commit ab198279120fe7937c0970a8bb881922726678f9
>    Author: Carl Love <cel@us.ibm.com>
>    Date:   Wed Nov 17 22:29:33 2021 +0000
> 
>        gdb fix for catch-syscall.exp
> 
> Fixed the test failure on PowerPC but broke the test on amd64.  The
> issue is
> both messages:
> 
>     Catchpoint 1 (call to syscall execve), ....
>     Catchpoint 1 (returned from syscall execve),  ....
> 
> are seen on amd64 whereas on PowerPC only the "call to syscall
> execve" is
> seen.  Based on the discussion on the mailing list, it is felt that
> this
> is really an issue with the PowerPC kernel support not reporting both
> events.
> 
> This patch just reverts the part of the commit that caused the
> regression
> failure on amd64.  The change for the istarget powerpc64-*-linux* is
> not
> being reverted.
> ---
>  gdb/testsuite/gdb.base/catch-syscall.exp | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp
> b/gdb/testsuite/gdb.base/catch-syscall.exp
> index 016d0a698a6..cdd5e2aec47 100644
> --- a/gdb/testsuite/gdb.base/catch-syscall.exp
> +++ b/gdb/testsuite/gdb.base/catch-syscall.exp
> @@ -348,9 +348,7 @@ proc test_catch_syscall_execve {} {
>  	# Check for entry/return across the execve, making sure that
> the
>  	# syscall_state isn't lost when turning into a new process.
>  	insert_catch_syscall_with_arg "execve"
> -
> -	# Check that the execve is called.
> -	check_call_to_syscall "execve"
> +	check_continue "execve"
>  
>  	# Continue to main so extended-remote can read files as needed.
>  	# (Otherwise that "Reading" output confuses
> gdb_continue_to_end.)
> -- 
> 2.30.2
> 
> -------------------------------------------------------------------
> ----
> [PATCH 2/2] gdb: Powerpc mark xfail in gdb.base/catch-syscall.exp
> 
> Powerpc is not reporting the
> 
>   Catchpoint 1 (returned from syscall execve),  ....
> 
> as expected.  The issue appears to be with the kernel not returning
> the
> expected result.  This patch marks the test failure as an xfail.
> 
> See gdb bugzilla 
> https://sourceware.org/bugzilla/show_bug.cgi?id=28623
> ---
>  gdb/testsuite/gdb.base/catch-syscall.exp | 37 ++++++++++++++++++++
> ----
>  1 file changed, 31 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp
> b/gdb/testsuite/gdb.base/catch-syscall.exp
> index cdd5e2aec47..0693f7afe27 100644
> --- a/gdb/testsuite/gdb.base/catch-syscall.exp
> +++ b/gdb/testsuite/gdb.base/catch-syscall.exp
> @@ -127,7 +127,24 @@ proc check_return_from_syscall { syscall {
> pattern "" } } {
>      }
>  
>      set thistest "syscall $syscall has returned"
> -    gdb_test "continue" "Catchpoint $decimal \\(returned from
> syscall ${pattern}\\).*" $thistest
> +    if { $pattern eq "execve" } {
> +	gdb_test_multiple "continue" $thistest {
> +	    -re -wrap "Catchpoint $decimal \\(returned from syscall
> ${pattern}\\).*" {
> +		pass $thistest
> +		return 1
> +	    }
> +	    -re -wrap ".*Breakpoint $decimal, main .*" {
> +		# On Powerpc kernel does not report the returned from
> syscall
> +		# as expected by the test.  GDB bugzilla 28623.
> +		xfail $thistest
> +		return 0
> +	    }
> +	}
> +
> +    } else {
> +	gdb_test "continue" "Catchpoint $decimal \\(returned from
> syscall ${pattern}\\).*" $thistest
> +	return 1
> +    }
>  }
>  
>  # Internal procedure that performs two 'continue' commands and
> checks if
> @@ -142,7 +159,11 @@ proc check_continue { syscall { pattern "" } } {
>      # Testing if the inferior has called the syscall.
>      check_call_to_syscall $syscall $pattern
>      # And now, that the syscall has returned.
> -    check_return_from_syscall $syscall $pattern
> +    if [check_return_from_syscall $syscall $pattern] {
> +	return 1
> +    } else {
> +	return 0
> +    }
>  }
>  
>  # Inserts a syscall catchpoint with an argument.
> @@ -348,11 +369,15 @@ proc test_catch_syscall_execve {} {
>  	# Check for entry/return across the execve, making sure that
> the
>  	# syscall_state isn't lost when turning into a new process.
>  	insert_catch_syscall_with_arg "execve"
> -	check_continue "execve"
> +	if [check_continue "execve"] {
> +	    # The check_continue test generates an XFAIL on
> Powerpc.  In
> +	    # that case, gdb is already at main so don't do the
> continue.
>  
> -	# Continue to main so extended-remote can read files as needed.
> -	# (Otherwise that "Reading" output confuses
> gdb_continue_to_end.)
> -	gdb_continue "main"
> +
> +	    # Continue to main so extended-remote can read files as
> needed.
> +	    # (Otherwise that "Reading" output confuses
> gdb_continue_to_end.)
> +	    gdb_continue "main"
> +	}
>  
>  	# Now can we finish?
>  	check_for_program_end


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gdb fix for catch-syscall.exp
  2021-11-29 16:46             ` Carl Love
  2021-12-02 16:32               ` Ping " Carl Love
@ 2021-12-10 18:36               ` Simon Marchi
  2021-12-10 19:59                 ` [PATCH v2] " Carl Love
  1 sibling, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2021-12-10 18:36 UTC (permalink / raw)
  To: Carl Love, John Baldwin, gdb-patches; +Cc: Rogerio Alves

On 2021-11-29 11:46 a.m., Carl Love via Gdb-patches wrote:
> Simon:
> 
> I split the latest patch so there is a separate revert patch and a
> patch to make the Powerepc test a xfail.  Note, the revert patch only
> reverts the part of the original patch that causes the regression test
> on amd64.  
> 
> Let me know if the first patch looks OK to commit to mainline.
> 
> We can continue to discuss the second patch to decide if there is a
> better implementation to add the xfail and if it really is a kernel
> failure.
> 
>                      Carl 
> --------------------------------------------------------------------
> [PATCH 1/2] gdb: Revert change to gdb.base/catch-syscall.exp
> 
> The previous commit:
>    commit ab198279120fe7937c0970a8bb881922726678f9
>    Author: Carl Love <cel@us.ibm.com>
>    Date:   Wed Nov 17 22:29:33 2021 +0000
> 
>        gdb fix for catch-syscall.exp
> 
> Fixed the test failure on PowerPC but broke the test on amd64.  The issue is
> both messages:
> 
>     Catchpoint 1 (call to syscall execve), ....
>     Catchpoint 1 (returned from syscall execve),  ....
> 
> are seen on amd64 whereas on PowerPC only the "call to syscall execve" is
> seen.  Based on the discussion on the mailing list, it is felt that this
> is really an issue with the PowerPC kernel support not reporting both events.
> 
> This patch just reverts the part of the commit that caused the regression
> failure on amd64.  The change for the istarget powerpc64-*-linux* is not
> being reverted.
> ---
>  gdb/testsuite/gdb.base/catch-syscall.exp | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
> index 016d0a698a6..cdd5e2aec47 100644
> --- a/gdb/testsuite/gdb.base/catch-syscall.exp
> +++ b/gdb/testsuite/gdb.base/catch-syscall.exp
> @@ -348,9 +348,7 @@ proc test_catch_syscall_execve {} {
>  	# Check for entry/return across the execve, making sure that the
>  	# syscall_state isn't lost when turning into a new process.
>  	insert_catch_syscall_with_arg "execve"
> -
> -	# Check that the execve is called.
> -	check_call_to_syscall "execve"
> +	check_continue "execve"
>  
>  	# Continue to main so extended-remote can read files as needed.
>  	# (Otherwise that "Reading" output confuses gdb_continue_to_end.)
> -- 
> 2.30.2

This was already done by this commit:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e19f8248297bb9864ff14368cc89d2446572837a

> diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
> index cdd5e2aec47..0693f7afe27 100644
> --- a/gdb/testsuite/gdb.base/catch-syscall.exp
> +++ b/gdb/testsuite/gdb.base/catch-syscall.exp
> @@ -127,7 +127,24 @@ proc check_return_from_syscall { syscall { pattern "" } } {
>      }
>  
>      set thistest "syscall $syscall has returned"
> -    gdb_test "continue" "Catchpoint $decimal \\(returned from syscall ${pattern}\\).*" $thistest
> +    if { $pattern eq "execve" } {
> +	gdb_test_multiple "continue" $thistest {
> +	    -re -wrap "Catchpoint $decimal \\(returned from syscall ${pattern}\\).*" {
> +		pass $thistest
> +		return 1
> +	    }
> +	    -re -wrap ".*Breakpoint $decimal, main .*" {
> +		# On Powerpc kernel does not report the returned from syscall
> +		# as expected by the test.  GDB bugzilla 28623.
> +		xfail $thistest
> +		return 0

This should check that the target is powerpc before doing the xfail, because
getting that result on any other architecture should result in a FAIL.  Probably
"[istarget "powerpc*-*-*"]".


> +	    }
> +	}
> +
> +    } else {
> +	gdb_test "continue" "Catchpoint $decimal \\(returned from syscall ${pattern}\\).*" $thistest
> +	return 1
> +    }
>  }
>  
>  # Internal procedure that performs two 'continue' commands and checks if
> @@ -142,7 +159,11 @@ proc check_continue { syscall { pattern "" } } {
>      # Testing if the inferior has called the syscall.
>      check_call_to_syscall $syscall $pattern
>      # And now, that the syscall has returned.
> -    check_return_from_syscall $syscall $pattern
> +    if [check_return_from_syscall $syscall $pattern] {
> +	return 1
> +    } else {
> +	return 0
> +    }

Can this just be

  return [check_return_from_syscall $syscall $pattern] ?

Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] gdb fix for catch-syscall.exp
  2021-12-10 18:36               ` Simon Marchi
@ 2021-12-10 19:59                 ` Carl Love
  2021-12-11  0:21                   ` Simon Marchi
  0 siblings, 1 reply; 18+ messages in thread
From: Carl Love @ 2021-12-10 19:59 UTC (permalink / raw)
  To: Simon Marchi, John Baldwin, gdb-patches; +Cc: Rogerio Alves, cel

Simon:

On Fri, 2021-12-10 at 13:36 -0500, Simon Marchi wrote:
> > +         -re -wrap ".*Breakpoint $decimal, main .*" {
> > +             # On Powerpc kernel does not report the returned from
> > syscall
> > +             # as expected by the test.  GDB bugzilla 28623.
> > +             xfail $thistest
> > +             return 0
> 
> This should check that the target is powerpc before doing the xfail,
> because
> getting that result on any other architecture should result in a
> FAIL.  Probably
> "[istarget "powerpc*-*-*"]".
> 

I added a check for istarget "powerpc64*-linux*".

< snip>

> >   
> >   # Internal procedure that performs two 'continue' commands and
> > checks if
> > @@ -142,7 +159,11 @@ proc check_continue { syscall { pattern "" } }
> > {
> >       # Testing if the inferior has called the syscall.
> >       check_call_to_syscall $syscall $pattern
> >       # And now, that the syscall has returned.
> > -    check_return_from_syscall $syscall $pattern
> > +    if [check_return_from_syscall $syscall $pattern] {
> > +     return 1
> > +    } else {
> > +     return 0
> > +    }
> 
> Can this just be
> 
>   return [check_return_from_syscall $syscall $pattern] ?
> 
Yup, that works for me.

The updated patch is below.  It has been retested on PowerPC and Intel
with no new regressions.

Please let me know if it is OK to commit to gdb mainline.  Thanks.

                     Carl 
-----------------------------------------------------
gdb: Powerpc mark xfail in gdb.base/catch-syscall.exp

Powerpc is not reporting the

  Catchpoint 1 (returned from syscall execve),  ....

as expected.  The issue appears to be with the kernel not returning the
expected result.  This patch marks the test failure as an xfail.

See gdb bugzilla https://sourceware.org/bugzilla/show_bug.cgi?id=28623
---
 gdb/testsuite/gdb.base/catch-syscall.exp | 38 ++++++++++++++++++++----
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
index cdd5e2aec47..76c5cfbc6d6 100644
--- a/gdb/testsuite/gdb.base/catch-syscall.exp
+++ b/gdb/testsuite/gdb.base/catch-syscall.exp
@@ -127,7 +127,29 @@ proc check_return_from_syscall { syscall { pattern "" } } {
     }
 
     set thistest "syscall $syscall has returned"
-    gdb_test "continue" "Catchpoint $decimal \\(returned from syscall ${pattern}\\).*" $thistest
+    if { $pattern eq "execve" } {
+	gdb_test_multiple "continue" $thistest {
+	    -re -wrap "Catchpoint $decimal \\(returned from syscall ${pattern}\\).*" {
+		pass $thistest
+		return 1
+	    }
+	    -re -wrap ".*Breakpoint $decimal, main .*" {
+		# On Powerpc kernel does not report the returned from syscall
+		# as expected by the test.  GDB bugzilla 28623.
+		puts "CARLL test failed"
+		if { [istarget "powerpc64*-linux*"] } {
+		    xfail $thistest
+		} else {
+		    fail $thistest
+		}
+		return 0
+	    }
+	}
+
+    } else {
+	gdb_test "continue" "Catchpoint $decimal \\(returned from syscall ${pattern}\\).*" $thistest
+	return 1
+    }
 }
 
 # Internal procedure that performs two 'continue' commands and checks if
@@ -142,7 +164,7 @@ proc check_continue { syscall { pattern "" } } {
     # Testing if the inferior has called the syscall.
     check_call_to_syscall $syscall $pattern
     # And now, that the syscall has returned.
-    check_return_from_syscall $syscall $pattern
+    return [check_return_from_syscall $syscall $pattern]
 }
 
 # Inserts a syscall catchpoint with an argument.
@@ -348,11 +370,15 @@ proc test_catch_syscall_execve {} {
 	# Check for entry/return across the execve, making sure that the
 	# syscall_state isn't lost when turning into a new process.
 	insert_catch_syscall_with_arg "execve"
-	check_continue "execve"
+	if [check_continue "execve"] {
+	    # The check_continue test generates an XFAIL on Powerpc.  In
+	    # that case, gdb is already at main so don't do the continue.
+
 
-	# Continue to main so extended-remote can read files as needed.
-	# (Otherwise that "Reading" output confuses gdb_continue_to_end.)
-	gdb_continue "main"
+	    # Continue to main so extended-remote can read files as needed.
+	    # (Otherwise that "Reading" output confuses gdb_continue_to_end.)
+	    gdb_continue "main"
+	}
 
 	# Now can we finish?
 	check_for_program_end
-- 
2.30.2




^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] gdb fix for catch-syscall.exp
  2021-12-10 19:59                 ` [PATCH v2] " Carl Love
@ 2021-12-11  0:21                   ` Simon Marchi
  0 siblings, 0 replies; 18+ messages in thread
From: Simon Marchi @ 2021-12-11  0:21 UTC (permalink / raw)
  To: Carl Love, Simon Marchi, John Baldwin, gdb-patches; +Cc: Rogerio Alves

> gdb: Powerpc mark xfail in gdb.base/catch-syscall.exp
> 
> Powerpc is not reporting the
> 
>   Catchpoint 1 (returned from syscall execve),  ....
> 
> as expected.  The issue appears to be with the kernel not returning the
> expected result.  This patch marks the test failure as an xfail.
> 
> See gdb bugzilla https://sourceware.org/bugzilla/show_bug.cgi?id=28623

When we got rid of ChangeLogs, we agreed on the format:

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28623

when referring to bugzilla entries, let's use that.

> ---
>  gdb/testsuite/gdb.base/catch-syscall.exp | 38 ++++++++++++++++++++----
>  1 file changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
> index cdd5e2aec47..76c5cfbc6d6 100644
> --- a/gdb/testsuite/gdb.base/catch-syscall.exp
> +++ b/gdb/testsuite/gdb.base/catch-syscall.exp
> @@ -127,7 +127,29 @@ proc check_return_from_syscall { syscall { pattern "" } } {
>      }
>  
>      set thistest "syscall $syscall has returned"
> -    gdb_test "continue" "Catchpoint $decimal \\(returned from syscall ${pattern}\\).*" $thistest
> +    if { $pattern eq "execve" } {
> +	gdb_test_multiple "continue" $thistest {
> +	    -re -wrap "Catchpoint $decimal \\(returned from syscall ${pattern}\\).*" {
> +		pass $thistest
> +		return 1
> +	    }
> +	    -re -wrap ".*Breakpoint $decimal, main .*" {
> +		# On Powerpc kernel does not report the returned from syscall

kernel -> the kernel
returned -> return

> +		# as expected by the test.  GDB bugzilla 28623.
> +		puts "CARLL test failed"

Leftover print.

Ok with those fixed.

Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-12-11  0:21 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-17 23:30 [PATCH] gdb fix for catch-syscall.exp Carl Love
2021-11-18 15:23 ` Tom Tromey
2021-11-18 18:10 ` Simon Marchi
2021-11-20  0:27   ` Carl Love
2021-11-22  1:07     ` Simon Marchi
2021-11-22 18:16       ` Carl Love
2021-11-22 17:01     ` John Baldwin
2021-11-23 20:34       ` Simon Marchi
2021-11-23 22:34         ` John Baldwin
2021-11-24 17:46           ` Carl Love
2021-11-24 17:51             ` John Baldwin
2021-11-24  1:15         ` Carl Love
2021-11-24 19:29           ` Simon Marchi
2021-11-29 16:46             ` Carl Love
2021-12-02 16:32               ` Ping " Carl Love
2021-12-10 18:36               ` Simon Marchi
2021-12-10 19:59                 ` [PATCH v2] " Carl Love
2021-12-11  0:21                   ` Simon Marchi

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).