This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[rfc] Consoldiate prologue skipping code - fix expand_line_sal_maybe internal error


Hello,

I'm running into the following internal error:

/home/uweigand/fsf/gdb-head/gdb/breakpoint.c:6705: 
internal-error: expand_line_sal_maybe: Assertion `found' failed.

This happens in a particular application on Cell/B.E. using multiple
SPU threads running the same SPU executable.  Once the first SPU thread
has started, I'm setting a breakpoint on "func" and continue.  As soon
as the second SPU thread then also starts, the breakpoint engine tries
to set the breakpoint on that thread's copy of "func" as well; at this
point the internal error hits.

The problem is related to prologue skipping.  The function in question
has been compiled with optimization, and looks like this:

0003e298 <func>:
main.c:71
   3e298:       40 fe f0 1a     il      $26,-544
   3e29c:       12 7e 84 a0     hbrr    3e31c <func+0x84>,3d6c0 <mfc_get_block>
main.c:90
   3e2a0:       40 80 40 05     il      $5,128  # 80
main.c:71
   3e2a4:       24 00 40 80     stqd    $0,16($1)
main.c:90
   3e2a8:       43 fd 00 03     ila     $3,260608       # 3fa00
main.c:71
   3e2ac:       24 ff 00 d3     stqd    $83,-64($1)
   3e2b0:       24 fc 80 dd     stqd    $93,-224($1)
   3e2b4:       24 fc 40 de     stqd    $94,-240($1)
   3e2b8:       24 fc 00 ff     stqd    $127,-256($1)
   3e2bc:       24 ff c0 d0     stqd    $80,-16($1)
   3e2c0:       24 ff 80 d1     stqd    $81,-32($1)
   3e2c4:       24 ff 40 d2     stqd    $82,-48($1)
   3e2c8:       24 fe c0 d4     stqd    $84,-80($1)
   3e2cc:       24 fe 80 d5     stqd    $85,-96($1)
   3e2d0:       24 fe 40 d6     stqd    $86,-112($1)
   3e2d4:       24 fe 00 d7     stqd    $87,-128($1)
   3e2d8:       24 fd c0 d8     stqd    $88,-144($1)
   3e2dc:       24 fd 80 d9     stqd    $89,-160($1)
   3e2e0:       24 fd 40 da     stqd    $90,-176($1)
   3e2e4:       35 90 00 00     hbrp    3e2e4 <func+0x4c>,$0
   3e2e8:       24 fd 00 db     stqd    $91,-192($1)
   3e2ec:       24 fc c0 dc     stqd    $92,-208($1)
   3e2f0:       40 20 00 7f     nop     $127
   3e2f4:       24 f7 80 81     stqd    $1,-544($1)
   3e2f8:       18 06 80 81     a       $1,$1,$26
main.c:82
   3e2fc:       33 82 b4 92     lqr     $18,3f8a0 <g_elem+0x20>
main.c:90
   3e300:       40 20 00 7f     nop     $127
   3e304:       33 82 f5 86     lqr     $6,3fab0 <g_tag>
main.c:71
   3e308:       1c 08 00 ff     ai      $127,$1,32
main.c:90
   3e30c:       33 82 ae 84     lqr     $4,3f880 <g_elem>

Note that 3e308 is the last prologue instruction (setting up the
frame pointer), so prologue skipping determines 3e30c as the
address to set a breakpoint on "func".  This address is associated
with the line main.c:90.

When expand_line_sal_maybe tries to look up this location after
new "shared libraries" (SPU executables) became available, it will
discover the adresses 3e2a0, because this is the first address
where line main.c:90 occurs.

Now, expand_line_sal_maybe in certain situations tries to once again
skip the prologue.  But none of these apply here:

- If just a single instance is found
  (we have two instances across two separate "shared libraries")

- For SALs installed with explicit line number
  (we set the breakpoint on a function name)

- If the address detected is at the very beginning of the function
  (it is not -- that would be 3e298, not 3e2a0)

Thus, the 3e2a0 addresses are never adjusted.  In addition, since we
now found two SALs, we're entering the sanity check that one of those
two ought to be at the same address as the original SAL, i.e. 3e30c:

  if (original_pc)
    {
      found = 0;
      for (i = 0; i < expanded.nelts; ++i)
        if (expanded.sals[i].pc == original_pc)
          {
            found = 1;
            break;
          }
      gdb_assert (found);
    }

But this is not the case, and thus the assert hits.

Looking somewhat further into the root cause of this problem, it seems to me
that there are a couple of different places in GDB today that try to skip
prologues -- and they all use similar but slightly different strategies:

- symtab.c:find_function_start_sal

  This is the most comprehensive algorithm and tries to take care of various
  situations (__main prologue, inlined functions) in addition to the gdbarch
  prologue skipping.  It works only on a function symbol (with debug info).

- linespec.c:minsym_found

  This is used when we have no debug symbol, but just a minsym for a function.
  It reimplements certain features of find_function_start_sal, but not all.
  (E.g. it doesn't attempt to handle __main prologue skipping.)

- breakpoint.c:skip_prologue_sal

  As opposed to find_function_start_sal, this works on an already filled in
  SAL and only modifies it to skip the prologue if appropriate.  Because it
  ultimately calls down into find_function_start_sal, it works only if it
  can resolve the SAL back to a function with full debug symbol.

- breakpoint.c:expand_line_sal_maybe

  This sometimes calls skip_prologue_sal, but in other cases attempt to do
  prologue skipping by hand, falling back directly on the gdbarch routine
  -- and therefore ignoring the other checks implemented elsewhere.


Now, given all these differences, it is not completely surprising that
these occasionally reach different results.  The problem is that in
expand_line_sal_maybe, this means triggering the ICE :-(


I'd propose to fix this by fundamentally converging all the separate
implementations into a single one.  This function would implement the
skip_prologue_sal functionality.  It should work on both functions
with and without debug symbols, and it should respect the SAL's
properties like explicit_pc or explicit_line.  In short, it should
always be valid to call skip_prologue_sal on any SAL and it will do
the right thing. (Calling it a second time on the same SAL will then
never change it any further.)

This will allow us to implement find_function_start_sal and minsym_found
by simply setting up a SAL and then calling skip_prologue_sal.  Also,
in expand_line_sal_maybe we can simply unconditionally call skip_prologue_sal
on all SALs we find.  This should then always get us back the original
result for the initial SAL ...

The following patch implements this idea, and it indeed fixes the
test case I mentioned above.

Tested on powerpc-linux with no regressions.

Any comments?  If there are no objections, I'm planning to commit
this in a week or so.

Bye,
Ulrich


ChangeLog:

	* breakpoint.c (expand_line_sal_maybe): Always call skip_prologue_sal.
	(skip_prologue_sal): Remove local definition.
	(resolve_sal_pc): Remove now unnecessary code.
	* linespec.c (minsym_found): Call skip_prologue_sal.
	* symtab.c (find_function_start_pc): Remove.
	(find_function_start_sal): Extract prologue skipping into ...
	(skip_prologue_sal): ... this new function.  Handle code both
	with and without debug info.  Respect SAL's explicit_pc and
	explicit_line flags.  Inline old find_function_start_pc.
	* symtab.h (find_function_start_pc): Remove.
	(skip_prologue_sal): Add prototype.


Index: gdb/breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.462
diff -u -p -r1.462 breakpoint.c
--- gdb/breakpoint.c	16 Mar 2010 08:42:20 -0000	1.462
+++ gdb/breakpoint.c	19 Mar 2010 14:35:25 -0000
@@ -226,8 +226,6 @@ static void disable_trace_command (char 
 
 static void trace_pass_command (char *, int);
 
-static void skip_prologue_sal (struct symtab_and_line *sal);
-
 
 /* Flag indicating that a command has proceeded the inferior past the
    current breakpoint.  */
@@ -6644,39 +6642,13 @@ expand_line_sal_maybe (struct symtab_and
 		  remove_sal (&expanded, i);
 		  --i;
 		}
-	      else if (func_addr == pc)	    
-		{	     
-		  /* We're at beginning of a function, and should
-		     skip prologue.  */
-		  struct symbol *sym = find_pc_function (pc);
-		  if (sym)
-		    expanded.sals[i] = find_function_start_sal (sym, 1);
-		  else
-		    {
-		      /* Since find_pc_partial_function returned true,
-			 we should really always find the section here.  */
-		      struct obj_section *section = find_pc_section (pc);
-		      if (section)
-			{
-			  struct gdbarch *gdbarch
-			    = get_objfile_arch (section->objfile);
-			  expanded.sals[i].pc
-			    = gdbarch_skip_prologue (gdbarch, pc);
-			}
-		    }
-		}
 	    }
 	}
     }
-  else
-    {
-      for (i = 0; i < expanded.nelts; ++i)
-	{
-	  /* If this SAL corresponds to a breakpoint inserted using a
-	     line number, then skip the function prologue if necessary.  */
-	  skip_prologue_sal (&expanded.sals[i]);
-	}
-    }
+
+  /* Skip the function prologue if necessary.  */
+  for (i = 0; i < expanded.nelts; ++i)
+    skip_prologue_sal (&expanded.sals[i]);
 
   do_cleanups (old_chain);
 
@@ -7177,37 +7149,6 @@ break_command_1 (char *arg, int flag, in
 
 
 
-/* Adjust SAL to the first instruction past the function prologue.
-   The end of the prologue is determined using the line table from
-   the debugging information.  explicit_pc and explicit_line are
-   not modified.
-
-   If SAL is already past the prologue, then do nothing.  */
-
-static void
-skip_prologue_sal (struct symtab_and_line *sal)
-{
-  struct symbol *sym;
-  struct symtab_and_line start_sal;
-  struct cleanup *old_chain;
-
-  old_chain = save_current_space_and_thread ();
-
-  sym = find_pc_function (sal->pc);
-  if (sym != NULL)
-    {
-      start_sal = find_function_start_sal (sym, 1);
-      if (sal->pc < start_sal.pc)
-	{
-	  start_sal.explicit_line = sal->explicit_line;
-	  start_sal.explicit_pc = sal->explicit_pc;
-	  *sal = start_sal;
-	}
-    }
-
-  do_cleanups (old_chain);
-}
-
 /* Helper function for break_command_1 and disassemble_command.  */
 
 void
@@ -7225,12 +7166,7 @@ resolve_sal_pc (struct symtab_and_line *
       /* If this SAL corresponds to a breakpoint inserted using
          a line number, then skip the function prologue if necessary.  */
       if (sal->explicit_line)
-	{
-	  /* Preserve the original line number.  */
-	  int saved_line = sal->line;
-	  skip_prologue_sal (sal);
-	  sal->line = saved_line;
-	}
+	skip_prologue_sal (sal);
     }
 
   if (sal->section == 0 && sal->symtab != NULL)
Index: gdb/linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.98
diff -u -p -r1.98 linespec.c
--- gdb/linespec.c	9 Mar 2010 18:09:07 -0000	1.98
+++ gdb/linespec.c	19 Mar 2010 14:35:27 -0000
@@ -1956,26 +1956,7 @@ minsym_found (int funfirstline, struct m
     values.sals[0] = find_pc_sect_line (pc, NULL, 0);
 
   if (funfirstline)
-    {
-      struct symtab_and_line sal;
-
-      values.sals[0].pc = find_function_start_pc (gdbarch,
-						  values.sals[0].pc,
-						  values.sals[0].section);
-
-      sal = find_pc_sect_line (values.sals[0].pc, values.sals[0].section, 0);
-
-      /* Check if SKIP_PROLOGUE left us in mid-line, and the next
-	 line is still part of the same function.  If there is no
-	 line information here, sal.pc will be the passed in PC.  */
-      if (sal.pc != values.sals[0].pc
-	  && (lookup_minimal_symbol_by_pc_section (values.sals[0].pc,
-						   values.sals[0].section)
-	      == lookup_minimal_symbol_by_pc_section (sal.end,
-						      values.sals[0].section)))
-	/* Recalculate the line number (might not be N+1).  */
-	values.sals[0] = find_pc_sect_line (sal.end, values.sals[0].section, 0);
-    }
+    skip_prologue_sal (&values.sals[0]);
 
   values.nelts = 1;
   return values;
Index: gdb/symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.227
diff -u -p -r1.227 symtab.c
--- gdb/symtab.c	15 Mar 2010 17:29:36 -0000	1.227
+++ gdb/symtab.c	19 Mar 2010 14:35:27 -0000
@@ -2240,26 +2240,6 @@ find_pc_line_pc_range (CORE_ADDR pc, COR
   return sal.symtab != 0;
 }
 
-/* Given a function start address PC and SECTION, find the first
-   address after the function prologue.  */
-CORE_ADDR
-find_function_start_pc (struct gdbarch *gdbarch,
-			CORE_ADDR pc, struct obj_section *section)
-{
-  /* If the function is in an unmapped overlay, use its unmapped LMA address,
-     so that gdbarch_skip_prologue has something unique to work on.  */
-  if (section_is_overlay (section) && !section_is_mapped (section))
-    pc = overlay_unmapped_address (pc, section);
-
-  pc += gdbarch_deprecated_function_start_offset (gdbarch);
-  pc = gdbarch_skip_prologue (gdbarch, pc);
-
-  /* For overlays, map pc back into its mapped VMA range.  */
-  pc = overlay_mapped_address (pc, section);
-
-  return pc;
-}
-
 /* Given a function start address FUNC_ADDR and SYMTAB, find the first
    address for that function that has an entry in SYMTAB's line info
    table.  If such an entry cannot be found, return FUNC_ADDR
@@ -2309,52 +2289,109 @@ skip_prologue_using_lineinfo (CORE_ADDR 
 struct symtab_and_line
 find_function_start_sal (struct symbol *sym, int funfirstline)
 {
-  struct block *block = SYMBOL_BLOCK_VALUE (sym);
-  struct objfile *objfile = lookup_objfile_from_block (block);
-  struct gdbarch *gdbarch = get_objfile_arch (objfile);
+  struct symtab_and_line sal;
 
+  fixup_symbol_section (sym, NULL);
+  sal = find_pc_sect_line (BLOCK_START (SYMBOL_BLOCK_VALUE (sym)),
+			   SYMBOL_OBJ_SECTION (sym), 0);
+
+  if (funfirstline)
+    skip_prologue_sal (&sal);
+
+  return sal;
+}
+
+/* Adjust SAL to the first instruction past the function prologue.
+   If the PC was explicitly specified, the SAL is not changed.
+   If the line number was explicitly specified, at most the SAL's PC
+   is updated.  If SAL is already past the prologue, then do nothing.  */
+void
+skip_prologue_sal (struct symtab_and_line *sal)
+{
+  struct symbol *sym;
+  struct symtab_and_line start_sal;
+  struct cleanup *old_chain;
   CORE_ADDR pc;
-  struct symtab_and_line sal;
+  struct obj_section *section;
+  const char *name;
+  struct objfile *objfile;
+  struct gdbarch *gdbarch;
   struct block *b, *function_block;
 
-  struct cleanup *old_chain;
+  /* Do not change the SAL is PC was specified explicitly.  */
+  if (sal->explicit_pc)
+    return;
 
   old_chain = save_current_space_and_thread ();
-  switch_to_program_space_and_thread (objfile->pspace);
+  switch_to_program_space_and_thread (sal->pspace);
 
-  pc = BLOCK_START (block);
-  fixup_symbol_section (sym, objfile);
-  if (funfirstline)
+  sym = find_pc_sect_function (sal->pc, sal->section);
+  if (sym != NULL)
     {
-      /* Skip "first line" of function (which is actually its prologue).  */
-      pc = find_function_start_pc (gdbarch, pc, SYMBOL_OBJ_SECTION (sym));
+      fixup_symbol_section (sym, NULL);
+
+      pc = BLOCK_START (SYMBOL_BLOCK_VALUE (sym));
+      section = SYMBOL_OBJ_SECTION (sym);
+      name = SYMBOL_LINKAGE_NAME (sym);
+      objfile = SYMBOL_SYMTAB (sym)->objfile;
+    }
+  else
+    {
+      struct minimal_symbol *msymbol
+        = lookup_minimal_symbol_by_pc_section (sal->pc, sal->section);
+      if (msymbol == NULL)
+	{
+	  do_cleanups (old_chain);
+	  return;
+	}
+
+      pc = SYMBOL_VALUE_ADDRESS (msymbol);
+      section = SYMBOL_OBJ_SECTION (msymbol);
+      name = SYMBOL_LINKAGE_NAME (msymbol);
+      objfile = msymbol_objfile (msymbol);
     }
-  sal = find_pc_sect_line (pc, SYMBOL_OBJ_SECTION (sym), 0);
+
+  gdbarch = get_objfile_arch (objfile);
+
+  /* If the function is in an unmapped overlay, use its unmapped LMA address,
+     so that gdbarch_skip_prologue has something unique to work on.  */
+  if (section_is_overlay (section) && !section_is_mapped (section))
+    pc = overlay_unmapped_address (pc, section);
+
+  /* Skip "first line" of function (which is actually its prologue).  */
+  pc += gdbarch_deprecated_function_start_offset (gdbarch);
+  pc = gdbarch_skip_prologue (gdbarch, pc);
+
+  /* For overlays, map pc back into its mapped VMA range.  */
+  pc = overlay_mapped_address (pc, section);
+
+  /* Calculate line number.  */
+  start_sal = find_pc_sect_line (pc, section, 0);
 
   /* Check if gdbarch_skip_prologue left us in mid-line, and the next
      line is still part of the same function.  */
-  if (sal.pc != pc
-      && BLOCK_START (block) <= sal.end
-      && sal.end < BLOCK_END (block))
+  if (start_sal.pc != pc
+      && (sym? (BLOCK_START (SYMBOL_BLOCK_VALUE (sym)) <= start_sal.end
+	        && start_sal.end < BLOCK_END (SYMBOL_BLOCK_VALUE (sym)))
+          : (lookup_minimal_symbol_by_pc_section (start_sal.end, section)
+             == lookup_minimal_symbol_by_pc_section (pc, section))))
     {
       /* First pc of next line */
-      pc = sal.end;
+      pc = start_sal.end;
       /* Recalculate the line number (might not be N+1).  */
-      sal = find_pc_sect_line (pc, SYMBOL_OBJ_SECTION (sym), 0);
+      start_sal = find_pc_sect_line (pc, section, 0);
     }
 
   /* On targets with executable formats that don't have a concept of
      constructors (ELF with .init has, PE doesn't), gcc emits a call
      to `__main' in `main' between the prologue and before user
      code.  */
