Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] btrace: check for indirect jump return in _Unwind_RaiseException
@ 2018-09-27  7:42 Markus Metzger
  2018-09-27  7:44 ` [PATCH] testsuite: fix is_amd64_regs_target Markus Metzger
  0 siblings, 1 reply; 6+ messages in thread
From: Markus Metzger @ 2018-09-27  7:42 UTC (permalink / raw)
  To: gdb-patches; +Cc: Markus Metzger

Some versions of _Unwind_RaiseException, e.g. on Fedora 28, use an
indirect jump to return to the exception handler.

This messes up the output of "record function-call-history /c" since the
return is interpreted as cross-function goto.  It had been detected by
gdb.btrace/exception.exp.

Add a heuristic for "_Unwind_*" functions to interpret an indirect jump
that ends in one of our caller functions as return to the first instance
of that function in our call stack.

Signed-off-by: Markus Metzger  <markus.t.metzger@intel.com>

gdb/
	* btrace.c (ftrace_update_function): Add indirect jump heuristic.
---
 gdb/btrace.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/gdb/btrace.c b/gdb/btrace.c
index e25f047ce24..d3ad0ab7de8 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -620,6 +620,20 @@ ftrace_update_function (struct btrace_thread_info *btinfo, CORE_ADDR pc)
 	    if (start == pc)
 	      return ftrace_new_tailcall (btinfo, mfun, fun);
 
+	    /* Some versions of _Unwind_RaiseException use an indirect
+	       jump to 'return' to the exception handler of the caller
+	       handling the exception instead of a return.  Let's restrict
+	       this heuristic to that and related functions.  */
+	    const char *fname = ftrace_print_function_name (bfun);
+	    if (strncmp (fname, "_Unwind_", strlen ("_Unwind_")) == 0)
+	      {
+		struct btrace_function *caller
+		  = ftrace_find_call_by_number (btinfo, bfun->up);
+		caller = ftrace_find_caller (btinfo, caller, mfun, fun);
+		if (caller != NULL)
+		  return ftrace_new_return (btinfo, mfun, fun);
+	      }
+
 	    /* If we can't determine the function for PC, we treat a jump at
 	       the end of the block as tail call if we're switching functions
 	       and as an intra-function branch if we don't.  */
-- 
2.17.1


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

* [PATCH] testsuite: fix is_amd64_regs_target
  2018-09-27  7:42 [PATCH] btrace: check for indirect jump return in _Unwind_RaiseException Markus Metzger
@ 2018-09-27  7:44 ` Markus Metzger
  2018-09-27 21:10   ` Tom Tromey
  0 siblings, 1 reply; 6+ messages in thread
From: Markus Metzger @ 2018-09-27  7:44 UTC (permalink / raw)
  To: gdb-patches

Commit

    c221b2f Testsuite: Add gdb_can_simple_compile

changed the source file name extension of the test program from .s to .c
resulting in compile fails.  This, in turn, causes is_amd64_regs_target
checks to fail.  In gdb.btrace/tailcall.exp and others, this causes the
wrong source file to be picked and the test to fail on 64-bit targets.

Change the test source from an assembly program to a C program using
inline assembly.

There is a similar case for is_aarch32_target that I have not touched as I
would not be able to test my changes.

2018-09-27  Markus Metzger  <markus.t.metzger@intel.com>

testsuite/
	* lib/gdb.exp (is_amd64_regs_target): Change assembly to C inline
	assembly.
---
 gdb/testsuite/lib/gdb.exp | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index f32abfedd52..1eea92298c4 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2467,13 +2467,14 @@ gdb_caching_proc is_amd64_regs_target {
 	return 0
     }
 
-    set list {}
-    foreach reg \
-	{rax rbx rcx rdx rsi rdi rbp rsp r8 r9 r10 r11 r12 r13 r14 r15} {
-	    lappend list "\tincq %$reg"
-	}
+    return [gdb_can_simple_compile is_amd64_regs_target {
+	int main (void) {
+	    asm ("incq %rax");
+	    asm ("incq %r15");
 
-    return [gdb_can_simple_compile is_amd64_regs_target [join $list \n]]
+	    return 0;
+	}
+    }]
 }
 
 # Return 1 if this target is an x86 or x86-64 with -m32.
-- 
2.17.1


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

* Re: [PATCH] testsuite: fix is_amd64_regs_target
  2018-09-27  7:44 ` [PATCH] testsuite: fix is_amd64_regs_target Markus Metzger
@ 2018-09-27 21:10   ` Tom Tromey
  2018-09-28  7:07     ` Metzger, Markus T
  0 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2018-09-27 21:10 UTC (permalink / raw)
  To: Markus Metzger; +Cc: gdb-patches

