gdb-expand-line-sal-maybe.patch 5.15 KB
Newer Older
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138

This is fixing http://bugreports.qt.nokia.com/browse/QTCREATORBUG-2004

-----------------------------------------------------------------------------    

* From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
    * To: gdb-patches at sourceware dot org
    * Date: Thu, 29 Jul 2010 00:39:54 +0200
    * Subject: [patch] Fix internal error on some -O2 -g breakpoints

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
-----------------------------------------------------------------------------    



--- 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);