Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH]: Setup sparc32 to support dwarf2 unwind sniffer
@ 2006-04-05  3:30 David S. Miller
  2006-04-05  9:11 ` Mark Kettenis
  0 siblings, 1 reply; 4+ messages in thread
From: David S. Miller @ 2006-04-05  3:30 UTC (permalink / raw)
  To: gdb-patches


This doesn't actually enable the sniffer, it just adds the
necessary dwarf2_frame_set_init_reg() handler, similar to what
sparc64 already has implemented in sparc64-tdep.c

We can't unilaterally make use of the dwarf2 unwinder on all
sparc targets yet, because of the mentioned StackGhost issues
mentioned in the comment.

But after this patch, we can decide to enable dwarf2 unwinding on a
per-target basis.  And we can do so right now on targets that do not
support StackGhost.  Later if the StackGhost issue is resolved, we
can do this for all Sparc targets.

This is very desirable because many current Sparc bugs are due to
the fact that sparc_analyze_prologue() cannot deal at all with clever
assembler sequences, for example in cases where the save is deferred
to somewhere past the beginning of the function which is also a
valid compiler optimization.

Ok to apply?

2006-04-04  David S. Miller  <davem@sunset.davemloft.net>

	* sparc-tdep.c (sparc32_dwarf2_frame_init_reg): New.
	(sparc32_gdbarch_init): Pass it to dwarf2_frame_set_init_reg.
	* Makefile.in (sparc-tdep.o): Update dependencies.
	
--- sparc-tdep.c.~1~	2006-02-25 15:05:03.000000000 -0800
+++ sparc-tdep.c	2006-04-04 20:19:46.000000000 -0700
@@ -22,6 +22,7 @@
 #include "defs.h"
 #include "arch-utils.h"
 #include "dis-asm.h"
+#include "dwarf2-frame.h"
 #include "floatformat.h"
 #include "frame.h"
 #include "frame-base.h"
@@ -994,6 +995,32 @@
 	  || (sparc_floating_p (type) && TYPE_LENGTH (type) == 16));
 }
 
+static void
+sparc32_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
+			       struct dwarf2_frame_state_reg *reg)
+{
+  switch (regnum)
+    {
+    case SPARC_G0_REGNUM:
+      /* Since %g0 is always zero, there is no point in saving it, and
+	 people will be inclined omit it from the CFI.  Make sure we
+	 don't warn about that.  */
+      reg->how = DWARF2_FRAME_REG_SAME_VALUE;
+      break;
+    case SPARC_SP_REGNUM:
+      reg->how = DWARF2_FRAME_REG_CFA;
+      break;
+    case SPARC32_PC_REGNUM:
+      reg->how = DWARF2_FRAME_REG_RA_OFFSET;
+      reg->loc.offset = 8;
+      break;
+    case SPARC32_NPC_REGNUM:
+      reg->how = DWARF2_FRAME_REG_RA_OFFSET;
+      reg->loc.offset = 12;
+      break;
+    }
+}
+
 \f
 /* The SPARC Architecture doesn't have hardware single-step support,
    and most operating systems don't implement it either, so we provide
@@ -1248,6 +1275,11 @@
   /* Hook in ABI-specific overrides, if they have been registered.  */
   gdbarch_init_osabi (info, gdbarch);
 
+  /* Hook in the DWARF CFI frame unwinder.  */
+  dwarf2_frame_set_init_reg (gdbarch, sparc32_dwarf2_frame_init_reg);
+  /* FIXME: kettenis/20050423: Don't enable the unwinder until the
+     StackGhost issues have been resolved.  */
+
   frame_unwind_append_sniffer (gdbarch, sparc32_frame_sniffer);
 
   /* If we have register sets, enable the generic core file support.  */
--- Makefile.in.~1~	2006-04-04 15:12:20.000000000 -0700
+++ Makefile.in	2006-04-04 20:23:16.000000000 -0700
@@ -2666,10 +2666,10 @@
 	$(sparc_tdep_h) $(solib_svr4_h)
 sparc-stub.o: sparc-stub.c
 sparc-tdep.o: sparc-tdep.c $(defs_h) $(arch_utils_h) $(dis_asm_h) \
-	$(floatformat_h) $(frame_h) $(frame_base_h) $(frame_unwind_h) \
-	$(gdbcore_h) $(gdbtypes_h) $(inferior_h) $(symtab_h) $(objfiles_h) \
-	$(osabi_h) $(regcache_h) $(target_h) $(value_h) $(gdb_assert_h) \
-	$(gdb_string_h) $(sparc_tdep_h)
+	$(dwarf2_frame_h) $(floatformat_h) $(frame_h) $(frame_base_h) \
+	$(frame_unwind_h) $(gdbcore_h) $(gdbtypes_h) $(inferior_h) \
+	$(symtab_h) $(objfiles_h) $(osabi_h) $(regcache_h) $(target_h) \
+	$(value_h) $(gdb_assert_h) $(gdb_string_h) $(sparc_tdep_h)
 stabsread.o: stabsread.c $(defs_h) $(gdb_string_h) $(bfd_h) $(gdb_obstack_h) \
 	$(symtab_h) $(gdbtypes_h) $(expression_h) $(symfile_h) $(objfiles_h) \
 	$(aout_stab_gnu_h) $(libaout_h) $(aout_aout64_h) $(gdb_stabs_h) \


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

* Re: [PATCH]: Setup sparc32 to support dwarf2 unwind sniffer
  2006-04-05  3:30 [PATCH]: Setup sparc32 to support dwarf2 unwind sniffer David S. Miller
@ 2006-04-05  9:11 ` Mark Kettenis
  2006-04-05 13:38   ` Daniel Jacobowitz
  2006-04-05 18:54   ` David S. Miller
  0 siblings, 2 replies; 4+ messages in thread
