On Tue, Nov 22, 2011 at 12:12 PM, Lassi Tuura <[email protected]> wrote:
> The new patch looks good. I've tested on RHEL5-derived x86_64 system with
> GCC 4.6.1 build, and things look good in a couple of test applications I
> tried on.
Thanks!
> There's one new compilation warning in release mode, because 'uc' is only
> used in assert() in the new tdep_stash_frame(). Maybe protect the entire
> signal frame 'else if' body in tdep_stash_frame() #if UNW_DEBUG?
>
> ../../../libunwind/src/x86_64/Gstash_frame.c: In function
> '_ULx86_64_stash_frame':
> ../../../libunwind/src/x86_64/Gstash_frame.c:84:22: warning: unused variable
> 'uc' [-Wunused-variable]
The UNW_DEBUG could be true and assert() could still be empty.
How about this:
diff --git a/src/x86_64/Gstash_frame.c b/src/x86_64/Gstash_frame.c
index 92477e0..43695d6 100644
--- a/src/x86_64/Gstash_frame.c
+++ b/src/x86_64/Gstash_frame.c
@@ -81,11 +81,13 @@ tdep_stash_frame (struct dwarf_cursor *d, struct
dwarf_reg_state *rs)
their ucontext_t offsets. Confirm DWARF info agrees with the
offsets we expect. */
+#ifndef NDEBUG
const unw_word_t uc = c->sigcontext_addr;
assert (DWARF_GET_LOC(d->loc[RIP]) == uc + UC_MCONTEXT_GREGS_RIP);
assert (DWARF_GET_LOC(d->loc[RBP]) == uc + UC_MCONTEXT_GREGS_RBP);
assert (DWARF_GET_LOC(d->loc[RSP]) == uc + UC_MCONTEXT_GREGS_RSP);
+#endif
Debug (4, " sigreturn frame\n");
}
I've also changed the assert slightly (moving 'uc' to the right): I think
it's slightly easier to understand that way.
Full patch attached.
Thanks,
--
Paul Pluzhnikov
diff --git a/src/x86_64/Gos-linux.c b/src/x86_64/Gos-linux.c
index a315ea1..a0ecce2 100644
--- a/src/x86_64/Gos-linux.c
+++ b/src/x86_64/Gos-linux.c
@@ -64,10 +64,9 @@ tdep_reuse_frame (struct dwarf_cursor *dw, struct
dwarf_reg_state *rs)
c->sigcontext_format = rs->signal_frame;
if (c->sigcontext_format == X86_64_SCF_LINUX_RT_SIGFRAME)
{
- /* Rest will be filled by tdep_stash_frame(), save what it needs. */
c->frame_info.frame_type = UNW_X86_64_FRAME_SIGRETURN;
- c->frame_info.cfa_reg_offset = -1;
- c->frame_info.cfa_reg_rsp = -1;
+ /* Offset from cfa to ucontext_t in signal frame. */
+ c->frame_info.cfa_reg_offset = 0;
c->sigcontext_addr = dw->cfa;
}
else
diff --git a/src/x86_64/Gstash_frame.c b/src/x86_64/Gstash_frame.c
index 2f81cc0..43695d6 100644
--- a/src/x86_64/Gstash_frame.c
+++ b/src/x86_64/Gstash_frame.c
@@ -74,19 +74,22 @@ tdep_stash_frame (struct dwarf_cursor *d, struct
dwarf_reg_state *rs)
Debug (4, " standard frame\n");
}
- /* Signal frame was detected via augmentation in tdep_fetch_frame()
- and partially filled in tdep_reuse_frame(). Now that we have
- the delta between inner and outer CFAs available to use, fill in
- the offsets for CFA and stored registers. We don't have space
- for RIP, it's location is calculated relative to RBP location. */
+ /* Signal frame was detected via augmentation in tdep_fetch_frame() */
else if (f->frame_type == UNW_X86_64_FRAME_SIGRETURN)
{
- assert (f->cfa_reg_offset == -1);
- f->cfa_reg_offset = d->cfa - c->sigcontext_addr;
- f->rbp_cfa_offset = DWARF_GET_LOC(d->loc[RBP]) - d->cfa;
- f->rsp_cfa_offset = DWARF_GET_LOC(d->loc[RSP]) - d->cfa;
- Debug (4, " sigreturn frame rbpoff %d rspoff %d\n",
- f->rbp_cfa_offset, f->rsp_cfa_offset);
+ /* Later we are going to fish out {RBP,RSP,RIP} from sigcontext via
+ their ucontext_t offsets. Confirm DWARF info agrees with the
+ offsets we expect. */
+
+#ifndef NDEBUG
+ const unw_word_t uc = c->sigcontext_addr;
+
+ assert (DWARF_GET_LOC(d->loc[RIP]) == uc + UC_MCONTEXT_GREGS_RIP);
+ assert (DWARF_GET_LOC(d->loc[RBP]) == uc + UC_MCONTEXT_GREGS_RBP);
+ assert (DWARF_GET_LOC(d->loc[RSP]) == uc + UC_MCONTEXT_GREGS_RSP);
+#endif
+
+ Debug (4, " sigreturn frame\n");
}
/* PLT and guessed RBP-walked frames are handled in unw_step(). */
diff --git a/src/x86_64/Gtrace.c b/src/x86_64/Gtrace.c
index f10737f..2d78729 100644
--- a/src/x86_64/Gtrace.c
+++ b/src/x86_64/Gtrace.c
@@ -35,12 +35,6 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE. */
/* Initial hash table size. Table expands by 2 bits (times four). */
#define HASH_MIN_BITS 14
-/* There's not enough space to store RIP's location in a signal
- frame, but we can calculate it relative to RBP's (or RSP's)
- position in mcontext structure. Note we don't want to use
- the UC_MCONTEXT_GREGS_* directly since we rely on DWARF info. */
-#define dRIP (UC_MCONTEXT_GREGS_RIP - UC_MCONTEXT_GREGS_RBP)
-
typedef struct
{
unw_tdep_frame_t *frames;
@@ -496,16 +490,13 @@ tdep_trace (unw_cursor_t *cursor, void **buffer, int
*size)
break;
case UNW_X86_64_FRAME_SIGRETURN:
- /* Advance standard signal frame, whose CFA points above saved
- registers (ucontext) among other things. We know the info
- is stored at some unknown constant offset off inner frame's
- CFA. We determine the actual offset from DWARF unwind info. */
- cfa = cfa + f->cfa_reg_offset;
- ACCESS_MEM_FAST(ret, c->validate, d, cfa + f->rbp_cfa_offset + dRIP,
rip);
+ cfa = cfa + f->cfa_reg_offset; /* cfa now points to ucontext_t. */
+
+ ACCESS_MEM_FAST(ret, c->validate, d, cfa + UC_MCONTEXT_GREGS_RIP, rip);
if (likely(ret >= 0))
- ACCESS_MEM_FAST(ret, c->validate, d, cfa + f->rbp_cfa_offset, rbp);
+ ACCESS_MEM_FAST(ret, c->validate, d, cfa + UC_MCONTEXT_GREGS_RBP, rbp);
if (likely(ret >= 0))
- ACCESS_MEM_FAST(ret, c->validate, d, cfa + f->rsp_cfa_offset, rsp);
+ ACCESS_MEM_FAST(ret, c->validate, d, cfa + UC_MCONTEXT_GREGS_RSP, rsp);
/* Resume stack at signal restoration point. The stack is not
necessarily continuous here, especially with sigaltstack(). */
_______________________________________________
Libunwind-devel mailing list
[email protected]
https://lists.nongnu.org/mailman/listinfo/libunwind-devel
