| 
									
										
										
										
											2010-10-12 15:11:17 +02:00
										 |  |  | 
 | 
					
						
							| 
									
										
										
										
											2011-08-09 17:32:46 +02:00
										 |  |  | This is fixing https://bugreports.qt.nokia.com/browse/QTCREATORBUG-2004 | 
					
						
							| 
									
										
										
										
											2010-10-12 15:11:17 +02:00
										 |  |  | 
 | 
					
						
							|  |  |  | -----------------------------------------------------------------------------    
 | 
					
						
							|  |  |  | 
 | 
					
						
							|  |  |  | * 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
 | 
					
						
							| 
									
										
										
										
											2011-06-23 12:36:25 +02:00
										 |  |  | @@ -7532,7 +7532,6 @@ expand_line_sal_maybe (struct symtab_and_line sal)
 | 
					
						
							| 
									
										
										
										
											2010-10-12 15:11:17 +02:00
										 |  |  |    struct symtabs_and_lines expanded; | 
					
						
							|  |  |  |    CORE_ADDR original_pc = sal.pc; | 
					
						
							|  |  |  |    char *original_function = NULL; | 
					
						
							|  |  |  | -  int found;
 | 
					
						
							|  |  |  |    int i; | 
					
						
							|  |  |  |    struct cleanup *old_chain; | 
					
						
							|  |  |  |   | 
					
						
							| 
									
										
										
										
											2011-06-23 12:36:25 +02:00
										 |  |  | @@ -7616,17 +7615,8 @@ expand_line_sal_maybe (struct symtab_and_line sal)
 | 
					
						
							| 
									
										
										
										
											2010-10-12 15:11:17 +02:00
										 |  |  |        return expanded;       | 
					
						
							|  |  |  |      } | 
					
						
							|  |  |  |   | 
					
						
							|  |  |  | -  if (original_pc)
 | 
					
						
							|  |  |  | -    {
 | 
					
						
							|  |  |  | -      found = 0;
 | 
					
						
							|  |  |  | -      for (i = 0; i < expanded.nelts; ++i)
 | 
					
						
							| 
									
										
										
										
											2011-06-23 12:36:25 +02:00
										 |  |  | -	if (expanded.sals[i].pc == original_pc)
 | 
					
						
							|  |  |  | -	  {
 | 
					
						
							|  |  |  | -	    found = 1;
 | 
					
						
							|  |  |  | -	    break;
 | 
					
						
							|  |  |  | -	  }
 | 
					
						
							| 
									
										
										
										
											2010-10-12 15:11:17 +02:00
										 |  |  | -      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
 | 
					
						
							| 
									
										
										
										
											2011-06-23 12:36:25 +02:00
										 |  |  | @@ -4750,11 +4750,20 @@ expand_line_sal (struct symtab_and_line sal)
 | 
					
						
							| 
									
										
										
										
											2010-10-12 15:11:17 +02:00
										 |  |  |    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); | 
					
						
							|  |  |  |   |