>>>>> "Markus" == Markus Metzger <markus.t.metzger@intel.com> writes:

Markus> testsuite/
Markus> 	* lib/gdb.exp (is_amd64_regs_target): Change assembly to C inline
Markus> 	assembly.

Markus> -    set list {}
Markus> -    foreach reg \
Markus> -	{rax rbx rcx rdx rsi rdi rbp rsp r8 r9 r10 r11 r12 r13 r14 r15} {
Markus> -	    lappend list "\tincq %$reg"
Markus> -	}
Markus> +    return [gdb_can_simple_compile is_amd64_regs_target {
Markus> +	int main (void) {
Markus> +	    asm ("incq %rax");
Markus> +	    asm ("incq %r15");

I suppose reducing the number of registers tested doesn't materially
affect the test.

Assuming that's the case, this is ok.  Thank you.

Tom


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

* RE: [PATCH] testsuite: fix is_amd64_regs_target
  2018-09-27 21:10   ` Tom Tromey
@ 2018-09-28  7:07     ` Metzger, Markus T
  2018-09-28 17:34       ` Tom Tromey
  0 siblings, 1 reply; 6+ messages in thread
From: Metzger, Markus T @ 2018-09-28  7:07 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

Hello Tom,

Thanks for your review.

> Markus> -    set list {}
> Markus> -    foreach reg \
> Markus> -	{rax rbx rcx rdx rsi rdi rbp rsp r8 r9 r10 r11 r12 r13 r14 r15} {
> Markus> -	    lappend list "\tincq %$reg"
> Markus> -	}
> Markus> +    return [gdb_can_simple_compile is_amd64_regs_target {
> Markus> +	int main (void) {
> Markus> +	    asm ("incq %rax");
> Markus> +	    asm ("incq %r15");
> 
> I suppose reducing the number of registers tested doesn't materially affect the
> test.

That's my assumption, as well.  We already checked the architecture so we know it
is some x86 flavor.  A single 64-bit register should suffice.

The gdb.btrace suite passes on 32-bit and 64-bit including m32.  I have not run the
full suite since the problem was already seen on the gdb.btrace suite.

Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


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

* Re: [PATCH] testsuite: fix is_amd64_regs_target
  2018-09-28  7:07     ` Metzger, Markus T
@ 2018-09-28 17:34       ` Tom Tromey
       [not found]         ` <8a5ded70-add4-df1d-5f7f-6f7682d58059@ericsson.com>
  0 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2018-09-28 17:34 UTC (permalink / raw)
  To: Metzger, Markus T; +Cc: Tom Tromey, gdb-patches

>>>>> "Markus" == Metzger, Markus T <markus.t.metzger@intel.com> writes:

Markus> That's my assumption, as well.  We already checked the architecture so we know it
Markus> is some x86 flavor.  A single 64-bit register should suffice.

I didn't see it go in, so I wanted to reiterate that I think it is ok.

Tom


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

* Re: [PATCH] testsuite: fix is_amd64_regs_target
       [not found]         ` <8a5ded70-add4-df1d-5f7f-6f7682d58059@ericsson.com>
@ 2018-09-29 18:09           ` Pedro Alves
  0 siblings, 0 replies; 6+ messages in thread
From: Pedro Alves @ 2018-09-29 18:09 UTC (permalink / raw)
  To: Simon Marchi, Tom Tromey, Metzger, Markus T; +Cc: gdb-patches

On 09/28/2018 09:27 PM, Simon Marchi wrote:

> Maybe I'm missing something obvious, but could one of you explain to me
> the comment of that proc, which is
> 
> # Return 1 if target has x86_64 registers - either amd64 or x32.
> # x32 target identifies as x86_64-*-linux*, therefore it cannot be determined
> # just from the target string.
> 
> If I understand correctly, is_amd64_regs_target should return 1 when using the
> x32 abi, as well as when using the standard amd64 abi.  The comment says an x32
> target has an x86_64-* triplet... so can't we just check the triplet, and return
> 1 if it's x86-64-*?

No, because toolchains configured with i686-* triplets can also be
used in combination with -mx32/-m64.

Same for is_64_target, is_lp64_target, etc.

Thanks,
Pedro Alves


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

end of thread, other threads:[~2018-09-29 18:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-27  7:42 [PATCH] btrace: check for indirect jump return in _Unwind_RaiseException Markus Metzger
2018-09-27  7:44 ` [PATCH] testsuite: fix is_amd64_regs_target Markus Metzger
2018-09-27 21:10   ` Tom Tromey
2018-09-28  7:07     ` Metzger, Markus T
2018-09-28 17:34       ` Tom Tromey
     [not found]         ` <8a5ded70-add4-df1d-5f7f-6f7682d58059@ericsson.com>
2018-09-29 18:09           ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox