On Fri, Nov 11, 2011 at 1:41 PM, Lassi Tuura <[email protected]> wrote:

> Yes, you are right, I noticed the same. But we probably shouldn't store
> the difference to the original stack but to the ucontext_t which should
> be in nearby offset, on the signal stack.

In fact, AFAICT for Linux ucontext_t is the very last thing copied onto
the stack before invoking the signal handler. That is, when signal handler
start running, the stack layout is:

  ... higher addresses ...
         ucontext
  CFA->
         __restore_rt (== pretcode in rt_sigframe from
                       linux-2.6/arch/x86/include/asm/sigframe.h)
  SP ->
        ... sighandler runs on this stack.

  ... lower addresses ...

This makes it very convenient to find ucontext from the CFA.

Attached patch re-tested on Linux/x86_64, no new failures.

Thanks,

P.S. test-setjmp is failing for me (before or after the patch).
When I enable assertions (to confirm my new assertions are correct), I see:

  lt-test-setjmp: ../../src/dwarf/Gparser.c:754: apply_reg_state: \
    Assertion `rs->reg[17].where == DWARF_WHERE_EXPR' failed.

which likely explains that failure.

-- 
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..da60613 100644
--- a/src/x86_64/Gstash_frame.c
+++ b/src/x86_64/Gstash_frame.c
@@ -74,19 +74,20 @@ 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.  */
+
+    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);
+
+    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

Reply via email to