The gdb x86 function prologue parser

The gdb x86 function prologue parser

[Jason Molenda]

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

Hello, I'm Jason Molenda and I work on gdb for Apple.  :-)

As was announced yesterday, we'll be transitioning from PPC to x86  
over the next couple of years.  Over the last few months I had a  
delightful little introduction to the x86 architecture (read: read  
the IA32 PDFs until I couldn't see straight any more) and worked to  
get our x86 gdb support up to snuff for this software release.

One of the biggest problems we found was the x86 function prologue  
parser is remarkably weak.  We have a very mature and featureful  
prologue parser on our ppc side and an amazing number of bugs were  
directed my way as we had people pounding on the x86 side.  We aren't  
using DWARF yet, so CFI can't save our bacon -- the prologue parser  
has to work or our gdb fails.

There are a couple classes of changes I made, and I spent today  
trying to massage them into some kind of presentable form.  This is  
not perfect -- well, to be honest, this is just a first sketch -- but  
this is a HUGE improvement over the existing facilities.  I wrote the  
code while under immense deadline pressure so I'm not particularly  
interested in how I implemented any of it.  But changes akin to these  
are necessary if you want to debug programs with optimized code on  
the stack and without CFI.  I'll guarantee it.

Roughly speaking, here are the class of changes included in this patch.

1. i386_match_insn() bug fixes.  It wouldn't work for an instruction  
pattern of 1 byte, and it would never check anything beyond the 2nd  
byte (notice where the final "return insn" is located).  I've added  
patterns that can match prologue instructions, so exceptions had to  
be added for the big two (push %ebp, mov %esp, %ebp) and the  
equivalent ones used in the first frame (_start()).

2. i386_frame_setup_skip_insns table expansion.  Because you can't  
skip over an unknown instruction on x86 without knowing its length,  
this was of paramount importance.  Initially I waited for users to  
tell me of prologues that gdb was failing on, but this was taking too  
long and there were too many instructions scheduled into prologues  
for me to hear of them in time.  So I wrote a little maintenance  
command (not included in the patch to keep things simple) which would  
tell you if gdb could parse through the prologue of a given  
function.  Then with a couple of shell scripts, I could have gdb try  
to analyze the prologues of every function in every library on my  
MacOS X system and show me the ones it failed on.  I'd add them to  
this list.  I also made a little testsuite generator where the input  
looks like

# SOURCE: RedHat FedoraCore2 /lib/ld-2.3.3.so _dl_reloc_bad_type
# PROLOGUE
   push %ebp
   shl $0x5, %ecx # [ 0xc1 0xe1 0x05 ]
   mov %esp, %ebp
   sub $0x8, %esp
# EPILOGUE
   add $0x8, %esp
   pop %ebp
   ret

and a script that transforms the patterns into a test program and a  
Dejagnu expect script.  So you can ensure that you don't regress the  
prologue parser.  This was the lesson we learned in writing our PPC  
parser -- we have this wonderfully ornate parser with lots of  
exceptions and known tricks, but no testsuite for it.  So whenever we  
change it we're cringing because the gdb testsuite has nothing useful  
in it.  (you need optimized, no debug info test cases to be sure it's  
still working right).  The testsuite stuff isn't included in this  
patch, but I'll put that together soon and send it along if anyone's  
interested.

3. relatively minor changes to i386_analyze_frame_setup().  It had to  
have the push %ebp as the very first instruction or it would give  
up.  That's really bad -- the compiler can (and does) schedule all  
sorts of stuff before that instruction.

4. new function, i386_find_esp_adjustments().  This is used in a  
frameless leaf function where the compiler may create space on the  
stack for local variables and stuff, but doesn't call anything so it  
doesn't save the caller's frame pointer.  And it allows -fomit-leaf- 
frame-pointer codegen to be debugged.  -fomit-frame-pointer is a  
whole lot more complicated, but this wasn't so bad.  (we didn't end  
up enabling -fomit-leaf-frame-pointer in this release because of the  
schedule time constraints, but that's why I wrote it)

5. Huge i386_frame_cache() changes.  There's no way around it, this  
function is just not right.  It doesn't handle frameless functions  
correctly at all.  It's written without a clear understanding of the  
different classes of functions it needs to handle and works primarily  
by luck.  And for goodness sakes, if we can't figure out anything  
about a function that's not at the top of the stack, don't you think  
it'd be reasonable to assume that the function has set up a stack  
frame and saved the caller's EBP?  Sure seems like a reasonable  
assumption to me.  Why can't this function do something even that  
basic?  This function really cheesed my mellow.

Mark, I want to say that I'm not directing any of these criticisms  
towards you -- I've been looking over the changes you've made and  
they're definite improvements over the existing code.  The existing  
code bites, though.  I can't even begin to imagine how annoyed  
developers using the FSF gdb on x86 must be.  The changes I'm sending  
here are not a panacea/beautiful/perfect, but *functionally* they're  
a huge improvement.  Now that our release is out there I'll be more  
than happy to revisit the decisions/implementation that I came up  
with on little sleep. :-)


Oh, and I ran my "find all prologues gdb can't parse" on a FedoraCore  
2 system I have handy here at the office today and added the patterns  
of the biggest offenders.  There are still a few patterns I need to  
add to get 100% parsing success but I want to go home :-) so that'll  
be for another day.

We're in the middle of an all-week Apple developer's conference, so  
my replies may not be very speedy (I'm off-line 10-12 hours a day; I  
slipped away to get this patch together today) until the weekend.   
But I'll try to stay on top of any questions or comments and address  
them promptly.

I'm looking forward to making x86 gdb excellent.  There's definitely  
more work to do and I intend to be a part of that.

Jason Molenda
Apple Computer


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 21548 bytes --]

Index: i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.213
diff -u -p -w -r1.213 i386-tdep.c
--- i386-tdep.c	28 May 2005 16:44:28 -0000	1.213
+++ i386-tdep.c	8 Jun 2005 05:24:14 -0000
@@ -21,6 +21,7 @@
    Foundation, Inc., 59 Temple Place - Suite 330,
    Boston, MA 02111-1307, USA.  */
 
+#include <stdint.h>
 #include "defs.h"
 #include "arch-utils.h"
 #include "command.h"
@@ -287,9 +288,10 @@ i386_breakpoint_from_pc (CORE_ADDR *pc, 
 
 struct i386_frame_cache
 {
-  /* Base address.  */
+  /* Base address, usually the contents of the EBP.  */
   CORE_ADDR base;
   CORE_ADDR sp_offset;
+  /* The function's start address for this frame.  */
   CORE_ADDR pc;
 
   /* Saved registers.  */
@@ -473,7 +475,7 @@ i386_skip_probe (CORE_ADDR pc)
 }
 
 /* Maximum instruction length we need to handle.  */
-#define I386_MAX_INSN_LEN	6
+#define I386_MAX_INSN_LEN	7
 
 /* Instruction description.  */
 struct i386_insn
@@ -495,25 +497,46 @@ i386_match_insn (CORE_ADDR pc, struct i3
 
   op = read_memory_unsigned_integer (pc, 1);
 
+  /* `push %ebp'?  Let the caller look into it.  */
+  if (op == 0x55)
+    return NULL;
+
   for (insn = skip_insns; insn->len > 0; insn++)
     {
       if ((op & insn->mask[0]) == insn->insn[0])
 	{
 	  unsigned char buf[I386_MAX_INSN_LEN - 1];
-	  size_t i;
+	  int i, insn_matched;
+
+          if (insn->len == 1)
+            return insn;
 
 	  gdb_assert (insn->len > 1);
 	  gdb_assert (insn->len <= I386_MAX_INSN_LEN);
 
 	  read_memory (pc + 1, buf, insn->len - 1);
+
+          /* `mov %esp, %ebp'?  `xor $ebp, $ebp'?  `push $0x0'?
+             Let the caller check it out.  */
+          if (insn->len == 2
+              && ((op == 0x89 && buf[0] == 0xe5)      /* mov %esp, %ebp */
+                  || (op == 0x8b && buf[0] == 0xec)   /* mov %esp, %ebp */
+                  || (op == 0x31 && buf[0] == 0xed)   /* xor %ebp, %ebp */
+                  || (op == 0x6a && buf[0] == 0x00))) /* push $0x0 */
+            return NULL;
+          
+          insn_matched = 1;
 	  for (i = 1; i < insn->len; i++)
 	    {
 	      if ((buf[i - 1] & insn->mask[i]) != insn->insn[i])
+                {
+                  insn_matched = 0;
 		break;
-
-	      return insn;
 	    }
 	}
+          if (insn_matched)
+            return insn;
+	}
     }
 
   return NULL;
@@ -554,6 +577,91 @@ struct i386_insn i386_frame_setup_skip_i
   /* `movl m32, %edx' */
   { 6, { 0x89, 0x15 }, {0xff, 0xff } },
 
+  /* "01 /r       ADD r/m32, r32" */
+  { 2, { 0x01, 0xd0 }, { 0xff, 0xd0 } },
+  /* "0F B6 /r    MOVZX r32, r/m8" (aka `movzbl %al, %eax') */
+  { 3, { 0x0f, 0xb6, 0xc0 }, { 0xff, 0xff, 0xc0 } },
+  /* "0F B7 /r    MOVZX r32, r/m16" (aka `movzwl r16, r32') */
+  { 3, { 0x0f, 0xb7, 0xc0 }, { 0xff, 0xff, 0xc0 } },
+  /* "25 id       AND EAX, imm32" */
+  { 5, { 0x25 }, { 0xff } },
+  /* "31 /r       XOR r/m32, r32" */
+  { 2, { 0x31, 0xc0 }, { 0xff, 0xc0 } },
+  /* "40+ rd      INC r32" */
+  { 1, { 0x40 }, { 0xf8 } },
+  /* "48+rw       DEC r32" */
+  { 1, { 0x48 }, { 0xf8 } },
+  /* "50+rd       PUSH r32" */
+  { 1, { 0x50 }, { 0xf8 } },
+  /* "58+ rd      POP r32" */
+  { 1, { 0x58 }, { 0xf8 } },
+  /* "B8+ rw      MOV r16, imm16" */
+  { 4, { 0x66, 0xb8 }, { 0xff, 0xf8 } },
+  /* "74 cb       JE rel8" */
+  { 2, { 0x74 }, { 0xff } },
+  /* "80 /7 ib    CMP r/m8, imm8" (aka `cmpb imm8,(r32)') */
+  { 3, { 0x80, 0x38 }, { 0xff, 0xf8 } },
+  /* "81 /4 id    AND r/m32, imm32" */
+  { 6, { 0x81, 0xe0 }, { 0xff, 0xf8 } },
+  /* "81 /0 id    ADD r/m32, imm32" */
+  { 6, { 0x81, 0xc0 }, { 0xff, 0xf8 } }, 
+  /* "83 /7 ib    CMP r/m32, imm8" */
+  { 7, { 0x83, 0x3d }, { 0xff, 0xff } },
+  /* "83 /0 ib    ADD r/m32, imm8" */
+  { 3, { 0x83, 0xc0 }, { 0xff, 0xf8 } },
+  /* "83 /1 ib    OR r/m32, imm8" */
+  { 3, { 0x83, 0xc8 }, { 0xff, 0xf8 } },
+  /* "83 /4 ib    AND r/m32, imm8" */
+  { 3, { 0x83, 0xe0 }, { 0xff, 0xf8 } },
+  /* "83 /5 ib    SUB r/m32, imm8" */
+  { 3, { 0x83, 0xe8 }, { 0xff, 0xf8 } },
+  /* "83 /7 ib    CMP r/m32, imm8" */
+  { 3, { 0x83, 0xf8 }, { 0xff, 0xf8 } },
+  /* "85 /r       TEST r/m32, r32" */
+  { 2, { 0x85, 0xc0 }, { 0xff, 0xc0 } },
+  /* "89 /r       MOV r/m32, r32" (aka `mov r32, (r32)') */
+  { 2, { 0x89, 0x00 }, { 0xff, 0xf8 } },
+  /* "89 /r       MOV r/m32, r32" (aka `mov r32, (r32)') */
+  { 2, { 0x89, 0x10 }, { 0xff, 0xf8 } },
+  /* "89 /r       MOV r/m32, r32" */
+  { 2, { 0x89, 0xc0 }, { 0xff, 0xc0 } }, 
+  /* "8B /r       MOV r32, r/m32" (aka `mov imm8(r32), r32') */
+  { 3, { 0x8b, 0x40 }, { 0xff, 0xf8 } },
+  /* "8B /r       MOV r32, r/m32" */
+  { 6, { 0x8b, 0x80 }, { 0xff, 0xf8 } },
+  /* "8B /r       MOV r32, r/m32" (aka `mov imm32(r32), r32') */
+  { 6, { 0x8b, 0x90 }, { 0xff, 0xf8 } },
+  /* "8D /r       LEA r32, m" (aka `lea imm8(r32), r32') */
+  { 3, { 0x8d, 0x48 }, { 0xff, 0xf8 } },
+  /* "8D /r       LEA r32, m" (aka `lea imm32(r32), r32') */
+  { 6, { 0x8d, 0x90 }, { 0xff, 0xf8 } },
+  /* "90          NOP" */
+  { 1, { 0x90 }, { 0xff } },
+  /* "A9 id       TEST EAX, imm32" */
+  { 5, { 0xa9 }, { 0xff } },
+  /* "C1 /4 ib    SAL r/m32, imm8" (aka `shl imm8, r32') */
+  { 3, { 0xc1, 0xe0 }, { 0xff, 0xf8 } },
+  /* "C1 /7 ib    SAR r/m32, imm8" */
+  { 3, { 0xc1, 0xf8 }, { 0xff, 0xf8 } },
+  /* "C6 /0       MOV r/m8, imm8" (aka `mov imm8, (r32)') */
+  { 3, { 0xc6, 0x00 }, { 0xff, 0xf8 } },
+  /* "C7 /o       MOV r/m32, imm32 (aka `mov imm32, (r32)') */
+  { 6, { 0xc7, 0x00 }, { 0xff, 0xf8 } },
+  /* "D9 EE       FLDZ" */
+  { 2, { 0xd9, 0xee }, { 0xff, 0xff } },
+  /* "E8 cd       CALL rel32" */
+  { 5, { 0xe8 }, { 0xff } },
+  /* "EB cb       JMP rel8" */
+  { 2, { 0xeb }, { 0xff } },
+  /* "F7 /6       DIV r/m32" */
+  { 2, { 0xf7, 0xf0 }, { 0xff, 0xf8 } },
+  /* "FC          CLD" */
+  { 1, { 0xfc }, { 0xff } },
+  /* "FF /6       PUSH r/m32" */
+  { 6, { 0xff, 0xb0 }, { 0xff, 0xf8 } },
+  /* "FF /4       JMP r/m32" */
+  { 2, { 0xff, 0xe0 }, { 0xff, 0xf8 } },
+
   /* Check for `xorl r32, r32' and the equivalent `subl r32, r32'.
      Because of the symmetry, there are actually two ways to encode
      these instructions; opcode bytes 0x29 and 0x2b for `subl' and
@@ -587,21 +695,71 @@ i386_analyze_frame_setup (CORE_ADDR pc, 
   gdb_byte op;
   int skip = 0;
 
+  /* This function returns a CORE_ADDR which is the instruction
+     following the last frame-setup instruction we saw such that "frame-setup
+     instruction" is one of push %ebp, push $0x0, mov %esp, %ebp, 
+     sub $<imm>, $esp, or enter.
+     Specifically, we may scan past some of these instructions but we don't
+     want to return the last address we scanned to -- we must return the 
+     address of the instruction after one of those frame setup insns so that
+     i386_analyze_register_saves () can look for register saves that may exist
+     after them.  
+     (and you can have register saves without any frame setup, e.g. in a 
+     frameless function.) 
+     I pedantically changed all returns to return end_of_frame_setup so it
+     is completely clear what is going on.  jmolenda/2005-05-03 */
+
+  CORE_ADDR end_of_frame_setup = pc;
+
   if (limit <= pc)
     return limit;
 
+
+  /* Skip over non-prologue instructions until we hit one of
+       push %ebp      [ 0x55 ]
+       push $0x0      [ 0x6a 0x00 ]
+       xor %ebp, %ebp [ 0x31 0xed ]
+     the latter instructions being the first frame setup insns in
+     MacOS X and Linux _start()s.  */
+
   op = read_memory_unsigned_integer (pc, 1);
+  while (op != 0x55
+         && (op != 0x6a || 
+                  read_memory_unsigned_integer (pc + skip + 1, 1) != 0x00)
+         && (op != 0x31 ||
+                  read_memory_unsigned_integer (pc + skip + 1, 1) != 0xed)
+         && pc + skip <= limit)
+    {
+      insn = i386_match_insn (pc + skip, i386_frame_setup_skip_insns);
+      if (insn)
+        {
+          skip += insn->len;
+          op = read_memory_unsigned_integer (pc + skip, 1);
+        }
+      else
+        break;
+    }
+  
+  if (limit <= pc + skip)
+    return end_of_frame_setup;
 
-  if (op == 0x55)		/* pushl %ebp */
+  /* Do we have our initial frame setup instruction?  */
+  if (op == 0x55 || op == 0x6a || op == 0x31)
     {
       /* Take into account that we've executed the `pushl %ebp' that
 	 starts this instruction sequence.  */
       cache->saved_regs[I386_EBP_REGNUM] = 0;
       cache->sp_offset += 4;
-      pc++;
+
+      /* Skip over the second byte of `push $0x0' and `xor %ebp, %ebp' */
+      if (op == 0x6a || op == 0x31)
+        skip += 2;
+      else 
+        skip++;
+      end_of_frame_setup = pc + skip;
 
       /* If that's all, return now.  */
-      if (limit <= pc)
+      if (limit <= pc + skip)
 	return limit;
 
       /* Check for some special instructions that might be migrated by
@@ -623,7 +781,7 @@ i386_analyze_frame_setup (CORE_ADDR pc, 
 
       /* If that's all, return now.  */
       if (limit <= pc + skip)
-	return limit;
+	return end_of_frame_setup;
 
       op = read_memory_unsigned_integer (pc + skip, 1);
 
@@ -632,14 +790,14 @@ i386_analyze_frame_setup (CORE_ADDR pc, 
 	{
 	case 0x8b:
 	  if (read_memory_unsigned_integer (pc + skip + 1, 1) != 0xec)
-	    return pc;
+	    return end_of_frame_setup;
 	  break;
 	case 0x89:
 	  if (read_memory_unsigned_integer (pc + skip + 1, 1) != 0xe5)
-	    return pc;
+	    return end_of_frame_setup;
 	  break;
 	default:
-	  return pc;
+	  return end_of_frame_setup;
 	}
 
       /* OK, we actually have a frame.  We just don't know how large
@@ -648,6 +806,7 @@ i386_analyze_frame_setup (CORE_ADDR pc, 
 	 instructions mentioned before.  */
       cache->locals = 0;
       pc += (skip + 2);
+      end_of_frame_setup = pc;
 
       /* If that's all, return now.  */
       if (limit <= pc)
@@ -695,7 +854,7 @@ i386_analyze_frame_setup (CORE_ADDR pc, 
       return pc + 4;
     }
 
-  return pc;
+  return end_of_frame_setup;
 }
 
 /* Check whether PC points at code that saves registers on the stack.
@@ -771,6 +930,11 @@ i386_analyze_prologue (CORE_ADDR pc, COR
 static CORE_ADDR
 i386_skip_prologue (CORE_ADDR start_pc)
 {
+  /* FIXME: this code assumes the "call L1; L1: pop $ebx" type of pic base
+     setup, but that's less optimal on x86 than a call to 
+     __i686.get_pc_thunk.bx and friends, so you're unlikely to see this
+     sequence these days.  jmolenda/2005-06-06.  */
+
   static gdb_byte pic_pat[6] =
   {
     0xe8, 0, 0, 0, 0,		/* call 0x0 */
@@ -844,6 +1008,95 @@ i386_skip_prologue (CORE_ADDR start_pc)
   return pc;
 }
 
+/* Find adjustments of the ESP so we can locate the
+   caller's saved EIP and backtrace out of a frameless function.  PC is
+   the start of the function.  CURRENT_PC is the current instruction,
+   or the last instruction of the function to limit this search.
+
+   Returns a signed offset of how much the ESP has moved since the
+   start of the function.  The return value should be a negative number
+   or 0.  */
+
+static int
+i386_find_esp_adjustments (CORE_ADDR pc, CORE_ADDR current_pc)
+{
+  gdb_byte op, next_op;
+  int esp_change = 0;
+  struct i386_insn *insn;
+
+  if (pc == current_pc)
+    return esp_change;
+  
+  /* We're looking for PUSH r32, POP r32, SUB $x,ESP, ADD $x ESP. */
+ 
+  while (pc < current_pc)
+    {
+      op = read_memory_unsigned_integer (pc, 1);
+      next_op = read_memory_unsigned_integer (pc + 1, 1);
+      
+      /* `push %ebp'?  We shouldn't see that in here; give up. */
+      if (op == 0x55)
+        return esp_change;
+      /* `ret'? We're at the end of the func; stop parsing. */
+      if (op == 0xc3)
+        return esp_change;
+
+      /* 50+rd       PUSH r32 */
+      if ((op & 0xf8) == 0x50)
+        {
+          esp_change -= 4;
+          pc += 1;
+          continue;
+        }
+      /* 58+ rd      POP r32 */
+      if ((op & 0xf8) == 0x58)
+        {
+          esp_change += 4;
+          pc += 1;
+          continue;
+        }
+      /* 83 /5 ib    SUB r/m32, imm8 */
+      if (op == 0x83 && next_op == 0xec)
+        {
+          uint8_t imm8 = read_memory_integer (pc + 2, 1);
+          esp_change -= imm8;
+          pc += 3;
+          continue;
+        }
+      /* 81 /5 id    SUB r/m32, imm32 */
+      if (op == 0x81 && next_op == 0xec)
+        {
+          uint32_t imm32 = read_memory_integer (pc + 2, 4);
+          esp_change -= imm32;
+          pc += 6;
+          continue;
+        }
+      /* 83 /0 ib    ADD r/m32, imm8 */
+      if (op == 0x83 && next_op == 0xc4)
+        {
+          uint8_t imm8 = read_memory_integer (pc + 2, 1);
+          esp_change += imm8;
+          pc += 3;
+          continue;
+        }
+      /* 81 /0 id    ADD r/m32, imm8 */
+      if (op == 0x81 && next_op == 0xc4)
+        {
+          uint32_t imm32 = read_memory_integer (pc + 2, 4);
+          esp_change += imm32;
+          pc += 6;
+          continue;
+        }
+
+      insn = i386_match_insn (pc, i386_frame_setup_skip_insns);
+      if (insn)
+        pc += insn->len;
+      else
+        return esp_change;  /* Hit an instruction we don't know; stop here. */
+    }
+  return esp_change;
+}
+
 /* This function is 64-bit safe.  */
 
 static CORE_ADDR
@@ -856,7 +1109,30 @@ i386_unwind_pc (struct gdbarch *gdbarch,
 }
 \f
 
-/* Normal frames.  */
+/* Normal frames.
+   This function must handle the following possible function types:
+
+   0.  We're in a function we cannot know anything about - we have no 
+       symbol for it; we can't find the start address.
+
+   1.  The function is frameless.
+     a. The ESP hasn't reached its Final Resting Place for the body of
+        the function yet.
+     b. We've finished the prologue (wherein we move the ESP, i.e. SUBs to
+        make space for local storage, PUSHes to preserve saved regs.)
+
+   2.  The function sets up a frame and
+     a. We haven't executed any prologue instructions.
+     b. We've executed the initial push %ebp (this one is critical).
+     c. We've executed the mov %esp, %ebp
+     d. We've completed the entire prologue.
+     e. We're in the middle of a function which has a prologue, 
+        but we can't parse it (we hit an unknown instruction mid-prologue).
+
+   When reading i386_frame_cache, keep these three function types in mind
+   and the different stages for #1 and #2 - the behavior of this function
+   differs greatly depending on where you are.  */
+
 
 static struct i386_frame_cache *
 i386_frame_cache (struct frame_info *next_frame, void **this_cache)
@@ -864,6 +1140,32 @@ i386_frame_cache (struct frame_info *nex
   struct i386_frame_cache *cache;
   gdb_byte buf[4];
   int i;
+  int potentially_frameless;
+  CORE_ADDR prologue_parsed_to = 0;
+  CORE_ADDR current_pc;
+
+  /* If the frame we're examining is frame #0, we could
+     be frameless.  If NEXT_FRAME is _sigtramp(), then we could be
+     frameless.
+
+      Explanation:  If a frameless function is executing when a
+      signal is caught, the frameless function will have _sigtramp()
+      as its next_frame, followed by whatever signal handler is defined.
+      This is not as rare as you'd think, at least in a testsuite:
+      On MacOS X, sleep() calls nanosleep() which calls mach_wait_until()
+      which is frameless.  If an alarm(1) is done before that sequence,
+      you'll get a frameless function in the middle of the stack.
+
+     If potentially_frameless == 0, there's no way the function we're
+     examining is frameless; it has a stack frame set up with 
+     the saved-EBP/saved-EIP at the standard locations.  
+     (not entirely true -- if gcc's -fomit-frame-pointer is used you can
+     have a function that doesn't ever set up the EBP, but calls other
+     functions.  Handling that situation correctly is not easy with
+     i386-tdep.c's frame code as it stands today.)  */
+
+  potentially_frameless = frame_relative_level (next_frame) == -1 
+                       || get_frame_type (next_frame) == SIGTRAMP_FRAME;
 
   if (*this_cache)
     return *this_cache;
@@ -871,41 +1173,86 @@ i386_frame_cache (struct frame_info *nex
   cache = i386_alloc_frame_cache ();
   *this_cache = cache;
 
-  /* In principle, for normal frames, %ebp holds the frame pointer,
-     which holds the base address for the current stack frame.
-     However, for functions that don't need it, the frame pointer is
-     optional.  For these "frameless" functions the frame pointer is
-     actually the frame pointer of the calling frame.  Signal
-     trampolines are just a special case of a "frameless" function.
-     They (usually) share their frame pointer with the frame that was
-     in progress when the signal occurred.  */
+  /* Set up reasonable defaults that we'll override later on as necessary.  */
+
+  /* For normal frames, saved-%eip is stored at 4(%ebp). */
+  cache->saved_regs[I386_EIP_REGNUM] = 4;
+
+  /* For normal frames, saved-%ebp is stored at 0(%ebp). */
+  cache->saved_regs[I386_EBP_REGNUM] = 0;
 
   frame_unwind_register (next_frame, I386_EBP_REGNUM, buf);
   cache->base = extract_unsigned_integer (buf, 4);
   if (cache->base == 0)
     return cache;
 
-  /* For normal frames, %eip is stored at 4(%ebp).  */
-  cache->saved_regs[I386_EIP_REGNUM] = 4;
-
   cache->pc = frame_func_unwind (next_frame);
+  current_pc = frame_pc_unwind (next_frame);
+
+  /* Only do i386_analyze_prologue () if we found a debug symbol pointing to
+     the actual start of the function.  */
   if (cache->pc != 0)
-    i386_analyze_prologue (cache->pc, frame_pc_unwind (next_frame), cache);
+    i386_analyze_prologue (cache->pc, current_pc, cache);
+
+  /* Let's see if the prologue parser didn't find any prologue instructions.
+     And our current function has the potential to be frameless.  
+     If so, let's go frameless.  Assume EBP is unused, or not yet used.
+
+     We'll put ESP in the cache->base instead of EBP; for genuinely
+     frameless (e.g. -momit-leaf-frame-pointer) functions, the
+     the debug info for function args will be relative to ESP once its 
+     setup/adjustements in the prologue are complete, so cache->base has
+     to hold the stack pointer if we're to find them. */
+
+      /* We found a function-start address, 
+         or $pc is at 0x0 (someone jmp'ed thru NULL ptr).  */
+  if ((cache->pc != 0 || current_pc == 0)
+      /* The prologue parser didn't find any prologue instructions.  */
+      && prologue_parsed_to == cache->pc
+      /* We have the potential to be frameless.  */
+      && potentially_frameless)
+    {
+      int esp_offset;
+      CORE_ADDR actual_frame_base;
+      esp_offset = i386_find_esp_adjustments (cache->pc, current_pc);
+      frame_unwind_register (next_frame, I386_ESP_REGNUM, buf);
+      actual_frame_base = extract_unsigned_integer (buf, 4) - esp_offset;
+      cache->base = extract_unsigned_integer (buf, 4);
+      cache->saved_sp = actual_frame_base + 4;
+      cache->saved_regs[I386_EBP_REGNUM] = -1;
+      cache->saved_regs[I386_EIP_REGNUM] = 0;
+      /* NB: There's a good chance we didn't record register saves a la
+         i386_analyze_register_saves.  It'd be nice to fix this.   
+         For now we'll say "Debugging optimized code is an adventure!"
+         jmolenda/2005-04-27 */
 
-  if (cache->locals < 0)
+      for (i = 0; i < I386_NUM_SAVED_REGS; i++)
+        if (cache->saved_regs[i] != -1)
+          cache->saved_regs[i] += actual_frame_base;
+      return cache;
+    }
+
+  if (cache->locals < 0 && potentially_frameless)
     {
-      /* We didn't find a valid frame, which means that CACHE->base
-	 currently holds the frame pointer for our calling frame.  If
-	 we're at the start of a function, or somewhere half-way its
-	 prologue, the function's frame probably hasn't been fully
-	 setup yet.  Try to reconstruct the base address for the stack
-	 frame by looking at the stack pointer.  For truly "frameless"
-	 functions this might work too.  */
+      /* We've seen PART of a frame setup, but not the whole deal.
+         We've probably executed just the `push %ebp'.
+         Right now, ESP has our real frame base address in it.  */
 
       frame_unwind_register (next_frame, I386_ESP_REGNUM, buf);
       cache->base = extract_unsigned_integer (buf, 4) + cache->sp_offset;
     }
 
+  /* If we have no idea where this function began (so we can't analyze
+     the prologue in any way), what should we assume?  Frameless or
+     not?  It's a tough call.  On Linux systems, a call to a system
+     library involves a couple of trampoline jumps that have no symbols,
+     so to work correctly on Linux we MUST assume frameless.  */
+  if (potentially_frameless && cache->pc == 0)
+    {
+      cache->saved_regs[I386_EIP_REGNUM] = 4;
+      cache->saved_regs[I386_EBP_REGNUM] = -1;
+    }
+
   /* Now that we have the base address for the stack frame we can
      calculate the value of %esp in the calling frame.  */
   cache->saved_sp = cache->base + 8;

Re: The gdb x86 function prologue parser

[Daniel Jacobowitz]

Hi Jason,

I don't know much about x86.  I also didn't spend a lot of time looking
at this patch or its context - I hope that Mark will be able to do that
in more detail.  But a couple of things...

On Tue, Jun 07, 2005 at 10:51:36PM -0700, Jason Molenda wrote:
> 2. i386_frame_setup_skip_insns table expansion.  Because you can't  
> skip over an unknown instruction on x86 without knowing its length,  
> this was of paramount importance.  Initially I waited for users to  

I looked at your table.

(A) You've added jump instructions to it.  Assuming that I'm following
what you're doing with this table correctly, I'm not real comfortable
with that without special cases checking the targets of the jumps.

(B) Can't you do this using the opcodes library?  With only three
instructions and their alternate encodings in the table, the table made
sense.  With dozens, it doesn't seem to any more.  Disassemble the
blasted thing and see if it's a push that way.

> this list.  I also made a little testsuite generator where the input  
> looks like
> 
> # SOURCE: RedHat FedoraCore2 /lib/ld-2.3.3.so _dl_reloc_bad_type
> # PROLOGUE
>   push %ebp
>   shl $0x5, %ecx # [ 0xc1 0xe1 0x05 ]
>   mov %esp, %ebp
>   sub $0x8, %esp
> # EPILOGUE
>   add $0x8, %esp
>   pop %ebp
>   ret
> 
> and a script that transforms the patterns into a test program and a  
> Dejagnu expect script.  So you can ensure that you don't regress the  
> prologue parser.  This was the lesson we learned in writing our PPC  
> parser -- we have this wonderfully ornate parser with lots of  
> exceptions and known tricks, but no testsuite for it.  So whenever we  
> change it we're cringing because the gdb testsuite has nothing useful  
> in it.  (you need optimized, no debug info test cases to be sure it's  
> still working right).  The testsuite stuff isn't included in this  
> patch, but I'll put that together soon and send it along if anyone's  
> interested.

Interested?  Hell yes.  And the scripts, too.  This could be seriously
useful.

> 5. Huge i386_frame_cache() changes.  There's no way around it, this  
> function is just not right.  It doesn't handle frameless functions  
> correctly at all.  It's written without a clear understanding of the  
> different classes of functions it needs to handle and works primarily  
> by luck.  And for goodness sakes, if we can't figure out anything  
> about a function that's not at the top of the stack, don't you think  
> it'd be reasonable to assume that the function has set up a stack  
> frame and saved the caller's EBP?  Sure seems like a reasonable  
> assumption to me.  Why can't this function do something even that  
> basic?  This function really cheesed my mellow.

Because there is a GDB policy to determine information about the frame
based on the current frame, not based on where it lies on the stack.
I've experimented with this before; this change can have some weird
consequences... for instance, in any case where we can backtrace
through "foo" only because of the addition of this case, we won't be
able to backtrace through "foo" if it is on top of the stack.

You can find more information about this in the list archives, in
plenty of places; most recently Mark pulled together an implementation
of "set i386 trust-frame-pointer".

That said, it may still be better than nothing.  I am still undecided.

> +     If potentially_frameless == 0, there's no way the function we're
> +     examining is frameless; it has a stack frame set up with 
> +     the saved-EBP/saved-EIP at the standard locations.  
> +     (not entirely true -- if gcc's -fomit-frame-pointer is used you can
> +     have a function that doesn't ever set up the EBP, but calls other
> +     functions.  Handling that situation correctly is not easy with
> +     i386-tdep.c's frame code as it stands today.)  */
> +
> +  potentially_frameless = frame_relative_level (next_frame) == -1 
> +                       || get_frame_type (next_frame) == SIGTRAMP_FRAME;

You want != NORMAL_FRAME.  You've ignored the dummy frame case.

> +      /* We found a function-start address, 
> +         or $pc is at 0x0 (someone jmp'ed thru NULL ptr).  */
> +  if ((cache->pc != 0 || current_pc == 0)

No way that's right.  A jump through 0x0 is no different from a jump
through any other unmapped, non-code address.  Normally one uses a
different frame unwinder for that case.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: The gdb x86 function prologue parser

[Eli Zaretskii]

> Cc: Mark Kettenis <kettenis@jive.nl>
> From: Jason Molenda <jmolenda@apple.com>
> Date: Tue, 7 Jun 2005 22:51:36 -0700
> 
> I can't even begin to imagine how annoyed developers using the FSF
> gdb on x86 must be.

Well, I'm one of the annoyed, although I have no idea whether the
problems that annoyed me would be solved by your patches.  In any
case, thanks.

> --- i386-tdep.c	28 May 2005 16:44:28 -0000	1.213
> +++ i386-tdep.c	8 Jun 2005 05:24:14 -0000
> @@ -21,6 +21,7 @@
>     Foundation, Inc., 59 Temple Place - Suite 330,
>     Boston, MA 02111-1307, USA.  */
>  
> +#include <stdint.h>

I don't think we can use stdint.h freely, as we still don't require a
C9x compiler.  I see you needed it for things like uint8_t and
uint32_t, which should be easily replaceable by suitable standard x86
types.

I also agree with Daniel: it would be nice to find a more graceful way
of storing knowledge about so many instructions.

Last, but not least, I'd surely appreciate some write-up, even in
plain ASCII, about how prologue analyzers in general and the x86 one
in particular work: that stuff is sorely needed in gdbint.texinfo.

TIA

Re: The gdb x86 function prologue parser

[Jason Molenda]

Hi Daniel, thanks for the comments.

On Wed, Jun 08, 2005 at 09:24:31AM -0400, Daniel Jacobowitz wrote:

> I looked at your table.
> 
> (A) You've added jump instructions to it.  Assuming that I'm following
> what you're doing with this table correctly, I'm not real comfortable
> with that without special cases checking the targets of the jumps.

These showed up in a couple of functions that had hand-written
assembly at the start of the function.  Like there's one who has a
special agreement with its caller about the contents of EDX, and
it'd compare EDX to a value and then jump to an alternate location
if it matched.  The only way this code is executed is if the PC is
past the jmp instruction, so I wasn't too concerned about it --
SOMEHOW we got past the jmp and back again.

> (B) Can't you do this using the opcodes library?  

Yeah, that's the right way to go, I just got too far into this
scheme to switch for the release we made.  I'm a little cautious
about the opcodes disassembler being fast enough (the prologue parser
can be a fairly hot piece of code) but I don't have any evidence
one way or the other.

> > and a script that transforms the patterns into a test program and a  
> > Dejagnu expect script.  So you can ensure that you don't regress the  
> > prologue parser.  

> Interested?  Hell yes.  And the scripts, too.  This could be seriously
> useful.

Yeah, I didn't mean to be danging it tauntingly or anything; I just
didn't have time last night to get it together to send.  This weekend
and next week I'll be able to get through a lot of this stuff more
easily.  The script is nothing fancy, but then the whole point is that
the patterns list is the really important bit and it can be transformed
in any way you'd like in the future.  Right now it calls a function with
the pattern, and the pattern calls a function itself.  I think gdb puts
a breakpoint in the leaf function, finishes out back to main(), then
continues to the next breakpoint, doing backtraces along the way.

> 
> > And for goodness sakes, if we can't figure out anything  
> > about a function that's not at the top of the stack, don't you think  
> > it'd be reasonable to assume that the function has set up a stack  
> > frame and saved the caller's EBP?  
> 
> Because there is a GDB policy to determine information about the frame
> based on the current frame, not based on where it lies on the stack.
> I've experimented with this before; this change can have some weird
> consequences... for instance, in any case where we can backtrace
> through "foo" only because of the addition of this case, we won't be
> able to backtrace through "foo" if it is on top of the stack.

I'd say that's an expected behavior, but yes, it's true that this
can happen.  It'd be great if the prologue analyzer never got confused
and could always figure out how to find a function's caller's saved
fp/pc, but even if we switch to using the opcodes disassembler so
we never lose on another instruction, on MacOS X we can have libraries
where the functions up the stack that have no symbols whatsoever.  
We have no idea where the function might begin--all we know is a saved
address in the middle of a function.  In such a situation, is it
preferable that we can't backtrace past tricky functions like these?
After a month of working on the x86 port, I got so frustrated I wrote
a user command that could backtrace --

define x86-bt
  set $frameno = 1
  set $cur_ebp = $ebp
  printf "frame 0 EBP: 0x%08x EIP: 0x%08x\n", $ebp, $eip
  x/1i $eip
  set $prev_ebp = *((uint32_t *) $cur_ebp)
  set $prev_eip = *((uint32_t *) ($cur_ebp + 4))
  while $prev_ebp != 0
    printf "frame %d EBP: 0x%08x EIP: 0x%08x\n", $frameno, $prev_ebp, $prev_eip
    x/1i $prev_eip
    set $cur_ebp = $prev_ebp
    set $prev_ebp = *((uint32_t *) $cur_ebp)
    set $prev_eip = *((uint32_t *) ($cur_ebp + 4))
    set $frameno = $frameno + 1
  end
end

because I was having to do backtraces by manually walking the stack
so often.  That's when I said, "enough is enough, this is stupid that
gdb can't do this."


> You can find more information about this in the list archives, in
> plenty of places; most recently Mark pulled together an implementation
> of "set i386 trust-frame-pointer".

Yeah, I couldn't comment at the time.  Mark's change was wrong.  
He said himself,

  You probably want to reset it to 0 before continuing your program
  since I found out that bad things happen with some of the tests
  in the gdb testsuite with this turned on.
	http://sourceware.org/ml/gdb/2005-04/msg00177.html

That's neither necessary nor acceptable.  Mark's initial
reading of the Sleep() vs SleepEx() was IMO not correct.
	http://sourceware.org/ml/gdb/2005-04/msg00156.html

Sleep() sets up a stack frame, then jumps to SleepEx().
SleepEx doesn't set up a stack frame, but that's fine --
Sleep() did.  This is another instance that bolsters my
"if the function MUST have stored the caller's pc/fp, assume
it did" method -- if you try to analyze SleepEx() where
the PC is, you'll see a frameless function.  But it's in
the middle of the stack; it can't be frameless.

(I was jumping in my seat while that whole conversation was
going on ;)

> That said, it may still be better than nothing.  I am still undecided.

Well, there's my thinking on the issue.

> > +  potentially_frameless = frame_relative_level (next_frame) == -1 
> > +                       || get_frame_type (next_frame) == SIGTRAMP_FRAME;
> 
> You want != NORMAL_FRAME.  You've ignored the dummy frame case.

Oh yeah, thanks.

> > +      /* We found a function-start address, 
> > +         or $pc is at 0x0 (someone jmp'ed thru NULL ptr).  */
> > +  if ((cache->pc != 0 || current_pc == 0)
> 
> No way that's right.  A jump through 0x0 is no different from a jump
> through any other unmapped, non-code address.  Normally one uses a
> different frame unwinder for that case.

OK, I didn't know the right practice.  Right now it goes through
i386_cache_frame.  It's "frameless", of course, but we don't have
a function symbol for it so cache->pc (WTF is up with that structure
variable name, anyway--it means the start address of the function for
this frame) is 0 (i.e. unset).  While I was porting my changes to
my FC2 system yesterday I had to add this bit --

  /* If we have no idea where this function began (so we can't analyze
     the prologue in any way), what should we assume?  Frameless or not?  
     It's a tough call.  On Linux systems, a call to a system library
     involves a couple of trampoline jumps that have no symbols,
     so to work correctly on Linux we MUST assume frameless.  */
 if (potentially_frameless && cache->pc == 0)
    {
      cache->saved_regs[I386_EIP_REGNUM] = 4;
      cache->saved_regs[I386_EBP_REGNUM] = -1;
    }

Which should catch the pc==0x0 case.  I didn't need this code on
MacOS X so I had to special case the pc==0x0.  I'll have to test
this, of course, but I can probably rely on this added code to 
do the right thing.

J

Re: The gdb x86 function prologue parser

[Jason Molenda]

Hi Eli,

On Wed, Jun 08, 2005 at 05:52:13PM +0300, Eli Zaretskii wrote:

> > +#include <stdint.h>
> 
> I don't think we can use stdint.h freely, as we still don't require a
> C9x compiler.  I see you needed it for things like uint8_t and
> uint32_t, which should be easily replaceable by suitable standard x86
> types.

I'll change the code.  I've become spoiled and taken to assuming
these types are available.

> Last, but not least, I'd surely appreciate some write-up, even in
> plain ASCII, about how prologue analyzers in general and the x86 one
> in particular work: that stuff is sorely needed in gdbint.texinfo.

OK, I'll look over gdbint next week and see what it says and where
something like that might fit in.

Thanks for the comments,

J

Re: The gdb x86 function prologue parser

[Eli Zaretskii]

> Date: Wed, 8 Jun 2005 10:00:57 -0700
> From: Jason Molenda <jason-swarelist@molenda.com>
> Cc: gdb-patches@sources.redhat.com, kettenis@jive.nl
> 
> > Last, but not least, I'd surely appreciate some write-up, even in
> > plain ASCII, about how prologue analyzers in general and the x86 one
> > in particular work: that stuff is sorely needed in gdbint.texinfo.
> 
> OK, I'll look over gdbint next week and see what it says

Alas, I think you will find that it says exactly nothing about this
important issue.

>  and where something like that might fit in.

There's a section called "Frames" in the "Algorithms" node.  I think
either a subsection of "Frame" or even a separate section "Function
prologue analysis" in that node would be a good place for this kind of
info.

> Thanks for the comments,

Thanks for working on this.

Re: The gdb x86 function prologue parser

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: Mark Kettenis <kettenis@jive.nl>
>> From: Jason Molenda <jmolenda@apple.com>
>> Date: Tue, 7 Jun 2005 22:51:36 -0700
>> 
>> --- i386-tdep.c	28 May 2005 16:44:28 -0000	1.213
>> +++ i386-tdep.c	8 Jun 2005 05:24:14 -0000
>> @@ -21,6 +21,7 @@
>>     Foundation, Inc., 59 Temple Place - Suite 330,
>>     Boston, MA 02111-1307, USA.  */
>>  
>> +#include <stdint.h>
>
> I don't think we can use stdint.h freely, as we still don't require a
> C9x compiler.  I see you needed it for things like uint8_t and
> uint32_t, which should be easily replaceable by suitable standard x86
> types.

Since this is a target (not native) file it needs to use types portable to
all hosts.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: The gdb x86 function prologue parser

[Jason Molenda]

Hi Andreas,

On Wed, Jun 08, 2005 at 09:58:12PM +0200, Andreas Schwab wrote:

> >> +#include <stdint.h>
> 
> Since this is a target (not native) file it needs to use types portable to
> all hosts.

I'll drop stdint.h because it requires ISO C99, but I don't understand
your comment.  Is there some environment where uint32_t isn't 4 bytes?  
This is how I was using uint32_t and uint8_t:

+      /* 81 /5 id    SUB r/m32, imm32 */
+      if (op == 0x81 && next_op == 0xec)
+        {
+          uint32_t imm32 = read_memory_integer (pc + 2, 4);
+          esp_change -= imm32;
+          pc += 6;
+          continue;
+        }

If code like this is wrong, I'd like to know.

Thanks,

Jason

Re: The gdb x86 function prologue parser

[Andreas Schwab]

Jason Molenda <jason-swarelist@molenda.com> writes:

> Hi Andreas,
>
> On Wed, Jun 08, 2005 at 09:58:12PM +0200, Andreas Schwab wrote:
>
>> >> +#include <stdint.h>
>> 
>> Since this is a target (not native) file it needs to use types portable to
>> all hosts.
>
> I'll drop stdint.h because it requires ISO C99, but I don't understand
> your comment.  Is there some environment where uint32_t isn't 4 bytes?  

If uint32_t is available at all, you can't universally assume that.  I was
referring to the statement from Eli, who said "replaceable by suitable
standard x86 types", but this file must be compilable on any host, not
only x86.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: The gdb x86 function prologue parser

[Michael Snyder]

Andreas Schwab wrote:
> Jason Molenda <jason-swarelist@molenda.com> writes:
> 
> 
>>Hi Andreas,
>>
>>On Wed, Jun 08, 2005 at 09:58:12PM +0200, Andreas Schwab wrote:
>>
>>
>>>>>+#include <stdint.h>
>>>
>>>Since this is a target (not native) file it needs to use types portable to
>>>all hosts.
>>
>>I'll drop stdint.h because it requires ISO C99, but I don't understand
>>your comment.  Is there some environment where uint32_t isn't 4 bytes?  
> 
> 
> If uint32_t is available at all, you can't universally assume that.  I was
> referring to the statement from Eli, who said "replaceable by suitable
> standard x86 types", but this file must be compilable on any host, not
> only x86.

Yes, you just have to make sure that the "standard x86 types"
are used only for target-side data objects, not host-side ones.

Re: The gdb x86 function prologue parser

[Michael Snyder]

Eli Zaretskii wrote:

> Last, but not least, I'd surely appreciate some write-up, even in
> plain ASCII, about how prologue analyzers in general and the x86 one
> in particular work: that stuff is sorely needed in gdbint.texinfo.

Prologue parsing is a black art, on its way to becoming a
lost art thanks to our increasing dependance on dwarf cfi.

It would be great if Jason could preserve some of what he's
having to relearn/reinvent -- but it wouldn't be fair of us
to require this of him.  This isn't new stuff, it's old stuff.

Not to discourage you, Jason -- your name will be revered
if you write this up, especially in the not-completely-unlikely
event that someone else has to do this again someday.

Re: The gdb x86 function prologue parser

[Eli Zaretskii]

> Date: Fri, 10 Jun 2005 13:28:38 -0700
> From: Michael Snyder <msnyder@redhat.com>
> CC: Jason Molenda <jmolenda@apple.com>, gdb-patches@sources.redhat.com,
>         kettenis@jive.nl
> 
> Prologue parsing is a black art, on its way to becoming a
> lost art thanks to our increasing dependance on dwarf cfi.

There are still quite a few platforms that cannot rely on CFI.  Not to
mention that currently there are many situations where I can't get a
decent backtrace even with DWARF CFI.

> It would be great if Jason could preserve some of what he's
> having to relearn/reinvent -- but it wouldn't be fair of us
> to require this of him.

I didn't require, I said I'd appreciate if he did that.

Someone must remind people from time to time to try documenting the
internals.  Otherwise gdbint.texinfo will never get better.

Re: The gdb x86 function prologue parser

[Mark Kettenis]

   From: Jason Molenda <jmolenda@apple.com>
   Date: Tue, 7 Jun 2005 22:51:36 -0700

Hi Jason,

Sorry I didn't reply before; I still was on vacation when you sent out
this mail.

   As was announced yesterday, we'll be transitioning from PPC to x86  
   over the next couple of years.  Over the last few months I had a  
   delightful little introduction to the x86 architecture (read: read  
   the IA32 PDFs until I couldn't see straight any more) and worked to  
   get our x86 gdb support up to snuff for this software release.

Seems like a step backwards to me, but hey who am I to judge Apple.
Watched the QuickTime video where your big boss announces the thingy.
Seems he's firmly in bed with Intel and not AMD.  Still makes me
wonder if Apple is going to provide a 64-bit version of MacOS X for
the new platform.

Another question that I have is what calling convention MacOS X will
use.  Is it something like the System V ABI where the caller is
supposed to clean up the stack after a function call?  I certainly
hope so since the Microsoft way of doing things poses a major headache
for prologue scanning.

   One of the biggest problems we found was the x86 function prologue  
   parser is remarkably weak.  We have a very mature and featureful  
   prologue parser on our ppc side and an amazing number of bugs were  
   directed my way as we had people pounding on the x86 side.  We aren't  
   using DWARF yet, so CFI can't save our bacon -- the prologue parser  
   has to work or our gdb fails.

I don't know to what extent your version of gdb is synched with the
FSF tree, but if it is anything close to gdb 6.x, then yes, you're
pretty much hosed if the prologue scanner fails.  It's no surprise to
me though that the prologue scanner appears a bit weak to you though.
It started out as a prologue scanner for the origional System V
compiler with some additions to older GCC versions when I took over.
At that point, GCC still had a fixed prologue.  When GCC 3.x started
scheduling prologue instructions, it also started generating usable
DWARF CFI, so whe took the conscious decision to rely on that and only
improve the prologue scanner on an as-needed basis.  Since GCC 3.x
targets for all major free OS'es all use DWARF by default, this means
that the prologue scanner really only handles some bits of
hand-optimized assembler.

   There are a couple classes of changes I made, and I spent today  
   trying to massage them into some kind of presentable form.  This is  
   not perfect -- well, to be honest, this is just a first sketch -- but  
   this is a HUGE improvement over the existing facilities.  I wrote the  
   code while under immense deadline pressure so I'm not particularly  
   interested in how I implemented any of it.  But changes akin to these  
   are necessary if you want to debug programs with optimized code on  
   the stack and without CFI.  I'll guarantee it.

It'd be great if we could improve the prologue scanner.  This reminds
me that I really should start testing -gstabs.

   Roughly speaking, here are the class of changes included in this patch.

   1. i386_match_insn() bug fixes.  It wouldn't work for an instruction  
   pattern of 1 byte, and it would never check anything beyond the 2nd  
   byte (notice where the final "return insn" is located).  I've added  
   patterns that can match prologue instructions, so exceptions had to  
   be added for the big two (push %ebp, mov %esp, %ebp) and the  
   equivalent ones used in the first frame (_start()).

Duh.  I must have been on drugs when I wrote that code.

   2. i386_frame_setup_skip_insns table expansion.  Because you can't  
   skip over an unknown instruction on x86 without knowing its length,  
   this was of paramount importance.  Initially I waited for users to  
   tell me of prologues that gdb was failing on, but this was taking too  
   long and there were too many instructions scheduled into prologues  
   for me to hear of them in time.  So I wrote a little maintenance  
   command (not included in the patch to keep things simple) which would  
   tell you if gdb could parse through the prologue of a given  
   function.  Then with a couple of shell scripts, I could have gdb try  
   to analyze the prologues of every function in every library on my  
   MacOS X system and show me the ones it failed on.  I'd add them to  
   this list.  I also made a little testsuite generator where the input  
   looks like

   # SOURCE: RedHat FedoraCore2 /lib/ld-2.3.3.so _dl_reloc_bad_type
   # PROLOGUE
      push %ebp
      shl $0x5, %ecx # [ 0xc1 0xe1 0x05 ]
      mov %esp, %ebp
      sub $0x8, %esp
   # EPILOGUE
      add $0x8, %esp
      pop %ebp
      ret

   and a script that transforms the patterns into a test program and a  
   Dejagnu expect script.  So you can ensure that you don't regress the  
   prologue parser.  This was the lesson we learned in writing our PPC  
   parser -- we have this wonderfully ornate parser with lots of  
   exceptions and known tricks, but no testsuite for it.  So whenever we  
   change it we're cringing because the gdb testsuite has nothing useful  
   in it.  (you need optimized, no debug info test cases to be sure it's  
   still working right).  The testsuite stuff isn't included in this  
   patch, but I'll put that together soon and send it along if anyone's  
   interested.

Certainly, if we change the prologue scanner to deal with new patterns
we should test these patterns in the testsuite.

   3. relatively minor changes to i386_analyze_frame_setup().  It had to  
   have the push %ebp as the very first instruction or it would give  
   up.  That's really bad -- the compiler can (and does) schedule all  
   sorts of stuff before that instruction.

I believe you.  I'm wondering though if the current way the prologue
scanner is built up makes sense for this new world of completely
scheduled prologues.

   4. new function, i386_find_esp_adjustments().  This is used in a  
   frameless leaf function where the compiler may create space on the  
   stack for local variables and stuff, but doesn't call anything so it  
   doesn't save the caller's frame pointer.  And it allows -fomit-leaf- 
   frame-pointer codegen to be debugged.  -fomit-frame-pointer is a  
   whole lot more complicated, but this wasn't so bad.  (we didn't end  
   up enabling -fomit-leaf-frame-pointer in this release because of the  
   schedule time constraints, but that's why I wrote it)

Being able to debug -fomit-frame-pointer code without CFI probably
means that instead of scanning the prologue, we'll have to scan the
complete function up to the current instruction pointer.  I really
wonder if that's the way we should go.

   5. Huge i386_frame_cache() changes.  There's no way around it, this  
   function is just not right.  It doesn't handle frameless functions  
   correctly at all.  It's written without a clear understanding of the  
   different classes of functions it needs to handle and works primarily  
   by luck.  And for goodness sakes, if we can't figure out anything  
   about a function that's not at the top of the stack, don't you think  
   it'd be reasonable to assume that the function has set up a stack  
   frame and saved the caller's EBP?  Sure seems like a reasonable  
   assumption to me.  Why can't this function do something even that  
   basic?  This function really cheesed my mellow.

Well, it handles most of the frameless functions encountered on a
GNU/Linux system with GCC 3.2 fine.  And no, assuming that a function
has set up a stack frame isn't right; it makes gdb silently skip
function calls in backtraces.  That can be very confusing.  As I've
stated before, I'd rather have a backtrace that's obviously wrong than
one that silently omits things.  But it's a trade-off.  Maybe
improving the prologue scanner can shift the balance far enough that
the assumption that a stack frame has been set up makes more sense
again.

Assuming that a function saves the previous value of %ebp is demanded
by the System V ABI, but GCC might violate the ABI for static
functions where it knows the caller has already saved %ebp.

   Mark, I want to say that I'm not directing any of these criticisms  
   towards you -- I've been looking over the changes you've made and  
   they're definite improvements over the existing code.  The existing  
   code bites, though.  I can't even begin to imagine how annoyed  
   developers using the FSF gdb on x86 must be.  The changes I'm sending  
   here are not a panacea/beautiful/perfect, but *functionally* they're  
   a huge improvement.  Now that our release is out there I'll be more  
   than happy to revisit the decisions/implementation that I came up  
   with on little sleep. :-)

Great.  I haven't looked at your patch in detail yet.  But it sounds
like some of the improvements can be made right away, so let's get
working on this ;-).

   Oh, and I ran my "find all prologues gdb can't parse" on a FedoraCore  
   2 system I have handy here at the office today and added the patterns  
   of the biggest offenders.  There are still a few patterns I need to  
   add to get 100% parsing success but I want to go home :-) so that'll  
   be for another day.

   We're in the middle of an all-week Apple developer's conference, so  
   my replies may not be very speedy (I'm off-line 10-12 hours a day; I  
   slipped away to get this patch together today) until the weekend.   
   But I'll try to stay on top of any questions or comments and address  
   them promptly.

No problem; I was having a vacation anyway ;-).

Mark

Re: The gdb x86 function prologue parser

[Mark Kettenis]

   Date: Wed, 8 Jun 2005 09:58:05 -0700
   From: Jason Molenda <jason-swarelist@molenda.com>

   Hi Daniel, thanks for the comments.

   On Wed, Jun 08, 2005 at 09:24:31AM -0400, Daniel Jacobowitz wrote:

   > I looked at your table.
   > 
   > (A) You've added jump instructions to it.  Assuming that I'm following
   > what you're doing with this table correctly, I'm not real comfortable
   > with that without special cases checking the targets of the jumps.

   These showed up in a couple of functions that had hand-written
   assembly at the start of the function.  Like there's one who has a
   special agreement with its caller about the contents of EDX, and
   it'd compare EDX to a value and then jump to an alternate location
   if it matched.  The only way this code is executed is if the PC is
   past the jmp instruction, so I wasn't too concerned about it --
   SOMEHOW we got past the jmp and back again.

You are aware of the fact that the prologue scanner is used for two
purposes?  I not, here they are:

1. Finding out the gory details about the stack frame being executed.

2. Determining the first bit of code after the prologue, i.e. the
   first bit of real code.

For (2) following jumps is usually a very bad thing to do.

That said, I'm not sure this dual usage of the prologue scanner really
makes sense these days.  There is a certain lack of consistency in gdb
how we handle this anyway.  Maybe the best thing to do is to not use
the prologue scanner for 2 at all.

   > 
   > > And for goodness sakes, if we can't figure out anything  
   > > about a function that's not at the top of the stack, don't you think  
   > > it'd be reasonable to assume that the function has set up a stack  
   > > frame and saved the caller's EBP?  
   > 
   > Because there is a GDB policy to determine information about the frame
   > based on the current frame, not based on where it lies on the stack.
   > I've experimented with this before; this change can have some weird
   > consequences... for instance, in any case where we can backtrace
   > through "foo" only because of the addition of this case, we won't be
   > able to backtrace through "foo" if it is on top of the stack.

   I'd say that's an expected behavior, but yes, it's true that this
   can happen.  It'd be great if the prologue analyzer never got confused
   and could always figure out how to find a function's caller's saved
   fp/pc, but even if we switch to using the opcodes disassembler so
   we never lose on another instruction, on MacOS X we can have libraries
   where the functions up the stack that have no symbols whatsoever.  
   We have no idea where the function might begin--all we know is a saved
   address in the middle of a function.  In such a situation, is it
   preferable that we can't backtrace past tricky functions like these?
   After a month of working on the x86 port, I got so frustrated I wrote
   a user command that could backtrace --

   define x86-bt
     set $frameno = 1
     set $cur_ebp = $ebp
     printf "frame 0 EBP: 0x%08x EIP: 0x%08x\n", $ebp, $eip
     x/1i $eip
     set $prev_ebp = *((uint32_t *) $cur_ebp)
     set $prev_eip = *((uint32_t *) ($cur_ebp + 4))
     while $prev_ebp != 0
       printf "frame %d EBP: 0x%08x EIP: 0x%08x\n", $frameno, $prev_ebp, $prev_eip
       x/1i $prev_eip
       set $cur_ebp = $prev_ebp
       set $prev_ebp = *((uint32_t *) $cur_ebp)
       set $prev_eip = *((uint32_t *) ($cur_ebp + 4))
       set $frameno = $frameno + 1
     end
   end

   because I was having to do backtraces by manually walking the stack
   so often.  That's when I said, "enough is enough, this is stupid that
   gdb can't do this."


   > You can find more information about this in the list archives, in
   > plenty of places; most recently Mark pulled together an implementation
   > of "set i386 trust-frame-pointer".

   Yeah, I couldn't comment at the time.  Mark's change was wrong.  

Oh, I'm so happy I don't live in that silly corporate world where you
grudgingly have to bite your tongue, because taking part in a
technical discussion would reveal a little too much about the
company's strategy ;-).

   He said himself,

     You probably want to reset it to 0 before continuing your program
     since I found out that bad things happen with some of the tests
     in the gdb testsuite with this turned on.
	   http://sourceware.org/ml/gdb/2005-04/msg00177.html

   That's neither necessary nor acceptable.  Mark's initial
   reading of the Sleep() vs SleepEx() was IMO not correct.
	   http://sourceware.org/ml/gdb/2005-04/msg00156.html

   Sleep() sets up a stack frame, then jumps to SleepEx().
   SleepEx doesn't set up a stack frame, but that's fine --
   Sleep() did.  This is another instance that bolsters my
   "if the function MUST have stored the caller's pc/fp, assume
   it did" method -- if you try to analyze SleepEx() where
   the PC is, you'll see a frameless function.  But it's in
   the middle of the stack; it can't be frameless.

Ah, but SleepEx() is a valid entry point itself.  That's where things
get tricky.  I'm not saying that there is no way out, but I simply
don't see it.

   > > +      /* We found a function-start address, 
   > > +         or $pc is at 0x0 (someone jmp'ed thru NULL ptr).  */
   > > +  if ((cache->pc != 0 || current_pc == 0)
   > 
   > No way that's right.  A jump through 0x0 is no different from a jump
   > through any other unmapped, non-code address.  Normally one uses a
   > different frame unwinder for that case.

   OK, I didn't know the right practice.  Right now it goes through
   i386_cache_frame.  It's "frameless", of course, but we don't have
   a function symbol for it so cache->pc (WTF is up with that structure
   variable name, anyway--it means the start address of the function for
   this frame) is 0 (i.e. unset).

It's probably historic; perhaps we should rename it.

Mark

Re: The gdb x86 function prologue parser

[Mark Kettenis]

   Date: Wed, 8 Jun 2005 23:25:56 -0700
   From: Jason Molenda <jason-swarelist@molenda.com>

   Hi Andreas,

   On Wed, Jun 08, 2005 at 09:58:12PM +0200, Andreas Schwab wrote:

   > >> +#include <stdint.h>
   > 
   > Since this is a target (not native) file it needs to use types portable to
   > all hosts.

   I'll drop stdint.h because it requires ISO C99, but I don't understand
   your comment.  Is there some environment where uint32_t isn't 4 bytes?  
   This is how I was using uint32_t and uint8_t:

   +      /* 81 /5 id    SUB r/m32, imm32 */
   +      if (op == 0x81 && next_op == 0xec)
   +        {
   +          uint32_t imm32 = read_memory_integer (pc + 2, 4);
   +          esp_change -= imm32;
   +          pc += 6;
   +          continue;
   +        }

   If code like this is wrong, I'd like to know.

Oh yes, it's very wrong ;-).  The function read_memory_integer returns
a signed integer which you're assigning to an unsigned integer type.

Conceptually, if we could assume int32_t was available on all systems,
that would be fine.  But since we cannot, using LONGEST is the safest
thing to do.

Mark

Re: The gdb x86 function prologue parser

[Eli Zaretskii]

> Date: Sun, 12 Jun 2005 09:48:07 +0200 (CEST)
> From: Mark Kettenis <mark.kettenis@xs4all.nl>
> CC: gdb-patches@sources.redhat.com
> 
> 1. Finding out the gory details about the stack frame being executed.
> 
> 2. Determining the first bit of code after the prologue, i.e. the
>    first bit of real code.
> 
> For (2) following jumps is usually a very bad thing to do.
> 
> That said, I'm not sure this dual usage of the prologue scanner really
> makes sense these days.  There is a certain lack of consistency in gdb
> how we handle this anyway.  Maybe the best thing to do is to not use
> the prologue scanner for 2 at all.

In what situations would GDB need to use (2), and what are the
alternative(s)?

Re: The gdb x86 function prologue parser

[Eli Zaretskii]

> Date: Sun, 12 Jun 2005 09:07:29 +0200 (CEST)
> From: Mark Kettenis <mark.kettenis@xs4all.nl>
> CC: gdb-patches@sources.redhat.com
> 
> Being able to debug -fomit-frame-pointer code without CFI probably
> means that instead of scanning the prologue, we'll have to scan the
> complete function up to the current instruction pointer.  I really
> wonder if that's the way we should go.

If this might make sense in some, possibly rare, situations, how about
if we do that conditioned on some user option?  It is IMHO much better
to have some way of dealing with such code than to have none.

> assuming that a function
> has set up a stack frame isn't right; it makes gdb silently skip
> function calls in backtraces.  That can be very confusing.  As I've
> stated before, I'd rather have a backtrace that's obviously wrong than
> one that silently omits things.

Again, a user option, off by default, that would produce a backtrace
with possibly omitted frames rather than a blatantly botched one,
could be a Good Thing here.

Just my $0.02.

Re: The gdb x86 function prologue parser

[Eli Zaretskii]

> Cc: Mark Kettenis <mark.kettenis@xs4all.nl>, jason-swarelist@molenda.com,
>         gdb-patches@sources.redhat.com
> From: Klee Dienes <klee@mit.edu>
> Date: Sun, 12 Jun 2005 07:59:01 -0400
> 
> (2) gets used by GDB when it wants to know where to set a breakpoint  
> (the idea being that when you say "break foo", it wants to insert the  
> breakpoint in foo() after the prologue has been fully executed).
> 
> It also gets used where GDB wants to know if it is inside a function  
> prologue for some reason (for example, so "next" knows if it has  
> stepped into a new function, or simply jumped interprocedurally).

Thanks.

(I'm collecting info for gdbint.texinfo, that's why I asked.)

Re: The gdb x86 function prologue parser

[Daniel Jacobowitz]

Hmm, Klee's message didn't make it to the list.

On Sun, Jun 12, 2005 at 04:17:38PM +0300, Eli Zaretskii wrote:
> > Cc: Mark Kettenis <mark.kettenis@xs4all.nl>, jason-swarelist@molenda.com,
> >         gdb-patches@sources.redhat.com
> > From: Klee Dienes <klee@mit.edu>
> > Date: Sun, 12 Jun 2005 07:59:01 -0400
> > 
> > (2) gets used by GDB when it wants to know where to set a breakpoint  
> > (the idea being that when you say "break foo", it wants to insert the  
> > breakpoint in foo() after the prologue has been fully executed).
> > 
> > It also gets used where GDB wants to know if it is inside a function  
> > prologue for some reason (for example, so "next" knows if it has  
> > stepped into a new function, or simply jumped interprocedurally).
> 
> Thanks.
> 
> (I'm collecting info for gdbint.texinfo, that's why I asked.)

FYI, the second part is decreasingly true now.  I only see one
reference to in_prologue left in infrun, and I'm pretty sure it's
obsolete.  Most uses are for the first reason.

And the first reason should, IMO, go away.  But it's a lot of work to
support stopping at the very beginning of a function, and still
printing arguments correctly, without help from the compiler.  We don't
have a clear plan on how to do that yet.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: The gdb x86 function prologue parser

[Michael Snyder]

Eli Zaretskii wrote:
>>Cc: Mark Kettenis <mark.kettenis@xs4all.nl>, jason-swarelist@molenda.com,
>>        gdb-patches@sources.redhat.com
>>From: Klee Dienes <klee@mit.edu>
>>Date: Sun, 12 Jun 2005 07:59:01 -0400
>>
>>(2) gets used by GDB when it wants to know where to set a breakpoint  
>>(the idea being that when you say "break foo", it wants to insert the  
>>breakpoint in foo() after the prologue has been fully executed).
>>
>>It also gets used where GDB wants to know if it is inside a function  
>>prologue for some reason (for example, so "next" knows if it has  
>>stepped into a new function, or simply jumped interprocedurally).
> 
> 
> Thanks.
> 
> (I'm collecting info for gdbint.texinfo, that's why I asked.)

Also so "step into" can step past the prologue and not stop
until it has reached user code.  Same basic purpose as 2),
but different context.

Re: The gdb x86 function prologue parser

[Eli Zaretskii]

> Date: Mon, 13 Jun 2005 11:21:08 -0700
> From: Michael Snyder <msnyder@redhat.com>
> CC: Klee Dienes <klee@mit.edu>, gdb-patches@sources.redhat.com
> 
> Also so "step into" can step past the prologue and not stop
> until it has reached user code.  Same basic purpose as 2),
> but different context.

Thanks.

Re: The gdb x86 function prologue parser

[Jason Molenda]

Hi Mark,

On Jun 12, 2005, at 12:07 AM, Mark Kettenis wrote:

> Sorry I didn't reply before; I still was on vacation when you sent out
> this mail.

Not at all.  Thanks for taking the time to look at it.

> Another question that I have is what calling convention MacOS X will
> use.  Is it something like the System V ABI where the caller is
> supposed to clean up the stack after a function call?

It's the System V IA-32 ABI except that small structs are returned in  
registers, stack frames are 16-byte aligned, and large types are kept  
at their natural alignment.  We haven't officially finalized our x86  
ABI yet, so it's possible we'll tweak it a bit before we're done.   
But it's a safe bet that any changes will be minor variations from  
the SysV IA32 ABI.

> I don't know to what extent your version of gdb is synched with the
> FSF tree, but if it is anything close to gdb 6.x, then yes, you're
> pretty much hosed if the prologue scanner fails.

Yeah, our currently-shipping sources are from a merge circa March  
2004.  We're starting to work on another merge to the current FSF tree.


> It's no surprise to
> me though that the prologue scanner appears a bit weak to you though.
> It started out as a prologue scanner for the origional System V
> compiler with some additions to older GCC versions when I took over.
> At that point, GCC still had a fixed prologue.  When GCC 3.x started
> scheduling prologue instructions, it also started generating usable
> DWARF CFI, so whe took the conscious decision to rely on that and only
> improve the prologue scanner on an as-needed basis.  Since GCC 3.x
> targets for all major free OS'es all use DWARF by default, this means
> that the prologue scanner really only handles some bits of
> hand-optimized assembler.


Ah!  Now it starts to make sense.  I couldn't understand how this had  
been so untested. :)

The one part I'm curious about -- does gdb get the CFI information  
out of gcc's eh_frame section or something?  How do developers debug  
KDE/GNOME applications, where many functions on their stack are from  
optimized libraries that don't have any debug info (except maybe  
eh_frame)?  It seems like these users should be tripping on these  
problems all the time.

>    4. new function, i386_find_esp_adjustments().  This is used in a
>
>
> Being able to debug -fomit-frame-pointer code without CFI probably

(I was only handling -fomit-leaf-frame-pointer)

> means that instead of scanning the prologue, we'll have to scan the
> complete function up to the current instruction pointer.  I really
> wonder if that's the way we should go.

Yeah, my method is not failsafe.  I was trying to handle the case  
where the prologue adjusts the stack pointer a constant amount for  
the lifetime of the function.  If a function has something like an  
alloca() call in it with a non-constant size, I'm going to lose.  If  
I had to scan through the entire function, I'm likely to hit  
instructions I don't understand and stop -- stack adjustments past  
that point would not work.  The only real way to do this is with CFI  
and changes like those in this proposal:

http://dwarf.freestandards.org/ShowIssue.php?issue=030812.2

(which, incidentally, is up for committee review tomorrow morning --  
there's some more detailed proposed text kicking around.)  I don't  
think gcc emits anything useful to help gdb in a case like alloca 
(argc) today, but should be possible to express.

> Well, it handles most of the frameless functions encountered on a
> GNU/Linux system with GCC 3.2 fine.

I tested the patch on a FedoraCore 2 system -- I have no idea which  
compiler was used to build the system, but I'd guess it's gcc 3.2 or  
3.3  A common idiom in this release of gcc is

0xa3b990 <htmlDefaultSAXHandlerInit>:   call   0xa3ba6d  
<htmlDefaultSAXHandlerInit+221>
0xa3b995 <htmlDefaultSAXHandlerInit+5>: add    $0x628df,%ecx
0xa3b99b <htmlDefaultSAXHandlerInit+11>:        push   %ebp
0xa3b99c <htmlDefaultSAXHandlerInit+12>:        mov    0xffffff5c(% 
ecx),%eax
0xa3b9a2 <htmlDefaultSAXHandlerInit+18>:        mov    0xffffff08(% 
ecx),%edx
0xa3b9a8 <htmlDefaultSAXHandlerInit+24>:        mov    %esp,%ebp
0xa3b9aa <htmlDefaultSAXHandlerInit+26>:        movl   $0x0,(%eax)
0xa3b9b0 <htmlDefaultSAXHandlerInit+32>:        mov    %edx,0x14(%eax)
0xa3b9b3 <htmlDefaultSAXHandlerInit+35>:        mov    0xfffffee0(% 
ecx),%edx
0xa3b9b9 <htmlDefaultSAXHandlerInit+41>:        movl   $0x0,0x4(%eax)

and

0xa3ba6d <htmlDefaultSAXHandlerInit+221>:       mov    (%esp),%ecx
0xa3ba70 <htmlDefaultSAXHandlerInit+224>:       ret

It's the standard sequence to find the current PC so static data can  
be located.  The add imm32, r32 isn't recognized by the prologue  
parser, so if you have a C file like

main ()
{
   foo ();
}
foo ()
{
   htmlDefaultSAXHandlerInit();  // invalid, but I'm not going to let  
it run
}

put a breakpoint on main, run the program and put a breakpoint after  
the mov %esp,%ebp:

(gdb) b *0xa3b9aa
Breakpoint 2 at 0xa3b9aa
(gdb) c
Continuing.

Breakpoint 2, 0x00a3b9aa in htmlDefaultSAXHandlerInit ()
    from /usr/lib/libxml.so.1
(gdb) bt
#0  0x00a3b9aa in htmlDefaultSAXHandlerInit () from /usr/lib/libxml.so.1
#1  0xfef1ecf8 in ?? ()
#2  0x080485c3 in foo () at /tmp/a.c:10
Previous frame identical to this frame (corrupt stack?)
(gdb)


It might seem like I'm being mean and picking the one routine that  
fails, but no, this idiom is used in many places and gcc often  
schedules it right into the prologue.  It's a system library, so  
there's no debug info to help you out.

I honestly think that gdb users are hitting this a lot more often  
than you'd think, but aren't reporting it.


> And no, assuming that a function
> has set up a stack frame isn't right; it makes gdb silently skip
> function calls in backtraces.  That can be very confusing.

On 0th frame, you're right, it's wrong to assume that the function  
sets up a stack frame.  But when you're off the 0th frame (and the  
next_frame isn't a signal trampoline or gdb dummy frame, etc.), the  
function set up a stack frame (assuming -fomit-frame-pointer codegen  
wasn't used, in which case we're in real trouble ;).  If we fail to  
parse the prologue instructions in that function, we should assume  
what must be the case - that it set up a stack frame.


> Assuming that a function saves the previous value of %ebp is demanded
> by the System V ABI, but GCC might violate the ABI for static
> functions where it knows the caller has already saved %ebp.

Yeah, rth was telling me the other week that gcc 4 can even ignore  
the ABI for such functions and pass arguments in registers.  My  
changes aren't the only problem in such a scenario.

> Great.  I haven't looked at your patch in detail yet.  But it sounds
> like some of the improvements can be made right away, so let's get
> working on this ;-).


Yeah, I need to integrate the feedback I received last week from  
Daniel and Eli -- lose inttypes.h, use the opcodes instruction parser  
instead of doing it myself -- so I'll get that stuff wrapped up and  
post an updated patch soon.

Thanks again for all the helpful notes,

Jason

Re: The gdb x86 function prologue parser

[Mark Kettenis]

   From: Jason Molenda <jmolenda@apple.com>
   Date: Mon, 13 Jun 2005 15:04:51 -0700

   Hi Mark,

   On Jun 12, 2005, at 12:07 AM, Mark Kettenis wrote:

   > Sorry I didn't reply before; I still was on vacation when you sent out
   > this mail.

   Not at all.  Thanks for taking the time to look at it.

   > Another question that I have is what calling convention MacOS X will
   > use.  Is it something like the System V ABI where the caller is
   > supposed to clean up the stack after a function call?

   It's the System V IA-32 ABI except that small structs are returned in  
   registers, stack frames are 16-byte aligned, and large types are kept  
   at their natural alignment.  We haven't officially finalized our x86  
   ABI yet, so it's possible we'll tweak it a bit before we're done.   
   But it's a safe bet that any changes will be minor variations from  
   the SysV IA32 ABI.

Great, that's rather similar to what OpenBSD and FreeBSD do.

   > It's no surprise to
   > me though that the prologue scanner appears a bit weak to you though.
   > It started out as a prologue scanner for the origional System V
   > compiler with some additions to older GCC versions when I took over.
   > At that point, GCC still had a fixed prologue.  When GCC 3.x started
   > scheduling prologue instructions, it also started generating usable
   > DWARF CFI, so whe took the conscious decision to rely on that and only
   > improve the prologue scanner on an as-needed basis.  Since GCC 3.x
   > targets for all major free OS'es all use DWARF by default, this means
   > that the prologue scanner really only handles some bits of
   > hand-optimized assembler.


   Ah!  Now it starts to make sense.  I couldn't understand how this had  
   been so untested. :)

   The one part I'm curious about -- does gdb get the CFI information  
   out of gcc's eh_frame section or something?  How do developers debug  
   KDE/GNOME applications, where many functions on their stack are from  
   optimized libraries that don't have any debug info (except maybe  
   eh_frame)?  It seems like these users should be tripping on these  
   problems all the time.

Yup.  We prefer .debug_frame but if that's not available we suck in
.eh_frame.  So anything that's compiled with -fexceptions (wich
implies all C++ code) basically has usable CFI.

   > Well, it handles most of the frameless functions encountered on a
   > GNU/Linux system with GCC 3.2 fine.

   I tested the patch on a FedoraCore 2 system -- I have no idea which  
   compiler was used to build the system, but I'd guess it's gcc 3.2 or  
   3.3

Hell, it's probably a heavily snapshot of something in between.  I've
mostly given up on Linux, because everyone seems to want to have the
latest bugs over there ;-).

   Breakpoint 2, 0x00a3b9aa in htmlDefaultSAXHandlerInit ()
       from /usr/lib/libxml.so.1
   (gdb) bt
   #0  0x00a3b9aa in htmlDefaultSAXHandlerInit () from /usr/lib/libxml.so.1
   #1  0xfef1ecf8 in ?? ()
   #2  0x080485c3 in foo () at /tmp/a.c:10
   Previous frame identical to this frame (corrupt stack?)
   (gdb)


   It might seem like I'm being mean and picking the one routine that  
   fails, but no, this idiom is used in many places and gcc often  
   schedules it right into the prologue.  It's a system library, so  
   there's no debug info to help you out.

   I honestly think that gdb users are hitting this a lot more often  
   than you'd think, but aren't reporting it.

