public inbox for frysk@sourceware.org
 help / color / mirror / Atom feed
* [patch] Stack based unwind fallback for plt entries on x86
@ 2008-03-12 13:26 Mark Wielaard
  2008-03-19 21:50 ` [patch] Stack based unwind fallback for plt entries on x86_64 Mark Wielaard
  0 siblings, 1 reply; 2+ messages in thread
From: Mark Wielaard @ 2008-03-12 13:26 UTC (permalink / raw)
  To: frysk

[-- Attachment #1: Type: text/plain, Size: 3402 bytes --]

Hi,

This patch makes the TestLibFunctionStepFrame PASS (but see below) and
makes nexting over shared library function calls (through plt entries)
work on x86.

To recap the problem. When there is no debug/unwind_frame info libunwind
needs to guess how to do a step. The final fallback on x86 is using the
frame pointer (ESB). There are two problems with this. First code
compiled with -fomit-frame-pointer doesn't have frame pointers, so then
we are chasing a possibly ghost frame. Second when stepping onto a plt
entry there is no real frame setup, the plt entry is kind of a jump
point to the real function to be called (and facilitates invoking the
dynamic linker of course to resolve the "jump slot" first). This means
that if you try to follow the frame pointer you are actually following
into the frame above the "plt frame". This confuses the SteppingEngine
terribly. This isn't a direct problem on x86_64 (at least for the
SteppingEngine at this time) because libunwind never tries to do any
frame pointer tricks there. So on x86_64 you currently cannot unwind,
which is better than getting the wrong outer frame. (On x86_64 nexting
works, but TestLibFunctionStepFrame doesn't PASS.)

There are a couple of properties of x86 plt entries that help with
resolving this issue. 1) They only occur as inner frames (think of them
as tail-calls). 2) They are always invoked with a CALL instruction (as
if it was a real function call). 3) If they are resolved they are just
one JMP instruction, with the return address on the stack, otherwise
they are a JMP, two PUSHs (selector and got table address) and a final
JUMP (into ld). The patch tries to recognize this situation.

Because we don't have much info to go on, we don't have any of the elf
section info inside libunwind, it is a bit heuristic at the moment. It
probes the stack for the pattern mentioned above, pulls out the return
address and checks if it comes from a CALL instruction, then adjusts the
stack and ip addresses for the cursor accordingly. I don't know if
upstream will like this solution (it contains one probe into an address
to see what the instruction there is, that could possibly not exist,
which is fine for us since that is just an error access and we fall
back, but when using libunwind in-process this could possibly be fatal
with a really bad address).

One change was made to the test. TestLibFunctionStepFrame used to step
through all of the dynamic loader and made sure all possible
instructions inside had a good backtrace. But there is one point that
seems to have bad unwind info (and of dl_fixup, the last ret
instruction). I filed #5917 to investigate this more. The test has been
adjusted to just check the first few instructions of the actual PLT and
ld and/or actual function entry.
                                      
2008-03-12  Mark Wielaard  <mwielaard@redhat.com>

    * TestLibFunctionStepFrame.java: Mark only unresolved on x86_64
    and ppc. Only check first 24 steps (bug #5917). Tighter check for
    main and foo order.

2008-03-12  Mark Wielaard  <mwielaard@redhat.com>

    * Frame.java (toString): New method.

2007-03-12  Mark Wielaard  <mwielaard@redhat.com>

        * src/x86/Gstep.c (is_call_instr_at): New function.
        (init_stack_based_ret): New function.
        (unw_step): Try stack based unwind with call instr, before
        fallback to frame pointer.

Committed,

Mark

[-- Attachment #2: plt.patch --]
[-- Type: text/x-patch, Size: 8931 bytes --]

diff --git a/frysk-core/frysk/stack/ChangeLog b/frysk-core/frysk/stack/ChangeLog
index 8ff178e..47a781f 100644
--- a/frysk-core/frysk/stack/ChangeLog
+++ b/frysk-core/frysk/stack/ChangeLog
@@ -1,3 +1,13 @@
+2008-03-12  Mark Wielaard  <mwielaard@redhat.com>
+
+	* TestLibFunctionStepFrame.java: Mark only unresolved on x86_64
+	and ppc. Only check first 24 steps (bug #5917). Tighter check for
+	main and foo order.
+	
+2008-03-12  Mark Wielaard  <mwielaard@redhat.com>
+
+	* Frame.java (toString): New method.
+	
 2008-03-10  Mark Wielaard  <mwielaard@redhat.com>
 
 	* TestLibFunctionStepFrame.java: New test for bug #5259.
diff --git a/frysk-core/frysk/stack/Frame.java b/frysk-core/frysk/stack/Frame.java
index e676008..bee5511 100644
--- a/frysk-core/frysk/stack/Frame.java
+++ b/frysk-core/frysk/stack/Frame.java
@@ -1,6 +1,6 @@
 // This file is part of the program FRYSK.
 //
-// Copyright 2007, Red Hat Inc.
+// Copyright 2007, 2008, Red Hat Inc.
 //
 // FRYSK is free software; you can redistribute it and/or modify it
 // under the terms of the GNU General Public License as published by
@@ -41,6 +41,7 @@ package frysk.stack;
 
 import java.io.File;
 import java.io.PrintWriter;
+import java.io.StringWriter;
 
 import lib.dwfl.Dwfl;
 import lib.dwfl.DwflModule;
@@ -192,7 +193,22 @@ public abstract class Frame {
 	    return "Unknown";
 	}
     }
-  
+
+    /**
+     * Returns a plain string representation if this frame.
+     * This is similar to the result of calling <code>toPrint()</code>
+     * with both printParameters and fullPaths set to false.
+     */
+    public String toString() {
+         StringWriter sw = new StringWriter();
+         PrintWriter pw = new PrintWriter(sw);
+         pw.print(this.getClass().getName());
+         pw.print('[');
+         toPrint(pw, false, false);
+         pw.print(']');
+         pw.flush();
+         return sw.toString();
+    }
 
     /**
      * Extracts OFFSET:LENGTH bytes of REGISTER storing them from
diff --git a/frysk-core/frysk/stack/TestLibFunctionStepFrame.java b/frysk-core/frysk/stack/TestLibFunctionStepFrame.java
index f1f8c59..d71f5ac 100644
--- a/frysk-core/frysk/stack/TestLibFunctionStepFrame.java
+++ b/frysk-core/frysk/stack/TestLibFunctionStepFrame.java
@@ -67,7 +67,7 @@ public class TestLibFunctionStepFrame
 {
   public void testStepIntoLibFunctionCall()
   {
-    if (unresolved(5259))
+    if (unresolvedOnx8664(5259) || unresolvedOnPPC(5259))
       return;
 
     String source = Config.getRootSrcDir()
@@ -116,7 +116,7 @@ public class TestLibFunctionStepFrame
     // We are at the first plt intruction (for the first time).
     long firstPLTAddress = task.getPC();
     Frame frame = StackFactory.createFrame(task);
-    assertFooAndMainOuterFrames("At first PLT address", frame);
+    assertFooAndMainOuterFrames("First entry in PLT", frame);
 
     // Count your steps. And bail out when there are too many.
     int steps = 1;
@@ -125,12 +125,15 @@ public class TestLibFunctionStepFrame
     while (currentPC != addressLast && steps < 1000)
       {
 	task.requestUnblock(this);
-	assertRunUntilStop("Do step: " + steps);
+	assertRunUntilStop("Do step: "
+			   + (seenSecondCall ? "second" : "")
+			   + steps);
 
 	currentPC = task.getPC();
 	if (currentPC == address2)
 	  {
 	    seenSecondCall = true;
+	    steps = 1;
 	    // When we step we should be at the first PLTAddress again.
 	    // One step and we should be at the first PLT instruction.
 	    task.requestUnblock(this);
@@ -139,14 +142,18 @@ public class TestLibFunctionStepFrame
 	    assertEquals("Second time in PLT", currentPC, firstPLTAddress);
 
 	    frame = StackFactory.createFrame(task);
-	    assertFooAndMainOuterFrames("Second time in PLT", frame);
+	    assertFooAndMainOuterFrames("Second entry in PLT", frame);
 	  }
-	else if (currentPC != addressLast)
+	else if (currentPC != addressLast && steps < 24)
 	  {
+	    // Only check first 24
 	    frame = StackFactory.createFrame(task);
-	    assertFooAndMainOuterFrames("Stepping, #" + steps
+	    assertFooAndMainOuterFrames("Stepping "
+					+ (seenSecondCall ? "second" : "")
+					+ ", #" + steps
 					+ " through (plt) call", frame);
 	  }
+	steps++;
       }
 
     assertTrue("less than a thousand steps", steps < 1000);
@@ -167,6 +174,7 @@ public class TestLibFunctionStepFrame
     assertTrue(message + " first inner frame should not be foo or main", ok);
 
     boolean foo_seen = false;
+    boolean main_seen = false;
     Frame outer = frame.getOuter();
     while (ok && outer != null)
       {
@@ -186,7 +194,11 @@ public class TestLibFunctionStepFrame
 
 	boolean sym_is_main = name.indexOf("main") != -1;
 	if (foo_seen && sym_is_main)
-	  break;
+	  {
+	    // Hurray done!
+	    main_seen = true;
+	    break;
+	  }
 
 	if (! foo_seen && sym_is_main)
 	  {
@@ -199,7 +211,7 @@ public class TestLibFunctionStepFrame
 	outer = outer.getOuter();
       }
 
-    ok = outer != null;
+    ok = ok && foo_seen && main_seen && outer != null;
     if (! ok)
       printBacktrace(frame);
     assertTrue(message
diff --git a/frysk-imports/libunwind/ChangeLog b/frysk-imports/libunwind/ChangeLog
index 6ecc99c..c34bbe9 100644
--- a/frysk-imports/libunwind/ChangeLog
+++ b/frysk-imports/libunwind/ChangeLog
@@ -1,3 +1,10 @@
+2007-03-12  Mark Wielaard  <mwielaard@redhat.com>
+
+	* src/x86/Gstep.c (is_call_instr_at): New function.
+	(init_stack_based_ret): New function.
+	(unw_step): Try stack based unwind with call instr, before
+	fallback to frame pointer.
+
 2007-01-31  Mark Wielaard  <mwielaard@redhat.com>
 
 	Fixup libunwind merge.
diff --git a/frysk-imports/libunwind/src/x86/Gstep.c b/frysk-imports/libunwind/src/x86/Gstep.c
index 618b8af..c9083cb 100644
--- a/frysk-imports/libunwind/src/x86/Gstep.c
+++ b/frysk-imports/libunwind/src/x86/Gstep.c
@@ -85,6 +85,72 @@ code_descriptor_trap (struct cursor *c, struct dwarf_loc *eip_loc_pointer)
   return 1;
 }
 
+// A CALL instruction starts with 0xFF.
+static int
+is_call_instr_at (struct cursor *c, unw_word_t addr)
+{
+  int ret;
+  unw_word_t instr;
+  ret = dwarf_get (&c->dwarf, DWARF_LOC (addr, 0), &instr);
+  return ret >= 0 && ((instr & 0xff000000) == 0xff000000);
+}
+
+// Checks whether this looks like a plt entry like cursor and returns
+// the stack offset where the return address can be found, or -1 if
+// not detected (also tries to make sure this is the inner most frame).
+// When this function returns positively (zero included) addr will
+// contain the return address.
+static int
+init_stack_based_ret (struct cursor *c, unw_word_t *addr)
+{
+  // See if this looks "clean", everything in actual registers
+  // which indicates this is most likely an inner most frame just
+  // initted.
+  int ret;
+  unw_word_t ip, cfa;
+  ret = dwarf_get (&c->dwarf, c->dwarf.loc[EIP], &ip);
+  if (ret < 0)
+    return ret;
+
+  ret = dwarf_get (&c->dwarf, c->dwarf.loc[ESP], &cfa);
+  if (ret < 0)
+    return ret;
+  
+  if (c->sigcontext_format == X86_SCF_NONE
+      && DWARF_IS_REG_LOC(c->dwarf.loc[EAX])
+      && DWARF_IS_REG_LOC(c->dwarf.loc[ECX])
+      && DWARF_IS_REG_LOC(c->dwarf.loc[EDX])
+      && DWARF_IS_REG_LOC(c->dwarf.loc[EBX])
+      && DWARF_IS_REG_LOC(c->dwarf.loc[ESP])
+      && DWARF_IS_REG_LOC(c->dwarf.loc[EBP])
+      && DWARF_IS_REG_LOC(c->dwarf.loc[ESI])
+      && DWARF_IS_REG_LOC(c->dwarf.loc[EDI])
+      && DWARF_IS_REG_LOC(c->dwarf.loc[EIP])
+      && DWARF_IS_REG_LOC(c->dwarf.loc[EFLAGS])
+      && DWARF_IS_REG_LOC(c->dwarf.loc[TRAPNO])
+      && ip == c->dwarf.ip && cfa == c->dwarf.cfa)
+    {
+      // See if one of the top 3 elements on the stack contains a
+      // return address.
+      int i;
+      for (i = 0; i <= 8; i += 4)
+	{
+	  // fprintf(stderr, "trying %d\n", i);
+	  ret = dwarf_get (&c->dwarf, DWARF_LOC (c->dwarf.cfa + i, 0), addr);
+	  if (ret < 0)
+	    return ret;
+
+	  // fprintf(stderr, "addr at %d: 0x%x\n", i, *addr);
+	  // Sanity check the address, not too low, and must
+	  // come from a call instruction.
+	  if (*addr > 0 && is_call_instr_at(c, (*addr) - 5))
+	    return i;
+	}
+    }
+
+  return -1;
+}
+
 PROTECTED int
 unw_step (unw_cursor_t *cursor)
 {
@@ -92,6 +158,7 @@ unw_step (unw_cursor_t *cursor)
   int ret, i;
   struct dwarf_loc eip_loc;
   int eip_loc_set = 0;
+  unw_word_t addr;
 
   Debug (1, "(cursor=%p, ip=0x%08x)\n", c, (unsigned) c->dwarf.ip);
 
@@ -202,6 +269,16 @@ unw_step (unw_cursor_t *cursor)
 	  c->dwarf.loc[TRAPNO] = DWARF_NULL_LOC;
 	  c->dwarf.loc[ST0] = DWARF_NULL_LOC;
 	}
+      else if((ret = init_stack_based_ret(c, &addr)) >= 0)
+	{
+	  // fprintf(stderr, "init_stack_based_ret() %d (0x%x)\n", ret, addr);
+	  c->dwarf.cfa += ret;
+	  c->dwarf.loc[ESP] = DWARF_LOC (c->dwarf.cfa, 0);
+	  c->dwarf.loc[EIP] = DWARF_LOC (addr, 0);
+	  c->dwarf.ret_addr_column = EIP;
+	  c->dwarf.ip = addr;
+	  return 1;
+	}
       else
 	{
 	  ret = dwarf_get (&c->dwarf, c->dwarf.loc[EBP], &c->dwarf.cfa);

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

* Re: [patch] Stack based unwind fallback for plt entries on x86_64
  2008-03-12 13:26 [patch] Stack based unwind fallback for plt entries on x86 Mark Wielaard
@ 2008-03-19 21:50 ` Mark Wielaard
  0 siblings, 0 replies; 2+ messages in thread