From: Mark Kettenis @ 2006-04-05  9:11 UTC (permalink / raw)
  To: David S. Miller; +Cc: gdb-patches

>
>  This doesn't actually enable the sniffer, it just adds the
>  necessary dwarf2_frame_set_init_reg() handler, similar to what
>  sparc64 already has implemented in sparc64-tdep.c
>
>  We can't unilaterally make use of the dwarf2 unwinder on all
>  sparc targets yet, because of the mentioned StackGhost issues
>  mentioned in the comment.
>
>  But after this patch, we can decide to enable dwarf2 unwinding on a
>  per-target basis.  And we can do so right now on targets that do not
>  support StackGhost.  Later if the StackGhost issue is resolved, we
>  can do this for all Sparc targets.
>
>  This is very desirable because many current Sparc bugs are due to
>  the fact that sparc_analyze_prologue() cannot deal at all with clever
>  assembler sequences, for example in cases where the save is deferred
>  to somewhere past the beginning of the function which is also a
>  valid compiler optimization.

Perhaps we should try to address those issues.  I never went through the
trouble of making the sparc and sparc64 prologue analyzers any smarter,
because I never encountered any code that made it fail.  This is probably
because OpenBSD/sparc64 uses GCC 3.3.5 (and OpenBSD/sparc uses an even
older compiler).  If you can post some disaambly of (real-life) prologues
that the current unwinder doesn't handle, I'll happiliy turn them into
testcases and try to improve the analyzer.

>  Ok to apply?

Yes, thanks!

I didn't do this before, because OpenBSD/sparc still uses stabs, so I
had no way to test this.

Don't hesitate to enable the dwarf2 unwinder on Linux.

Marl