I sometimes wonder whether people are using gdb at all...

...then I find it incredibly stupid that vendors of an Open Source
operating system ship libraries without debugging information.

   > And no, assuming that a function
   > has set up a stack frame isn't right; it makes gdb silently skip
   > function calls in backtraces.  That can be very confusing.

   On 0th frame, you're right, it's wrong to assume that the function  
   sets up a stack frame.  But when you're off the 0th frame (and the  
   next_frame isn't a signal trampoline or gdb dummy frame, etc.), the  
   function set up a stack frame (assuming -fomit-frame-pointer codegen  
   wasn't used, in which case we're in real trouble ;).  If we fail to  
   parse the prologue instructions in that function, we should assume  
   what must be the case - that it set up a stack frame.

I'm fairly sure I've seen cases that I've seen GCC generate frameless
functions even without -fomit-frame-pointer.

   > Great.  I haven't looked at your patch in detail yet.  But it sounds
   > like some of the improvements can be made right away, so let's get
   > working on this ;-).

   Yeah, I need to integrate the feedback I received last week from  
   Daniel and Eli -- lose inttypes.h, use the opcodes instruction parser  
   instead of doing it myself -- so I'll get that stuff wrapped up and  
   post an updated patch soon.

Looking forward to it.

Mark

Re: The gdb x86 function prologue parser

