public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH][gdb/testsuite] Fix gdb.arch/amd64-stap-three-arg-disp.S
@ 2021-01-11  7:06 Tom de Vries
  2021-01-11 17:16 ` Sergio Durigan Junior
  0 siblings, 1 reply; 2+ messages in thread
From: Tom de Vries @ 2021-01-11  7:06 UTC (permalink / raw)
  To: gdb-patches; +Cc: sergiodj

Hi,

On SLE-11 I ran into:
...
(gdb) print $_probe_arg0^M
Cannot access memory at address 0x8000003fe05c^M
(gdb) FAIL: gdb.arch/amd64-stap-special-operands.exp: probe: three_arg: \
  print $_probe_arg0
...

The memory cannot be accessed because the address used to evaluate
$_probe_arg0 at the probe point is incorrect.

The address is calculated using this expression:
...
.asciz "-4@-4(%rbp,%ebx,0)"
...
which uses $ebx, but $ebx is uninitialized at the probe point.

The test-case does contain a "movl $0, %ebx" insn to set $ebx to 0, but that
insn is placed after the probe point.  We could fix this by moving the insn
to before the probe point.  But, $ebx is also a callee-save register, so
normally, if we modify it, we also need to save and restore it, which is
currently not done.  This is currently not harmful, because we don't run the
test-case further than the probe point, but it's bound to cause confusion.

So, fix this instead by using $eax instead in the expression, and moving the
insn setting $eax to 0 to before the probe point.

Any comments?

Thanks,
- Tom

[gdb/testsuite] Fix gdb.arch/amd64-stap-three-arg-disp.S

gdb/testsuite/ChangeLog:

2021-01-10  Tom de Vries  <tdevries@suse.de>

	PR testsuite/26968
	* gdb.arch/amd64-stap-three-arg-disp.S: Remove insn modifying $ebx.
	Move insn setting $eax to before probe point.

---
 gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.S | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.S b/gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.S
index cf3856f41e9..17b64048082 100644
--- a/gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.S
+++ b/gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.S
@@ -15,6 +15,7 @@ main:
 	movl	%edi, -20(%rbp)
 	movq	%rsi, -32(%rbp)
 	movl	$10, -4(%rbp)
+	movl	$0, %eax
 #APP
 # 8 "amd64-stap-three-arg-disp.c" 1
 	990: nop
@@ -28,7 +29,7 @@ main:
 .8byte 0
 .asciz "test"
 .asciz "three_arg"
-.asciz "-4@-4(%rbp,%ebx,0)"
+.asciz "-4@-4(%rbp,%eax,0)"
 994: .balign 4
 .popsection
 
@@ -45,8 +46,6 @@ _.stapsdt.base: .space 1
 
 # 0 "" 2
 #NO_APP
-	movl	$0, %eax
-	movl	$0, %ebx
 	popq	%rbp
 	.cfi_def_cfa 7, 8
 # SUCC: EXIT [100.0%] 

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

* Re: [PATCH][gdb/testsuite] Fix gdb.arch/amd64-stap-three-arg-disp.S
  2021-01-11  7:06 [PATCH][gdb/testsuite] Fix gdb.arch/amd64-stap-three-arg-disp.S Tom de Vries
@ 2021-01-11 17:16 ` Sergio Durigan Junior
  0 siblings, 0 replies; 2+ messages in thread
From: Sergio Durigan Junior @ 2021-01-11 17:16 UTC (permalink / raw)
  To: Tom de Vries; +Cc: gdb-patches

On Monday, January 11 2021, Tom de Vries wrote:

> Hi,
>
> On SLE-11 I ran into:
> ...
> (gdb) print $_probe_arg0^M
> Cannot access memory at address 0x8000003fe05c^M
> (gdb) FAIL: gdb.arch/amd64-stap-special-operands.exp: probe: three_arg: \
>   print $_probe_arg0
> ...
>
> The memory cannot be accessed because the address used to evaluate
> $_probe_arg0 at the probe point is incorrect.
>
> The address is calculated using this expression:
> ...
> .asciz "-4@-4(%rbp,%ebx,0)"
> ...
> which uses $ebx, but $ebx is uninitialized at the probe point.
>
> The test-case does contain a "movl $0, %ebx" insn to set $ebx to 0, but that
> insn is placed after the probe point.  We could fix this by moving the insn
> to before the probe point.  But, $ebx is also a callee-save register, so
> normally, if we modify it, we also need to save and restore it, which is
> currently not done.  This is currently not harmful, because we don't run the
> test-case further than the probe point, but it's bound to cause confusion.
>
> So, fix this instead by using $eax instead in the expression, and moving the
> insn setting $eax to 0 to before the probe point.

Thanks for the patch.

> Any comments?

The fix makes sense to me as-is.

Eventually, I'd like to convert this testcase to use the same format as
gdb.arch/amd64-stap-optional-prefix.S, which is much simpler to
read/hack.  It shouldn't be a hard thing to do, but I don't want to
impose on you and ask you to do that, so if you want to go ahead and
push this patch, feel free to do so.

Thanks,

> Thanks,
> - Tom
>
> [gdb/testsuite] Fix gdb.arch/amd64-stap-three-arg-disp.S
>
> gdb/testsuite/ChangeLog:
>
> 2021-01-10  Tom de Vries  <tdevries@suse.de>
>
> 	PR testsuite/26968
> 	* gdb.arch/amd64-stap-three-arg-disp.S: Remove insn modifying $ebx.
> 	Move insn setting $eax to before probe point.
>
> ---
>  gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.S | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.S b/gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.S
> index cf3856f41e9..17b64048082 100644
> --- a/gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.S
> +++ b/gdb/testsuite/gdb.arch/amd64-stap-three-arg-disp.S
> @@ -15,6 +15,7 @@ main:
>  	movl	%edi, -20(%rbp)
>  	movq	%rsi, -32(%rbp)
>  	movl	$10, -4(%rbp)
> +	movl	$0, %eax
>  #APP
>  # 8 "amd64-stap-three-arg-disp.c" 1
>  	990: nop
> @@ -28,7 +29,7 @@ main:
>  .8byte 0
>  .asciz "test"
>  .asciz "three_arg"
> -.asciz "-4@-4(%rbp,%ebx,0)"
> +.asciz "-4@-4(%rbp,%eax,0)"
>  994: .balign 4
>  .popsection
>  
> @@ -45,8 +46,6 @@ _.stapsdt.base: .space 1
>  
>  # 0 "" 2
>  #NO_APP
> -	movl	$0, %eax
> -	movl	$0, %ebx
>  	popq	%rbp
>  	.cfi_def_cfa 7, 8
>  # SUCC: EXIT [100.0%] 

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
https://sergiodj.net/

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

end of thread, other threads:[~2021-01-11 17:16 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-11  7:06 [PATCH][gdb/testsuite] Fix gdb.arch/amd64-stap-three-arg-disp.S Tom de Vries
2021-01-11 17:16 ` Sergio Durigan Junior

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