From: Mark Wielaard @ 2008-03-19 21:50 UTC (permalink / raw)
  To: frysk

[-- Attachment #1: Type: text/plain, Size: 982 bytes --]

Hi,

x86_64 plt entries weren't so different as I thought. At least not for
those things needed to make our current tests pass. Like x86 plt entries
we take advantage of the fact that they are called through a CALL
instruction and that the stack contains the return address (and possibly
a selector and the GOT table address).

frysk-core/frysk/stack/ChangeLog
2008-03-19  Mark Wielaard  <mwielaard@redhat.com>

    * TestLibFunctionStepFrame.java: No longer unresolved on x86_64.

frysk-imports/libunwind/ChangeLog
2007-03-19  Mark Wielaard  <mwielaard@redhat.com>

    * src/x86_64/Gstep.c (is_call_instr_at): New function.
    (init_stack_based_ret): New function.
    (unw_step): Try stack based unwind with call instr, before
    fallback to frame pointer.

Looking through the abi and instruction manuals however I see there are
other possibilities that aren't matches by the above heuristics. I'll
try and find some examples and add testcases for those later.

Cheers,

Mark

[-- Attachment #2: plt-x86_64.patch --]
[-- Type: text/x-patch, Size: 3297 bytes --]

diff --git a/frysk-imports/libunwind/src/x86_64/Gstep.c b/frysk-imports/libunwind/src/x86_64/Gstep.c
index fe69929..e5ed7b9 100644
--- a/frysk-imports/libunwind/src/x86_64/Gstep.c
+++ b/frysk-imports/libunwind/src/x86_64/Gstep.c
@@ -4,6 +4,9 @@
 
    Modified for x86_64 by Max Asbock <masbock@us.ibm.com>
 
+   Copyright (C) 2008 Red Hat, Inc.
+	Contributed by Mark Wielaard <mwielaard@redhat.com>
+
 This file is part of libunwind.
 
 Permission is hereby granted, free of charge, to any person obtaining
@@ -83,6 +86,59 @@ code_descriptor_trap (struct cursor *c, struct dwarf_loc *rip_loc_pointer)
   return 1;
 }
 
+// A CALL instruction starts with 0xFF.
+static int
+is_call_instr_at (struct cursor *c, unw_word_t addr)
+{
+  int ret;
+  unw_word_t instr;
+  ret = dwarf_get (&c->dwarf, DWARF_LOC (addr, 0), &instr);
+  Debug (99, "ret %d, instr 0x%x\n", ret, instr);
+  return ret >= 0 && ((instr & 0xff000000) == 0xff000000);
+}
+
+// Checks whether this looks like a plt entry like cursor and returns
+// the stack offset where the return address can be found, or -1 if
+// not detected (also tries to make sure this is the inner most frame).
+// When this function returns positively (zero included) addr will
+// contain the return address.
+static int
+init_stack_based_ret (struct cursor *c, unw_word_t *addr)
+{
+  // See if this looks "clean", everything in actual registers
+  // which indicates this is most likely an inner most frame just
+  // initted.
+  int ret;
+  unw_word_t ip, cfa;
+  ret = dwarf_get (&c->dwarf, c->dwarf.loc[RIP], &ip);
+  if (ret < 0)
+    return ret;
+  
+  ret = dwarf_get (&c->dwarf, c->dwarf.loc[RSP], &cfa);
+  if (ret < 0)
+    return ret;
+  
+  // See if one of the top 3 elements on the stack contains a
+  // return address.
+  int i;
+  for (i = 0; i <= 16; i += 8)
+    {
+      Debug (99, "trying %d\n", i);
+      ret = dwarf_get (&c->dwarf, DWARF_LOC (c->dwarf.cfa + i, 0), addr);
+      if (ret < 0)
+	      return ret;
+      
+      Debug (99, "addr at %d: 0x%x\n", i, *addr);
+      // Sanity check the address, not too low, and must
+      // come from a call instruction.
+      if (*addr > 0 && is_call_instr_at(c, (*addr) - 5))
+	return i;
+    }
+  
+  return -1;
+}
+
+
 PROTECTED int
 unw_step (unw_cursor_t *cursor)
 {
@@ -91,6 +147,7 @@ unw_step (unw_cursor_t *cursor)
   struct dwarf_loc rip_loc;
   int rip_loc_set = 0;
   unw_word_t prev_ip = c->dwarf.ip, prev_cfa = c->dwarf.cfa;
+  unw_word_t addr;
 
   Debug (1, "(cursor=%p, ip=0x%016llx)\n",
 	 c, (unsigned long long) c->dwarf.ip);
@@ -186,6 +243,16 @@ unw_step (unw_cursor_t *cursor)
 	  c->dwarf.loc[RBP] = rbp_loc;
 	  c->dwarf.loc[RSP] = rsp_loc;
 	}
+      else if((ret = init_stack_based_ret(c, &addr)) >= 0)
+	{
+	  Debug (99, "init_stack_based_ret() %d (0x%x)\n", ret, addr);
+	  c->dwarf.cfa += ret + 8;
+	  c->dwarf.loc[RSP] = DWARF_LOC (c->dwarf.cfa, 0);
+	  c->dwarf.loc[RIP] = DWARF_LOC (addr, 0);
+	  c->dwarf.ret_addr_column = RIP;
+	  c->dwarf.ip = addr;
+	  return 1;
+	}
       else
 	{
 	  unw_word_t rbp;
@@ -258,6 +325,6 @@ unw_step (unw_cursor_t *cursor)
     return -UNW_EBADFRAME;
 
   ret = (c->dwarf.ip == 0) ? 0 : 1;
-  Debug (2, "returning %d\n", ret);
+  Debug (2, "dwarf.ip = 0x%x, returning %d\n", c->dwarf.ip, ret);
   return ret;
 }

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

end of thread, other threads:[~2008-03-19 21:50 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-03-12 13:26 [patch] Stack based unwind fallback for plt entries on x86 Mark Wielaard
2008-03-19 21:50 ` [patch] Stack based unwind fallback for plt entries on x86_64 Mark Wielaard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).