Re: Broken prologue skipping with non-returning function

[Jonathan Larmour <jifl@eCosCentric.com>]

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

Daniel Jacobowitz wrote:
> On Fri, Sep 19, 2008 at 03:32:59PM +0100, Jonathan Larmour wrote:
>> We end up with a .loc for both lines 6 and 7 with no intervening
>> instructions. gdb's symtab.c:find_pc_sect_line() looks for when the pc
>> changes to something different and thus ends up returning a symtab_and_line
>> indicating that the line at that pc is at the 'if' and runs from the start
>> of the function to the ldr after the .loc 1 9 0.
> 
> skip_prologue_using_sal is supposed to detect this.  We have a
> patch to improve it in our internal tree that we haven't gotten round
> to yet.  Here it is; I do not remember what the language_asm check was
> really about, except that I'm sure it came up running the gdb
> testsuite, so removing it and running asm-source.exp would probably
> explain it.

Thanks! The current arm-tdep.c doesn't presently use
skip_prologue_using_sal() however. At a guess that's also lurking in your
internal tree, but nevermind, I'm attaching a patch assuming that's useful.

With both of these (and my tentative patch reverted) I can confirm it works
as expected.

If it helps, I have write after approval perms, and a valid current FSF
copyright assignment, including disclaimer with my current employer. I
noticed I need to update my email address in the MAINTAINERS file which I
can do too.

I can check in your change too. If so, presumably you already have a
ChangeLog entry you'd like me to use to ease your merges?

Jifl

2008-09-19  Jonathan Larmour  <jifl@eCosCentric.com>

	* arm-tdep.c (arm_skip_prologue): Call skip_prologue_using_sal
	instead of determining symbol and line info directly.

-- 
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
------["Si fractum non sit, noli id reficere"]------       Opinions==mine
    >>>> Visit us on stand 905 at the Embedded Systems Show 2008 <<<<
    >>>> Oct 1-2, NEC, Birmingham, UK http://www.embedded.co.uk  <<<<

[-- Attachment #2: arm.skip.prologue.using.sal.patch --]
[-- Type: text/x-patch, Size: 2441 bytes --]

--- arm-tdep.c.old	2008-09-19 17:01:32.000000000 +0100
+++ arm-tdep.c	2008-09-19 17:23:42.000000000 +0100
@@ -519,43 +519,40 @@ arm_skip_prologue (struct gdbarch *gdbar
 {
   unsigned long inst;
   CORE_ADDR skip_pc;
-  CORE_ADDR func_addr, func_end = 0;
-  char *func_name;
+  CORE_ADDR func_addr, limit_pc;
   struct symtab_and_line sal;
 
   /* If we're in a dummy frame, don't even try to skip the prologue.  */
   if (deprecated_pc_in_call_dummy (pc))
     return pc;
 
-  /* See what the symbol table says.  */
-
-  if (find_pc_partial_function (pc, &func_name, &func_addr, &func_end))
-    {
-      struct symbol *sym;
-
-      /* Found a function.  */
-      sym = lookup_symbol (func_name, NULL, VAR_DOMAIN, NULL);
-      if (sym && SYMBOL_LANGUAGE (sym) != language_asm)
-        {
-	  /* Don't use this trick for assembly source files.  */
-	  sal = find_pc_line (func_addr, 0);
-	  if ((sal.line != 0) && (sal.end < func_end))
-	    return sal.end;
-        }
-    }
-
-  /* Can't find the prologue end in the symbol table, try it the hard way
-     by disassembling the instructions.  */
-
+  /* See if we can determine the end of the prologue via the symbol table.
+     If so, then return either PC, or the PC after the prologue, whichever
+     is greater.  */
+  if (find_pc_partial_function (pc, NULL, &func_addr, NULL))
+    {
+      CORE_ADDR post_prologue_pc = skip_prologue_using_sal (func_addr);
+      if (post_prologue_pc != 0)
+	return max (pc, post_prologue_pc);
+    }
+
+  /* Can't determine prologue from the symbol table, need to examine
+     instructions.  */
+
+  /* Find an upper limit on the function prologue using the debug
+     information.  If the debug information could not be used to provide
+     that bound, then use an arbitrary large number as the upper bound.  */
   /* Like arm_scan_prologue, stop no later than pc + 64. */
-  if (func_end == 0 || func_end > pc + 64)
-    func_end = pc + 64;
+  limit_pc = skip_prologue_using_sal (pc);
+  if (limit_pc == 0)
+    limit_pc = pc + 64;          /* Magic.  */
+
 
   /* Check if this is Thumb code.  */
   if (arm_pc_is_thumb (pc))
-    return thumb_analyze_prologue (gdbarch, pc, func_end, NULL);
+    return thumb_analyze_prologue (gdbarch, pc, limit_pc, NULL);
 
-  for (skip_pc = pc; skip_pc < func_end; skip_pc += 4)
+  for (skip_pc = pc; skip_pc < limit_pc; skip_pc += 4)
     {
       inst = read_memory_unsigned_integer (skip_pc, 4);