>
>  2006-04-04  David S. Miller  <davem@sunset.davemloft.net>
>
>  	* sparc-tdep.c (sparc32_dwarf2_frame_init_reg): New.
>  	(sparc32_gdbarch_init): Pass it to dwarf2_frame_set_init_reg.
>  	* Makefile.in (sparc-tdep.o): Update dependencies.
>
>  --- sparc-tdep.c.~1~	2006-02-25 15:05:03.000000000 -0800
>  +++ sparc-tdep.c	2006-04-04 20:19:46.000000000 -0700
>  @@ -22,6 +22,7 @@
>   #include "defs.h"
>   #include "arch-utils.h"
>   #include "dis-asm.h"
>  +#include "dwarf2-frame.h"
>   #include "floatformat.h"
>   #include "frame.h"
>   #include "frame-base.h"
>  @@ -994,6 +995,32 @@
>   	  || (sparc_floating_p (type) && TYPE_LENGTH (type) == 16));
>   }
>
>  +static void
>  +sparc32_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>  +			       struct dwarf2_frame_state_reg *reg)
>  +{
>  +  switch (regnum)
>  +    {
>  +    case SPARC_G0_REGNUM:
>  +      /* Since %g0 is always zero, there is no point in saving it, and
>  +	 people will be inclined omit it from the CFI.  Make sure we
>  +	 don't warn about that.  */
>  +      reg->how = DWARF2_FRAME_REG_SAME_VALUE;
>  +      break;
>  +    case SPARC_SP_REGNUM:
>  +      reg->how = DWARF2_FRAME_REG_CFA;
>  +      break;
>  +    case SPARC32_PC_REGNUM:
>  +      reg->how = DWARF2_FRAME_REG_RA_OFFSET;
>  +      reg->loc.offset = 8;
>  +      break;
>  +    case SPARC32_NPC_REGNUM:
>  +      reg->how = DWARF2_FRAME_REG_RA_OFFSET;
>  +      reg->loc.offset = 12;
>  +      break;
>  +    }
>  +}
>  +
>   \f
>   /* The SPARC Architecture doesn't have hardware single-step support,
>      and most operating systems don't implement it either, so we provide
>  @@ -1248,6 +1275,11 @@
>     /* Hook in ABI-specific overrides, if they have been registered.  */
>     gdbarch_init_osabi (info, gdbarch);
>
>  +  /* Hook in the DWARF CFI frame unwinder.  */
>  +  dwarf2_frame_set_init_reg (gdbarch, sparc32_dwarf2_frame_init_reg);
>  +  /* FIXME: kettenis/20050423: Don't enable the unwinder until the
>  +     StackGhost issues have been resolved.  */
>  +
>     frame_unwind_append_sniffer (gdbarch, sparc32_frame_sniffer);
>
>     /* If we have register sets, enable the generic core file support.  */
>  --- Makefile.in.~1~	2006-04-04 15:12:20.000000000 -0700
>  +++ Makefile.in	2006-04-04 20:23:16.000000000 -0700
>  @@ -2666,10 +2666,10 @@
>   	$(sparc_tdep_h) $(solib_svr4_h)
>   sparc-stub.o: sparc-stub.c
>   sparc-tdep.o: sparc-tdep.c $(defs_h) $(arch_utils_h) $(dis_asm_h) \
>  -	$(floatformat_h) $(frame_h) $(frame_base_h) $(frame_unwind_h) \
>  -	$(gdbcore_h) $(gdbtypes_h) $(inferior_h) $(symtab_h) $(objfiles_h) \
>  -	$(osabi_h) $(regcache_h) $(target_h) $(value_h) $(gdb_assert_h) \
>  -	$(gdb_string_h) $(sparc_tdep_h)
>  +	$(dwarf2_frame_h) $(floatformat_h) $(frame_h) $(frame_base_h) \
>  +	$(frame_unwind_h) $(gdbcore_h) $(gdbtypes_h) $(inferior_h) \
>  +	$(symtab_h) $(objfiles_h) $(osabi_h) $(regcache_h) $(target_h) \
>  +	$(value_h) $(gdb_assert_h) $(gdb_string_h) $(sparc_tdep_h)
>   stabsread.o: stabsread.c $(defs_h) $(gdb_string_h) $(bfd_h)
>  $(gdb_obstack_h) \
>   	$(symtab_h) $(gdbtypes_h) $(expression_h) $(symfile_h) $(objfiles_h) \
>   	$(aout_stab_gnu_h) $(libaout_h) $(aout_aout64_h) $(gdb_stabs_h) \
>



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