[Daniel Jacobowitz]

On Tue, Jun 14, 2005 at 12:34:55AM +0200, Mark Kettenis wrote:
>    From: Jason Molenda <jmolenda@apple.com>
>    Date: Mon, 13 Jun 2005 15:04:51 -0700
> 
>    Ah!  Now it starts to make sense.  I couldn't understand how this had  
>    been so untested. :)
> 
>    The one part I'm curious about -- does gdb get the CFI information  
>    out of gcc's eh_frame section or something?  How do developers debug  
>    KDE/GNOME applications, where many functions on their stack are from  
>    optimized libraries that don't have any debug info (except maybe  
>    eh_frame)?  It seems like these users should be tripping on these  
>    problems all the time.
> 
> Yup.  We prefer .debug_frame but if that's not available we suck in
> .eh_frame.  So anything that's compiled with -fexceptions (wich
> implies all C++ code) basically has usable CFI.

> I sometimes wonder whether people are using gdb at all...

(me too)

> ...then I find it incredibly stupid that vendors of an Open Source
> operating system ship libraries without debugging information.

More and more vendors are shipping libraries with optional debugging
information.  This is what objcopy --only-keep-debug was invented for. 
For Debian, I also have a couple of hacks to ship unwind information
for some libraries we don't want to provide debug information for by
default (like glibc).

If you install libc6-dbg, backtraces will suddenly Work Better.  No
other intervention required.

For the next release of Debian I hope we'll be using this feature even
more heavily.

-- 
Daniel Jacobowitz
CodeSourcery, LLC