-  if (funfirstline
-      && gdbarch_skip_main_prologue_p (gdbarch)
-      && SYMBOL_LINKAGE_NAME (sym)
-      && strcmp (SYMBOL_LINKAGE_NAME (sym), "main") == 0)
+  if (gdbarch_skip_main_prologue_p (gdbarch)
+      && name && strcmp (name, "main") == 0)
     {
       pc = gdbarch_skip_main_prologue (gdbarch, pc);
       /* Recalculate the line number (might not be N+1).  */
-      sal = find_pc_sect_line (pc, SYMBOL_OBJ_SECTION (sym), 0);
+      start_sal = find_pc_sect_line (pc, section, 0);
     }
 
   /* If we still don't have a valid source line, try to find the first
@@ -2365,19 +2402,35 @@ find_function_start_sal (struct symbol *
      the case with the DJGPP target using "gcc -gcoff" when the
      compiler inserted code after the prologue to make sure the stack
      is aligned.  */
-  if (funfirstline && sal.symtab == NULL)
+  if (sym && start_sal.symtab == NULL)
     {
       pc = skip_prologue_using_lineinfo (pc, SYMBOL_SYMTAB (sym));
       /* Recalculate the line number.  */
-      sal = find_pc_sect_line (pc, SYMBOL_OBJ_SECTION (sym), 0);
+      start_sal = find_pc_sect_line (pc, section, 0);
     }
 
-  sal.pc = pc;
-  sal.pspace = objfile->pspace;
+  do_cleanups (old_chain);
+
+  /* If we're already past the prologue, leave SAL unchanged.  Otherwise
+     forward SAL to the end of the prologue.  */
+  if (sal->pc >= pc)
+    return;
+
+  sal->pc = pc;
+  sal->section = section;
+
+  /* Unless the explicit_line flag was set, update the SAL line
+     and symtab to correspond to the modified PC location.  */
+  if (sal->explicit_line)
+    return;
+
+  sal->symtab = start_sal.symtab;
+  sal->line = start_sal.line;
+  sal->end = start_sal.end;
 
   /* Check if we are now inside an inlined function.  If we can,
      use the call site of the function instead.  */
-  b = block_for_pc_sect (sal.pc, SYMBOL_OBJ_SECTION (sym));
+  b = block_for_pc_sect (sal->pc, sal->section);
   function_block = NULL;
   while (b != NULL)
     {
@@ -2390,12 +2443,9 @@ find_function_start_sal (struct symbol *
   if (function_block != NULL
       && SYMBOL_LINE (BLOCK_FUNCTION (function_block)) != 0)
     {
-      sal.line = SYMBOL_LINE (BLOCK_FUNCTION (function_block));
-      sal.symtab = SYMBOL_SYMTAB (BLOCK_FUNCTION (function_block));
+      sal->line = SYMBOL_LINE (BLOCK_FUNCTION (function_block));
+      sal->symtab = SYMBOL_SYMTAB (BLOCK_FUNCTION (function_block));
     }
-
-  do_cleanups (old_chain);
-  return sal;
 }
 
 /* If P is of the form "operator[ \t]+..." where `...' is
Index: gdb/symtab.h
===================================================================
RCS file: /cvs/src/src/gdb/symtab.h,v
retrieving revision 1.148
diff -u -p -r1.148 symtab.h
--- gdb/symtab.h	10 Mar 2010 18:20:07 -0000	1.148
+++ gdb/symtab.h	19 Mar 2010 14:35:28 -0000
@@ -1132,12 +1132,11 @@ extern char *find_main_filename (void);
 
 extern struct symtab *find_line_symtab (struct symtab *, int, int *, int *);
 
-extern CORE_ADDR find_function_start_pc (struct gdbarch *,
-					 CORE_ADDR, struct obj_section *);
-
 extern struct symtab_and_line find_function_start_sal (struct symbol *sym,
 						       int);
 
+extern void skip_prologue_sal (struct symtab_and_line *);
+
 /* symfile.c */
 
 extern void clear_symtab_users (void);
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]