* Re: [PATCH]: Setup sparc32 to support dwarf2 unwind sniffer
  2006-04-05  9:11 ` Mark Kettenis
@ 2006-04-05 13:38   ` Daniel Jacobowitz
  2006-04-05 18:54   ` David S. Miller
  1 sibling, 0 replies; 4+ messages in thread
From: Daniel Jacobowitz @ 2006-04-05 13:38 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: David S. Miller, gdb-patches

On Wed, Apr 05, 2006 at 11:11:05AM +0200, Mark Kettenis wrote:
> Perhaps we should try to address those issues.  I never went through the
> trouble of making the sparc and sparc64 prologue analyzers any smarter,
> because I never encountered any code that made it fail.

If you want to make the analyzer smarter, which would be nice, may I
recommend trying Jim's now-committed prologue-value.h interface?

I'll get around to posting the rewritten Thumb analyzer soon, I hope.
It was remarkably easy to write, and works better too.


-- 
Daniel Jacobowitz
CodeSourcery


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

* Re: [PATCH]: Setup sparc32 to support dwarf2 unwind sniffer
  2006-04-05  9:11 ` Mark Kettenis
  2006-04-05 13:38   ` Daniel Jacobowitz
@ 2006-04-05 18:54   ` David S. Miller
  1 sibling, 0 replies; 4+ messages in thread
From: David S. Miller @ 2006-04-05 18:54 UTC (permalink / raw)
  To: mark.kettenis; +Cc: gdb-patches

From: "Mark Kettenis" <mark.kettenis@xs4all.nl>
Date: Wed, 5 Apr 2006 11:11:05 +0200 (CEST)

> >  This is very desirable because many current Sparc bugs are due to
> >  the fact that sparc_analyze_prologue() cannot deal at all with clever
> >  assembler sequences, for example in cases where the save is deferred
> >  to somewhere past the beginning of the function which is also a
> >  valid compiler optimization.
> 
> Perhaps we should try to address those issues.  I never went through the
> trouble of making the sparc and sparc64 prologue analyzers any smarter,
> because I never encountered any code that made it fail.  This is probably
> because OpenBSD/sparc64 uses GCC 3.3.5 (and OpenBSD/sparc uses an even
> older compiler).  If you can post some disaambly of (real-life) prologues
> that the current unwinder doesn't handle, I'll happiliy turn them into
> testcases and try to improve the analyzer.

I think working on making the sparc prologue analyzer smarter is
a double edged sword personally.

On the one hand I could argue that dwarf2 unwind information solves
all of these problems.

On the other hand I know that without some kind of sophisticated
prologue analyzer we'll never have reasonable backtraces in things
like JIT compilers such as Mono, which is an incredibly hard piece
of code to debug because of this.  There is no symbol information
or function start/end markers available to gdb when there are JIT
compiled functions in the backtrace, so this is not an easy problem
to solve.

Anyways, here is an example of a code sequence that didn't work with
the current prologue analyzer:

<nanosleep>:    ld  [ %g7 + 0xc ], %g1
<nanosleep+4>:  cmp  %g1, 0
<nanosleep+8>:  bne  0x9e35c <__nanosleep_nocancel+24>
<__nanosleep_nocancel>: mov  0xf9, %g1
<__nanosleep_nocancel+4>:       ta  0x10
<__nanosleep_nocancel+8>:       bcs  0x9e394 <__syscall_error_handler>
<__nanosleep_nocancel+12>:      nop 
<__nanosleep_nocancel+16>:      retl 
<__nanosleep_nocancel+20>:      nop 
<__nanosleep_nocancel+24>:      save  %sp, -96, %sp
<__nanosleep_nocancel+28>:	call  0xed120 <__libc_enable_asynccancel>
<__nanosleep_nocancel+32>:      nop 
<__nanosleep_nocancel+36>:      mov  %o0, %l0
<__nanosleep_nocancel+40>:      mov  %i0, %o0
<__nanosleep_nocancel+44>:      mov  %i1, %o1
<__nanosleep_nocancel+48>:      mov  0xf9, %g1
<__nanosleep_nocancel+52>:      ta  0x10
<__nanosleep_nocancel+56>:      bcs  0x9e3bc <__syscall_error_handler2>
<__nanosleep_nocancel+60>:      mov  %o0, %l1
<__nanosleep_nocancel+64>:	call  0xed1c0 <__libc_disable_asynccancel>
<__nanosleep_nocancel+68>:      mov  %l0, %o0
<__nanosleep_nocancel+72>:      ret 
<__nanosleep_nocancel+76>:      restore  %g0, %l1, %o0
<__syscall_error_handler>:      save  %sp, -96, %sp
<__syscall_error_handler+4>:    sethi  %hi(0x1000), %l1
<__syscall_error_handler+8>:    sethi  %hi(0xb4c00), %l7
<__syscall_error_handler+12>:   call  0x11e460 <__sparc_get_pic_l7>
<__syscall_error_handler+16>:	add  %l7, 0x60, %l7 ! 0xb4c60 <re_compile_internal+3456>
<__syscall_error_handler+20>:   add  %l1, 0xd4, %l1
<__syscall_error_handler+24>:   ld  [ %l7 + %l1 ], %l1
<__syscall_error_handler2>:	call  0xed1c0 <__libc_disable_asynccancel>
<__syscall_error_handler2+4>:   mov  %l0, %o0
<__syscall_error_handler2+8>:   call  0x154354 <__errno_location@plt>
<__syscall_error_handler2+12>:  nop 
<__syscall_error_handler2+16>:  st  %l1, [ %o0 ]
<__syscall_error_handler2+20>:  ret 
<__syscall_error_handler2+24>:  restore  %g0, -1, %o0

But I hesitate to show this because the compiler can move save/restore
sequences to arbitrary places, and this makes the most obvious approaches
to handle this easy to fool.

For example, your first idea might be to walk from the first instruction
up to current_pc looking for a save, but this does not analyze the
control transfer instructions, what if current_pc is in a basic block
outside of the save/restore sequence?  You'll interpret things incorrectly
in that case.  You really have to analyze the control transfers to get
this right.  Example:

	.text
	.globl	func
func:
	cmp	%o0, 1
	be	1f
	 nop
	save	%sp, -96, %sp
	call	some_other_func
	 mov	%i0, %o0
	restore	%g0, %g0, %g0
1:	add	%o0, 5, %o0
	retl
	 nop

So if the PC were at "1", the naive analyzer would see the "save"
when scanning from func to PC and thing that we have a frame, when
in fact we're in the "frameless" part of the function.

Another case you'll run into problems with are PLT entries and that
trick we use to move the PC back to the first PLT entry where the save
is.  That causes problems for testcases such as recurse.exp when
emitting sparc v7 code and this the multiply in the test case expands
into a library call through the PLT.  When we try to step over the
multiply it hits the PLT and the frameless_p logic gets very confused.

This is a deep and dark passage and I've been down it before. :-)

My recommendation is:

1) For things like the above code sequence, depend upon dwarf2
   unwind information.  If the compiler starts to defer save/restore
   sequences in the same way, it will emit the proper dwarf2 information
   just like the glibc by-hand assembler above does.

2) For PLT entry sequences, each target knows what it's PLT entries
   look like so should provide the frame building logic or at least
   the helper functions to help the prologue analyzer decide what to
   do.

> >  Ok to apply?
> 
> Yes, thanks!

Thanks for reviewing, applied.


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

end of thread, other threads:[~2006-04-05 18:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-04-05  3:30 [PATCH]: Setup sparc32 to support dwarf2 unwind sniffer David S. Miller
2006-04-05  9:11 ` Mark Kettenis
2006-04-05 13:38   ` Daniel Jacobowitz
2006-04-05 18:54   ` David S. Miller

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