* [rfa/ppc] prologue parser tweaks
@ 2004-03-16 22:28 Andrew Cagney
2004-03-19 0:09 ` Kevin Buettner
2004-03-19 0:09 ` Andrew Cagney
0 siblings, 2 replies; 5+ messages in thread
From: Andrew Cagney @ 2004-03-16 22:28 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 524 bytes --]
Hello,
Attached are two tweaks to the PPC skip_prologue method so that it works
better with glibc:
- handle glibc syscall
Without this an unwinder won't be able to find its way out of a system
call - it needs to figure out the location of a local label, see comment
for details.
- handle PIC code where the LR appears to be saved twice
It appears that skip_prologue code tried to handle this but missed an
edge case. Without this the wrong LR gets used when trying to unwind
out of GLIBC.
Ok for mainline?
Andrew
[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 3171 bytes --]
2004-03-07 Andrew Cagney <cagney@redhat.com>
* rs6000-tdep.c: Add field "func_start".
(skip_prologue): New variable num_skip_syscall_insn, use to skip
over first half of a GNU/Linux syscall and update "func_start".
Record only the first LR save, use to skip over PIC code.
Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.183
diff -u -r1.183 rs6000-tdep.c
--- rs6000-tdep.c 2 Mar 2004 02:20:25 -0000 1.183
+++ rs6000-tdep.c 16 Mar 2004 22:08:24 -0000
@@ -65,6 +65,7 @@
struct rs6000_framedata
{
+ CORE_ADDR func_start; /* true function start */
int offset; /* total size of frame --- the distance
by which we decrement sp to allocate
the frame */
@@ -502,6 +503,7 @@
int minimal_toc_loaded = 0;
int prev_insn_was_prologue_insn = 1;
int num_skip_non_prologue_insns = 0;
+ int num_skip_syscall_insn = 0;
const struct bfd_arch_info *arch_info = gdbarch_bfd_arch_info (current_gdbarch);
struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
@@ -521,6 +523,7 @@
lim_pc = refine_prologue_limit (pc, lim_pc);
memset (fdata, 0, sizeof (struct rs6000_framedata));
+ fdata->func_start = pc;
fdata->saved_gpr = -1;
fdata->saved_fpr = -1;
fdata->saved_vr = -1;
@@ -548,6 +551,70 @@
if (target_read_memory (pc, buf, 4))
break;
op = extract_signed_integer (buf, 4);
+
+ /* A PPC64 GNU/Linux system call function starts with a
+ non-threaded fast-path, only when that fails is a stack frame
+ created, treat it as several functions:
+ nptl/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep-cancel.h
+
+ *INDENT-OFF*
+ NAME:
+ SINGLE_THREAD_P
+ bne- .Lpseudo_cancel
+ __NAME_nocancel:
+ li r0,162
+ sc
+ bnslr+
+ b 0x7fe014ef64 <.__syscall_error>
+ Lpseudo_cancel:
+ stdu r1,-128(r1)
+ ...
+ *INDENT-ON* */
+
+ if (((op & 0xffff0000) == 0x38000000 /* li r0,N */
+ && pc == fdata->func_start + 0)
+ || (op == 0x44000002 /* sc */
+ && pc == fdata->func_start + 4
+ && num_skip_syscall_insn == 1)
+ || (op == 0x4ca30020 /* bnslr+ */
+ && pc == fdata->func_start + 8
+ && num_skip_syscall_insn == 2))
+ {
+ num_skip_syscall_insn++;
+ continue;
+ }
+ else if ((op & 0xfc000003) == 0x48000000 /* b __syscall_error */
+ && pc == fdata->func_start + 12
+ && num_skip_syscall_insn == 3)
+ {
+ num_skip_syscall_insn++;
+ fdata->func_start = pc;
+ continue;
+ }
+
+ if ((op & 0xfc1fffff) == 0x7c0802a6)
+ { /* mflr Rx */
+ /* Since shared library / PIC code, which needs to get its
+ address at runtime, can appear to save more than one link
+ register vis:
+
+ *INDENT-OFF*
+ stwu r1,-304(r1)
+ mflr r3
+ bl 0xff570d0 (blrl)
+ stw r30,296(r1)
+ mflr r30
+ stw r31,300(r1)
+ stw r3,308(r1);
+ ...
+ *INDENT-ON*
+
+ remember just the first one, but skip over additional
+ ones. */
+ if (lr_reg < 0)
+ lr_reg = (op & 0x03e00000);
+ continue;
+ }
if ((op & 0xfc1fffff) == 0x7c0802a6)
{ /* mflr Rx */
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [rfa/ppc] prologue parser tweaks
2004-03-19 0:09 ` Kevin Buettner
@ 2004-03-18 16:15 ` Kevin Buettner
2004-03-19 0:10 ` Andrew Cagney
1 sibling, 0 replies; 5+ messages in thread
From: Kevin Buettner @ 2004-03-18 16:15 UTC (permalink / raw)
To: gdb-patches
On Tue, 16 Mar 2004 17:28:54 -0500
Andrew Cagney <cagney@gnu.org> wrote:
> * rs6000-tdep.c: Add field "func_start".
If this part goes in, it should be:
* rs6000-tdep.c (struct rs6000_framedata): Add field "func_start".
> (skip_prologue): New variable num_skip_syscall_insn, use to skip
> over first half of a GNU/Linux syscall and update "func_start".
> Record only the first LR save, use to skip over PIC code.
>
> Index: rs6000-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
> retrieving revision 1.183
> diff -u -r1.183 rs6000-tdep.c
> --- rs6000-tdep.c 2 Mar 2004 02:20:25 -0000 1.183
> +++ rs6000-tdep.c 16 Mar 2004 22:08:24 -0000
> @@ -65,6 +65,7 @@
>
> struct rs6000_framedata
> {
> + CORE_ADDR func_start; /* true function start */
Why do you need to add this field? Do you have some other patches which
depend on it?
> int offset; /* total size of frame --- the distance
> by which we decrement sp to allocate
> the frame */
> @@ -502,6 +503,7 @@
> int minimal_toc_loaded = 0;
> int prev_insn_was_prologue_insn = 1;
> int num_skip_non_prologue_insns = 0;
> + int num_skip_syscall_insn = 0;
Could you add a brief comment indicating that this is a state variable
for the PPC64 GNU/Linux system call tests? I don't want someone
to inadvertently start using it for some other purpose. Hmm, even
better might be to name it something like ``ppc64_linux_syscall_state''.
With a name like that, it's unlikely that it'll inadvertently get used
for some other purpose.
> const struct bfd_arch_info *arch_info = gdbarch_bfd_arch_info (current_gdbarch);
> struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
>
> @@ -521,6 +523,7 @@
> lim_pc = refine_prologue_limit (pc, lim_pc);
>
> memset (fdata, 0, sizeof (struct rs6000_framedata));
> + fdata->func_start = pc;
> fdata->saved_gpr = -1;
> fdata->saved_fpr = -1;
> fdata->saved_vr = -1;
> @@ -548,6 +551,70 @@
[...]
> + if (((op & 0xffff0000) == 0x38000000 /* li r0,N */
> + && pc == fdata->func_start + 0)
It seems to me that substituting ``orig_pc'' in place of ``fdata->func_start''
will work, wont't it? E.g.:
&& pc == orig_pc + 0)
> + || (op == 0x44000002 /* sc */
> + && pc == fdata->func_start + 4
&& pc == orig_pc + 4
??
> + && num_skip_syscall_insn == 1)
> + || (op == 0x4ca30020 /* bnslr+ */
> + && pc == fdata->func_start + 8
&& pc == orig_pc + 8
??
> + && num_skip_syscall_insn == 2))
> + {
> + num_skip_syscall_insn++;
> + continue;
> + }
> + else if ((op & 0xfc000003) == 0x48000000 /* b __syscall_error */
> + && pc == fdata->func_start + 12
&& pc == orig_pc + 12
> + && num_skip_syscall_insn == 3)
> + {
> + num_skip_syscall_insn++;
> + fdata->func_start = pc;
What's this about? Where else does fdata->func_start get used?
> + continue;
> + }
I'd like to see the above test moved down a ways in the sequence of
tests. Hmm, maybe it could go right before the AltiVec tests?
> +
> + if ((op & 0xfc1fffff) == 0x7c0802a6)
> + { /* mflr Rx */
> + /* Since shared library / PIC code, which needs to get its
> + address at runtime, can appear to save more than one link
> + register vis:
> +
> + *INDENT-OFF*
> + stwu r1,-304(r1)
> + mflr r3
> + bl 0xff570d0 (blrl)
> + stw r30,296(r1)
> + mflr r30
> + stw r31,300(r1)
> + stw r3,308(r1);
> + ...
> + *INDENT-ON*
> +
> + remember just the first one, but skip over additional
> + ones. */
> + if (lr_reg < 0)
> + lr_reg = (op & 0x03e00000);
> + continue;
> + }
>
> if ((op & 0xfc1fffff) == 0x7c0802a6)
> { /* mflr Rx */
The above is okay so long as you remove the old "mflr Rx" test.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [rfa/ppc] prologue parser tweaks
2004-03-16 22:28 [rfa/ppc] prologue parser tweaks Andrew Cagney
2004-03-19 0:09 ` Kevin Buettner
@ 2004-03-19 0:09 ` Andrew Cagney
1 sibling, 0 replies; 5+ messages in thread
From: Andrew Cagney @ 2004-03-19 0:09 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 524 bytes --]
Hello,
Attached are two tweaks to the PPC skip_prologue method so that it works
better with glibc:
- handle glibc syscall
Without this an unwinder won't be able to find its way out of a system
call - it needs to figure out the location of a local label, see comment
for details.
- handle PIC code where the LR appears to be saved twice
It appears that skip_prologue code tried to handle this but missed an
edge case. Without this the wrong LR gets used when trying to unwind
out of GLIBC.
Ok for mainline?
Andrew
[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 3171 bytes --]
2004-03-07 Andrew Cagney <cagney@redhat.com>
* rs6000-tdep.c: Add field "func_start".
(skip_prologue): New variable num_skip_syscall_insn, use to skip
over first half of a GNU/Linux syscall and update "func_start".
Record only the first LR save, use to skip over PIC code.
Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.183
diff -u -r1.183 rs6000-tdep.c
--- rs6000-tdep.c 2 Mar 2004 02:20:25 -0000 1.183
+++ rs6000-tdep.c 16 Mar 2004 22:08:24 -0000
@@ -65,6 +65,7 @@
struct rs6000_framedata
{
+ CORE_ADDR func_start; /* true function start */
int offset; /* total size of frame --- the distance
by which we decrement sp to allocate
the frame */
@@ -502,6 +503,7 @@
int minimal_toc_loaded = 0;
int prev_insn_was_prologue_insn = 1;
int num_skip_non_prologue_insns = 0;
+ int num_skip_syscall_insn = 0;
const struct bfd_arch_info *arch_info = gdbarch_bfd_arch_info (current_gdbarch);
struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
@@ -521,6 +523,7 @@
lim_pc = refine_prologue_limit (pc, lim_pc);
memset (fdata, 0, sizeof (struct rs6000_framedata));
+ fdata->func_start = pc;
fdata->saved_gpr = -1;
fdata->saved_fpr = -1;
fdata->saved_vr = -1;
@@ -548,6 +551,70 @@
if (target_read_memory (pc, buf, 4))
break;
op = extract_signed_integer (buf, 4);
+
+ /* A PPC64 GNU/Linux system call function starts with a
+ non-threaded fast-path, only when that fails is a stack frame
+ created, treat it as several functions:
+ nptl/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep-cancel.h
+
+ *INDENT-OFF*
+ NAME:
+ SINGLE_THREAD_P
+ bne- .Lpseudo_cancel
+ __NAME_nocancel:
+ li r0,162
+ sc
+ bnslr+
+ b 0x7fe014ef64 <.__syscall_error>
+ Lpseudo_cancel:
+ stdu r1,-128(r1)
+ ...
+ *INDENT-ON* */
+
+ if (((op & 0xffff0000) == 0x38000000 /* li r0,N */
+ && pc == fdata->func_start + 0)
+ || (op == 0x44000002 /* sc */
+ && pc == fdata->func_start + 4
+ && num_skip_syscall_insn == 1)
+ || (op == 0x4ca30020 /* bnslr+ */
+ && pc == fdata->func_start + 8
+ && num_skip_syscall_insn == 2))
+ {
+ num_skip_syscall_insn++;
+ continue;
+ }
+ else if ((op & 0xfc000003) == 0x48000000 /* b __syscall_error */
+ && pc == fdata->func_start + 12
+ && num_skip_syscall_insn == 3)
+ {
+ num_skip_syscall_insn++;
+ fdata->func_start = pc;
+ continue;
+ }
+
+ if ((op & 0xfc1fffff) == 0x7c0802a6)
+ { /* mflr Rx */
+ /* Since shared library / PIC code, which needs to get its
+ address at runtime, can appear to save more than one link
+ register vis:
+
+ *INDENT-OFF*
+ stwu r1,-304(r1)
+ mflr r3
+ bl 0xff570d0 (blrl)
+ stw r30,296(r1)
+ mflr r30
+ stw r31,300(r1)
+ stw r3,308(r1);
+ ...
+ *INDENT-ON*
+
+ remember just the first one, but skip over additional
+ ones. */
+ if (lr_reg < 0)
+ lr_reg = (op & 0x03e00000);
+ continue;
+ }
if ((op & 0xfc1fffff) == 0x7c0802a6)
{ /* mflr Rx */
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [rfa/ppc] prologue parser tweaks
2004-03-16 22:28 [rfa/ppc] prologue parser tweaks Andrew Cagney
@ 2004-03-19 0:09 ` Kevin Buettner
2004-03-18 16:15 ` Kevin Buettner
2004-03-19 0:10 ` Andrew Cagney
2004-03-19 0:09 ` Andrew Cagney
1 sibling, 2 replies; 5+ messages in thread
From: Kevin Buettner @ 2004-03-19 0:09 UTC (permalink / raw)
To: gdb-patches
On Tue, 16 Mar 2004 17:28:54 -0500
Andrew Cagney <cagney@gnu.org> wrote:
> * rs6000-tdep.c: Add field "func_start".
If this part goes in, it should be:
* rs6000-tdep.c (struct rs6000_framedata): Add field "func_start".
> (skip_prologue): New variable num_skip_syscall_insn, use to skip
> over first half of a GNU/Linux syscall and update "func_start".
> Record only the first LR save, use to skip over PIC code.
>
> Index: rs6000-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
> retrieving revision 1.183
> diff -u -r1.183 rs6000-tdep.c
> --- rs6000-tdep.c 2 Mar 2004 02:20:25 -0000 1.183
> +++ rs6000-tdep.c 16 Mar 2004 22:08:24 -0000
> @@ -65,6 +65,7 @@
>
> struct rs6000_framedata
> {
> + CORE_ADDR func_start; /* true function start */
Why do you need to add this field? Do you have some other patches which
depend on it?
> int offset; /* total size of frame --- the distance
> by which we decrement sp to allocate
> the frame */
> @@ -502,6 +503,7 @@
> int minimal_toc_loaded = 0;
> int prev_insn_was_prologue_insn = 1;
> int num_skip_non_prologue_insns = 0;
> + int num_skip_syscall_insn = 0;
Could you add a brief comment indicating that this is a state variable
for the PPC64 GNU/Linux system call tests? I don't want someone
to inadvertently start using it for some other purpose. Hmm, even
better might be to name it something like ``ppc64_linux_syscall_state''.
With a name like that, it's unlikely that it'll inadvertently get used
for some other purpose.
> const struct bfd_arch_info *arch_info = gdbarch_bfd_arch_info (current_gdbarch);
> struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
>
> @@ -521,6 +523,7 @@
> lim_pc = refine_prologue_limit (pc, lim_pc);
>
> memset (fdata, 0, sizeof (struct rs6000_framedata));
> + fdata->func_start = pc;
> fdata->saved_gpr = -1;
> fdata->saved_fpr = -1;
> fdata->saved_vr = -1;
> @@ -548,6 +551,70 @@
[...]
> + if (((op & 0xffff0000) == 0x38000000 /* li r0,N */
> + && pc == fdata->func_start + 0)
It seems to me that substituting ``orig_pc'' in place of ``fdata->func_start''
will work, wont't it? E.g.:
&& pc == orig_pc + 0)
> + || (op == 0x44000002 /* sc */
> + && pc == fdata->func_start + 4
&& pc == orig_pc + 4
??
> + && num_skip_syscall_insn == 1)
> + || (op == 0x4ca30020 /* bnslr+ */
> + && pc == fdata->func_start + 8
&& pc == orig_pc + 8
??
> + && num_skip_syscall_insn == 2))
> + {
> + num_skip_syscall_insn++;
> + continue;
> + }
> + else if ((op & 0xfc000003) == 0x48000000 /* b __syscall_error */
> + && pc == fdata->func_start + 12
&& pc == orig_pc + 12
> + && num_skip_syscall_insn == 3)
> + {
> + num_skip_syscall_insn++;
> + fdata->func_start = pc;
What's this about? Where else does fdata->func_start get used?
> + continue;
> + }
I'd like to see the above test moved down a ways in the sequence of
tests. Hmm, maybe it could go right before the AltiVec tests?
> +
> + if ((op & 0xfc1fffff) == 0x7c0802a6)
> + { /* mflr Rx */
> + /* Since shared library / PIC code, which needs to get its
> + address at runtime, can appear to save more than one link
> + register vis:
> +
> + *INDENT-OFF*
> + stwu r1,-304(r1)
> + mflr r3
> + bl 0xff570d0 (blrl)
> + stw r30,296(r1)
> + mflr r30
> + stw r31,300(r1)
> + stw r3,308(r1);
> + ...
> + *INDENT-ON*
> +
> + remember just the first one, but skip over additional
> + ones. */
> + if (lr_reg < 0)
> + lr_reg = (op & 0x03e00000);
> + continue;
> + }
>
> if ((op & 0xfc1fffff) == 0x7c0802a6)
> { /* mflr Rx */
The above is okay so long as you remove the old "mflr Rx" test.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [rfa/ppc] prologue parser tweaks
2004-03-19 0:09 ` Kevin Buettner
2004-03-18 16:15 ` Kevin Buettner
@ 2004-03-19 0:10 ` Andrew Cagney
1 sibling, 0 replies; 5+ messages in thread
From: Andrew Cagney @ 2004-03-19 0:10 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 891 bytes --]
>>> +
>>> + if ((op & 0xfc1fffff) == 0x7c0802a6)
>>> + { /* mflr Rx */
>>> + /* Since shared library / PIC code, which needs to get its
>>> + address at runtime, can appear to save more than one link
>>> + register vis:
>>> +
>>> + *INDENT-OFF*
>>> + stwu r1,-304(r1)
>>> + mflr r3
>>> + bl 0xff570d0 (blrl)
>>> + stw r30,296(r1)
>>> + mflr r30
>>> + stw r31,300(r1)
>>> + stw r3,308(r1);
>>> + ...
>>> + *INDENT-ON*
>>> +
>>> + remember just the first one, but skip over additional
>>> + ones. */
>>> + if (lr_reg < 0)
>>> + lr_reg = (op & 0x03e00000);
>>> + continue;
>>> + }
>>>
>>> if ((op & 0xfc1fffff) == 0x7c0802a6)
>>> { /* mflr Rx */
>
>
> The above is okay so long as you remove the old "mflr Rx" test.
I've checked this part in, I'll follow up the other part later.
Andrew
[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 1104 bytes --]
2004-03-18 Andrew Cagney <cagney@redhat.com>
* rs6000-tdep.c (skip_prologue): Record only the first LR save.
Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.184
diff -u -r1.184 rs6000-tdep.c
--- rs6000-tdep.c 15 Mar 2004 21:21:01 -0000 1.184
+++ rs6000-tdep.c 18 Mar 2004 20:14:38 -0000
@@ -551,9 +551,26 @@
if ((op & 0xfc1fffff) == 0x7c0802a6)
{ /* mflr Rx */
- lr_reg = (op & 0x03e00000);
- continue;
+ /* Since shared library / PIC code, which needs to get its
+ address at runtime, can appear to save more than one link
+ register vis:
+
+ *INDENT-OFF*
+ stwu r1,-304(r1)
+ mflr r3
+ bl 0xff570d0 (blrl)
+ stw r30,296(r1)
+ mflr r30
+ stw r31,300(r1)
+ stw r3,308(r1);
+ ...
+ *INDENT-ON*
+ remember just the first one, but skip over additional
+ ones. */
+ if (lr_reg < 0)
+ lr_reg = (op & 0x03e00000);
+ continue;
}
else if ((op & 0xfc1fffff) == 0x7c000026)
{ /* mfcr Rx */
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2004-03-19 0:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-03-16 22:28 [rfa/ppc] prologue parser tweaks Andrew Cagney
2004-03-19 0:09 ` Kevin Buettner
2004-03-18 16:15 ` Kevin Buettner
2004-03-19 0:10 ` Andrew Cagney
2004-03-19 0:09 ` Andrew Cagney
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox