[patch] Fix internal error on some -O2 -g breakpoints

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi,

https://bugzilla.redhat.com/show_bug.cgi?id=612253
(gdb) file ./cc1plus
(gdb) break add_new_name_mapping
../../gdb/breakpoint.c:6594: internal-error: expand_line_sal_maybe: Assertion
`found' failed.

# Test various conditions of prologue analysis on -O2 -g code.  Particularly
# when the first command of a function is an inlined function from a different
# symtab.  The inlined function can have also multiple concrete instances.
# The next first line after the inlined function should have two PC ranges.

expand_line_sal_maybe processes already too complex data it cannot rely on the
exact input PC would be found between the output resolved results.  There are
various bugs I was trying to fix which cause such incorrect multiple results
in this case.  But even after fixing all of them the code would be too fragile
to stay relying on this conditional.

After this patch GDB will put the breakpoint only _after_ the initial inlined
call (=incorrectly) in the testcase; it will no longer crash on it, though.

The symtab.c==expand_line_sal change is optional.  I believe it is a correct
change but I do not find it much important.  With this change GDB will not
only no longer crash but it will even place the breakpoint correctly in the
real world cc1plus testcase from the downstream bug above.

Defining -DLEXICAL still crashes FSF GDB HEAD but just with the
symtab.c==expand_line_sal change it no longer crashes.

Defining -DINLINED crashes FSF GDB HEAD even with the
symtab.c==expand_line_sal change, therefore also
breakpoint.c==expand_line_sal_maybe is patches in this patches.

I can remove the LEXICAL part and keep there only the un-#ifdef-ed INLINED
one if anyone cares about.

<background>
There remain more issues to solve.  skip_prologue_sal should be called for
each separate inlined instance (line number alias).  It is called now first
and then the other instances are being found... Also skip_prologue_sal should
be able to _decrease_ the PC, not just increase it as it may be needed from
breakpoint_re_set.  Just reread_symbols contains bugs causing common GDB
crashes on the reload so I did not try to fix skip_prologue_sal for it now.

Also there is a general problem that GDB now tries to heuristically guess some
expected defects of the debuginfo in -O2 -g cases (see "For optimized code" in
expand_line_sal), it is there because GCC does not generate proper is_stmt
(DW_LNS_negate_stmt).  Moreover very magic skip_prologue_sal is there just
because GCC does not generate proper DW_LNS_set_prologue_end markers.

As GCC should produce these markers in a foreseeable future I did not want to
try to do anything with these GDB heuristics.

There remains a question whether the expand_line_sal_maybe patch part should
not possibly use just the first breakpoint if ORIGINAL_PC cannot be found.
I find it safer this way without much disadvantages, multi-PC breakpoints are
very transparent to the user, the other breakpoints are always somehow related
and more breakpoints is always better than less of them, IMO.
</background>


No regressions on {x86_64,x86_64-m32,i686}-fedora13-linux-gnu.

The testcase may not necessarily crash current FSF GDB HEAD on other arches.


Thanks,
Jan


gdb/
2010-07-29  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* breakpoint.c (expand_line_sal_maybe): Remove variable found.  Remove
	the verification ORIGINAL_PC can be found.
	* symtab.c (expand_line_sal): New variable bl.  Compare functions, not
	blocks.

gdb/testsuite/
2010-07-29  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.dwarf2/dw2-skip-prologue.exp: New file.
	* gdb.dwarf2/dw2-skip-prologue.c: New file.
	* gdb.dwarf2/dw2-skip-prologue.S: New file.
	* lib/gdb.exp (gdb_breakpoint): New case for a GDB internal error.

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7048,7 +7048,6 @@ expand_line_sal_maybe (struct symtab_and_line sal)
   struct symtabs_and_lines expanded;
   CORE_ADDR original_pc = sal.pc;
   char *original_function = NULL;
-  int found;
   int i;
   struct cleanup *old_chain;
 
@@ -7131,17 +7130,8 @@ expand_line_sal_maybe (struct symtab_and_line sal)
       return expanded;      
     }
 
-  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);
-    }
+  /* ORIGINAL_PC may not be found between EXPANDED.SALS.  expand_line_sal may
+     have skipped too far.  */
 
   return expanded;
 }
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4636,11 +4636,20 @@ expand_line_sal (struct symtab_and_line sal)
   blocks = alloca (ret.nelts * sizeof (struct block *));
   for (i = 0; i < ret.nelts; ++i)
     {
+      struct block *bl;
+
       set_current_program_space (ret.sals[i].pspace);
 
-      filter[i] = 1;
-      blocks[i] = block_for_pc_sect (ret.sals[i].pc, ret.sals[i].section);
+      /* Place breakpoint only to the first PC in a function, even if some of
+	 them are in a lexical sub-block.  Put it too all the function
+	 instances incl. the inlined ones.  */
 
+      bl = block_for_pc_sect (ret.sals[i].pc, ret.sals[i].section);
+      while (bl != NULL && BLOCK_FUNCTION (bl) == NULL)
+	bl = BLOCK_SUPERBLOCK (bl);
+
+      filter[i] = 1;
+      blocks[i] = bl;
     }
   do_cleanups (old_chain);
 
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-skip-prologue.S
@@ -0,0 +1,288 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+	.section .debug_info
+.Lcu1_begin:
+	/* CU header */
+	.4byte	.Lcu1_end - .Lcu1_start		/* Length of Compilation Unit */
+.Lcu1_start:
+	.2byte	2				/* DWARF Version */
+	.4byte	.Labbrev1_begin			/* Offset into abbrev section */
+	.byte	4				/* Pointer size */
+
+	/* CU die */
+	.uleb128 1				/* Abbrev: DW_TAG_compile_unit */
+	.4byte	.Lline1_begin			/* DW_AT_stmt_list */
+	.4byte	main_start			/* DW_AT_low_pc */
+	.4byte	main_end			/* DW_AT_high_pc */
+	.ascii	"main.c\0"			/* DW_AT_name */
+	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
+	.byte	2				/* DW_AT_language (DW_LANG_C) */
+
+	.uleb128	2			/* Abbrev: DW_TAG_subprogram */
+	.byte		1			/* DW_AT_external */
+	.ascii		"main\0"		/* DW_AT_name */
+	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
+	.4byte		main_start		/* DW_AT_low_pc */
+	.4byte		main_end		/* DW_AT_high_pc */
+
+	.uleb128	4			/* Abbrev: DW_TAG_inlined_subroutine */
+	.ascii		"inlined\0"		/* DW_AT_name */
+	.4byte		main0			/* DW_AT_low_pc */
+	.4byte		main1			/* DW_AT_high_pc */
+	.byte		3			/* DW_AT_inline (DW_INL_declared_inlined) */
+	.byte		1			/* DW_AT_call_file */
+	.byte		8			/* DW_AT_call_line */
+
+	.uleb128	4			/* Abbrev: DW_TAG_inlined_subroutine */
+	.ascii		"inlined2\0"		/* DW_AT_name */
+	.4byte		main2			/* DW_AT_low_pc */
+	.4byte		main3			/* DW_AT_high_pc */
+	.byte		3			/* DW_AT_inline (DW_INL_declared_inlined) */
+	.byte		1			/* DW_AT_call_file */
+	.byte		11			/* DW_AT_call_line */
+
+#ifdef INLINED
+	.uleb128	4			/* Abbrev: DW_TAG_inlined_subroutine */
+	.ascii		"otherinline\0"		/* DW_AT_name */
+	.4byte		main3			/* DW_AT_low_pc */
+	.4byte		main_end		/* DW_AT_high_pc */
+	.byte		3			/* DW_AT_inline (DW_INL_declared_inlined) */
+	.byte		1			/* DW_AT_call_file */
+	.byte		9			/* DW_AT_call_line */
+#endif
+
+#ifdef LEXICAL
+	.uleb128	5			/* Abbrev: DW_TAG_lexical_block */
+	.4byte		main3			/* DW_AT_low_pc */
+	.4byte		main_end		/* DW_AT_high_pc */
+
+	/* GDB would otherwise ignore the DW_TAG_lexical_block.  */
+	.uleb128	6			/* Abbrev: DW_TAG_variable */
+	.ascii		"lexicalvar\0"		/* DW_AT_name */
+	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
+
+	.byte		0			/* End of children of DW_TAG_lexical_block */
+#endif
+
+	.byte		0			/* End of children of DW_TAG_subprogram */
+
+.Ltype_int:
+	.uleb128	3			/* Abbrev: DW_TAG_base_type */
+	.ascii		"int\0"			/* DW_AT_name */
+	.byte		4			/* DW_AT_byte_size */
+	.byte		5			/* DW_AT_encoding */
+
+	.byte		0			/* End of children of CU */
+
+.Lcu1_end:
+
+/* Abbrev table */
+	.section .debug_abbrev
+.Labbrev1_begin:
+	.uleb128	1			/* Abbrev code */
+	.uleb128	0x11			/* DW_TAG_compile_unit */
+	.byte		1			/* has_children */
+	.uleb128	0x10			/* DW_AT_stmt_list */
+	.uleb128	0x6			/* DW_FORM_data4 */
+	.uleb128	0x11			/* DW_AT_low_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x25			/* DW_AT_producer */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x13			/* DW_AT_language */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	2			/* Abbrev code */
+	.uleb128	0x2e			/* DW_TAG_subprogram */
+	.byte		1			/* has_children */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x49			/* DW_AT_type */
+	.uleb128	0x13			/* DW_FORM_ref4 */
+	.uleb128	0x11			/* DW_AT_low_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	3			/* Abbrev code */
+	.uleb128	0x24			/* DW_TAG_base_type */
+	.byte		0			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0xb			/* DW_AT_byte_size */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x3e			/* DW_AT_encoding */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	4			/* Abbrev code */
+	.uleb128	0x1d			/* DW_TAG_inlined_subroutine */
+	.byte		0			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x11			/* DW_AT_low_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x20			/* DW_AT_inline */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x58			/* DW_AT_call_file */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x59			/* DW_AT_call_line */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	5			/* Abbrev code */
+	.uleb128	0x0b			/* DW_TAG_lexical_block */
+	.byte		1			/* has_children */
+	.uleb128	0x11			/* DW_AT_low_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	6			/* Abbrev code */
+	.uleb128	0x34			/* DW_TAG_variable */
+	.byte		0			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x49			/* DW_AT_type */
+	.uleb128	0x13			/* DW_FORM_ref4 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+/* Line table */
+	.section .debug_line
+.Lline1_begin:
+	.4byte		.Lline1_end - .Lline1_start	/* Initial length */
+.Lline1_start:
+	.2byte		2			/* Version */
+	.4byte		.Lline1_lines - .Lline1_hdr	/* header_length */
+.Lline1_hdr:
+	.byte		1			/* Minimum insn length */
+	.byte		1			/* default_is_stmt */
+	.byte		1			/* line_base */
+	.byte		1			/* line_range */
+	.byte		0x10			/* opcode_base */
+
+	/* Standard lengths */
+	.byte		0
+	.byte		1
+	.byte		1
+	.byte		1
+	.byte		1
+	.byte		0
+	.byte		0
+	.byte		0
+	.byte		1
+	.byte		0
+	.byte		0
+	.byte		1
+	.byte		0
+	.byte		0
+	.byte		0
+
+	/* Include directories */
+	.byte		0
+
+	/* File names */
+	.ascii		"main.c\0"
+	.uleb128	0
+	.uleb128	0
+	.uleb128	0
+
+	.ascii		"other.c\0"
+	.uleb128	0
+	.uleb128	0
+	.uleb128	0
+
+	.byte		0
+
+.Lline1_lines:
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		main_start
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	4	/* ... to 5 */
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		main0
+	.byte		4	/* DW_LNS_set_file */
+	.uleb128	2
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	-4	/* ... to 1 */
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		main1
+	.byte		4	/* DW_LNS_set_file */
+	.uleb128	1
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	8	/* ... to 9 */
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		main2
+	.byte		4	/* DW_LNS_set_file */
+	.uleb128	2
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	-8	/* ... to 1 */
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		main3
+	.byte		4	/* DW_LNS_set_file */
+	.uleb128	1
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	8	/* ... to 9 */
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		main_end
+	.byte		0	/* DW_LNE_end_of_sequence */
+	.uleb128	1
+	.byte		1
+
+.Lline1_end:
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-skip-prologue.c
@@ -0,0 +1,34 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+static volatile int v;
+
+asm ("main_start: .globl main_start\n");
+int
+main (void)
+{
+  v++;
+asm ("main0: .globl main0\n");
+  v++;
+asm ("main1: .globl main1\n");
+  v++;
+asm ("main2: .globl main2\n");
+  v++;
+asm ("main3: .globl main3\n");
+  return v;
+}
+asm ("main_end: .globl main_end\n");
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-skip-prologue.exp
@@ -0,0 +1,55 @@
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test various conditions of prologue analysis on -O2 -g code.  Particularly
+# when the first command of a function is an inlined function from a different
+# symtab.  The inlined function can have also multiple concrete instances.
+# The next first line after the inlined function should have two PC ranges.
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+# For now pick a sampling of likely targets.
+if {![istarget *-*-linux*]
+    && ![istarget *-*-gnu*]
+    && ![istarget *-*-elf*]
+    && ![istarget *-*-openbsd*]
+    && ![istarget arm-*-eabi*]
+    && ![istarget powerpc-*-eabi*]} {
+    return 0  
+}
+
+set testfile "dw2-skip-prologue"
+set executable ${testfile}
+set binfile ${objdir}/${subdir}/${executable}
+
+if {[build_executable ${testfile}.exp ${executable} "${testfile}.c ${testfile}.S" {additional_flags=-DINLINED}] == -1} {
+    return -1
+}
+
+# We need those symbols global to access them from the .S file.
+set test "strip stub symbols"
+set objcopy_program [transform objcopy]
+set result [catch "exec $objcopy_program -N main0 -N main1 -N main2 -N main3 ${binfile}" output]
+verbose "result is $result"
+verbose "output is $output"
+if {$result != 0} {
+    fail $test
+    return
+}
+pass $test
+
+clean_restart $executable
+
+gdb_breakpoint "main"
+pass "breakpoint put"
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -375,6 +375,11 @@ proc gdb_breakpoint { function args } {
 		send_gdb "$pending_response\n"
 		exp_continue
 	}
+	-re ".*A problem internal to GDB has been detected" {
+		fail "setting breakpoint at $function in runto (GDB internal error)"
+		gdb_internal_error_resync
+		return 0
+	}
 	-re "$gdb_prompt $" {
 		if { $no_message == 0 } {
 			fail "setting breakpoint at $function"