Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [9/9] expand locations
@ 2007-09-07 23:14 Vladimir Prus
  2007-09-08  0:21 ` Pedro Alves
  2007-09-08 11:50 ` Eli Zaretskii
  0 siblings, 2 replies; 7+ messages in thread
From: Vladimir Prus @ 2007-09-07 23:14 UTC (permalink / raw)
  To: gdb-patches

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


This patch makes gdb actually create breakpoints with
multiple locations. First, we use decode_line_1 to get
sal. After that, we try to find all PC values that correspond
to the same file:line as the original sal. Those new sals
are used for locations.

Some care is taken not to generate extra sals when not necessary:

	- When breakpoint is requested at address, we don't
	expand at all.
	- We never generate two sals with PC values in the same
	block, so that if scheduler scattered the line, we don't
	set breakpoints on all pieces of a line.

This patch includes testcases for constructors and templates. OK?

- Volodya


[-- Attachment #2: mainline_9_expansion.ChangeLog --]
[-- Type: text/plain, Size: 694 bytes --]

	gdb/
	* breakpoint.c (remove_sal): New.
	(expand_line_sal_maybe): New.
	(create_breakpoints): Call expand_line_sal_maybe.
	(clear_command): Add comment.
	(breakpoint_re_set_one): Call expand_line_sal_maybe.
	* linespec.c (decode_indirect): Set explicit_pc to 1.
	(decode_all_digits): Set explicit_line to 1.
	(append_expanded_sal): New.
	(expand_line_sal): New.
	* linespec.h (expand_line_sal): Declare.
	* symtab.c (init_sal): Initialize explicit_pc 
	and explicit_line.
	* symtab.h (struct symtab_and_line): New fields
	explicit_pc and explicit_line.	
	
	gdb/testsuite/
	* gdb.cp/mb-ctor.cc: New.
	* gdb.cp/mb-ctor.exp: New.
	* gdb.cp/mb-templates.cc: New.
	* gdb.cp/mb-templates.exp: New.


[-- Attachment #3: mainline_9_expansion.diff --]
[-- Type: text/x-diff, Size: 21401 bytes --]

--- gdb/breakpoint.c	(/work/mb_mainline/8_multiple_locations)	(revision 4755)
+++ gdb/breakpoint.c	(/work/mb_mainline/9_expansion)	(revision 4755)
@@ -5195,6 +5195,128 @@ create_breakpoint (struct symtabs_and_li
   mention (b);
 }
 
+/* Remove element at INDEX_TO_REMOVE from SAL, shifting other
+   elements to fill the void space.  */
+static void remove_sal (struct symtabs_and_lines *sal, int index_to_remove)
+{
+  int i = index_to_remove+1;
+  int last_index = sal->nelts-1;
+
+  for (;i <= last_index; ++i)
+    sal->sals[i-1] = sal->sals[i];
+
+  --(sal->nelts);
+}
+
+/* If appropriate, obtains all sals that correspond
+   to the same file and line as SAL.  This is done
+   only if SAL does not have explicit PC and has
+   line and file information.  If we got just a single
+   expanded sal, return the original.
+
+   Otherwise, if SAL.explicit_line is not set, filter out 
+   all sals for which the name of enclosing function 
+   is different from SAL. This makes sure that if we have
+   breakpoint originally set in template instantiation, say
+   foo<int>(), we won't expand SAL to locations at the same
+   line in all existing instantiations of 'foo'.
+
+*/
+struct symtabs_and_lines
+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;
+
+  /* If we have explicit pc, don't expand.
+     If we have no line number, we can't expand.  */
+  if (sal.explicit_pc || sal.line == 0 || sal.symtab == NULL)
+    {
+      expanded.nelts = 1;
+      expanded.sals = xmalloc (sizeof (struct symtab_and_line));
+      expanded.sals[0] = sal;
+      return expanded;
+    }
+
+  sal.pc = 0;
+  find_pc_partial_function (original_pc, &original_function, NULL, NULL);
+  
+  expanded = expand_line_sal (sal);
+  if (expanded.nelts == 1)
+    {
+      /* We had one sal, we got one sal.  Without futher
+	 processing, just return the original sal.  */
+      xfree (expanded.sals);
+      expanded.nelts = 1;
+      expanded.sals = xmalloc (sizeof (struct symtab_and_line));
+      sal.pc = original_pc;
+      expanded.sals[0] = sal;
+      return expanded;      
+    }
+
+  if (!sal.explicit_line)
+    {
+      CORE_ADDR func_addr, func_end;
+      for (i = 0; i < expanded.nelts; ++i)
+	{
+	  CORE_ADDR pc = expanded.sals[i].pc;
+	  char *this_function;
+	  if (find_pc_partial_function (pc, &this_function, 
+					&func_addr, &func_end))
+	    {
+	      if (this_function && 
+		  strcmp (this_function, original_function) != 0)
+		{
+		  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
+		    expanded.sals[i].pc 
+		      = gdbarch_skip_prologue (current_gdbarch, pc);
+		}
+	    }
+	}
+    }
+
+  
+  if (expanded.nelts <= 1)
+    {
+      /* This is un ugly workaround. If we get zero
+       expanded sals then something is really wrong.
+      Fix that by returnign the original sal. */
+      xfree (expanded.sals);
+      expanded.nelts = 1;
+      expanded.sals = xmalloc (sizeof (struct symtab_and_line));
+      sal.pc = original_pc;
+      expanded.sals[0] = 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);
+    }
+
+  return expanded;
+}
+
 /* Add SALS.nelts breakpoints to the breakpoint table.  For each
    SALS.sal[i] breakpoint, include the corresponding ADDR_STRING[i]
    value.  COND_STRING, if not NULL, specified the condition to be
@@ -5225,11 +5347,10 @@ create_breakpoints (struct symtabs_and_l
   int i;
   for (i = 0; i < sals.nelts; ++i)
     {
-      struct symtabs_and_lines sals2;
-      sals2.sals = sals.sals + i;
-      sals2.nelts = 1;
+      struct symtabs_and_lines expanded = 
+	expand_line_sal_maybe (sals.sals[i]);
 
-      create_breakpoint (sals2, addr_string[i],
+      create_breakpoint (expanded, addr_string[i],
 			 cond_string, type, disposition,
 			 thread, ignore_count, from_tty,
 			 pending_bp);
@@ -6900,6 +7021,18 @@ clear_command (char *arg, int from_tty)
       default_match = 1;
     }
 
+  /* We don't call resolve_sal_pc here. That's not
+     as bad as it seems, because all existing breakpoints
+     typically have both file/line and pc set.  So, if
+     clear is given file/line, we can match this to existing
+     breakpoint without obtaining pc at all.
+
+     One case I'm not sure about is where originally we've
+     set breakpoint at file:line. There were several PC values
+     for that file:line, due to optimization, all in one block.
+     We've picked on PC value. If "clear" is issued with another
+     PC corresponding to the same file:line, what should we do?  */
+
   /* For each line spec given, delete bps which correspond
      to it.  Do it in two passes, solely to preserve the current
      behavior that from_tty is forced true if we delete more than
@@ -7415,8 +7548,12 @@ update_breakpoint_locations (struct brea
       }
   }
 
-  if (existing_locations)
-    free_bp_location (existing_locations);
+  while (existing_locations)
+    {
+      struct bp_location *next = existing_locations->next;
+      free_bp_location (existing_locations);
+      existing_locations = next;
+    }
 }
 
 
@@ -7434,6 +7571,7 @@ breakpoint_re_set_one (void *bint)
   int not_found = 0;
   int *not_found_ptr = &not_found;
   struct symtabs_and_lines sals = {};
+  struct symtabs_and_lines expanded;
   char *s;
   enum enable_state save_enable;
   struct gdb_exception e;
@@ -7508,8 +7646,8 @@ breakpoint_re_set_one (void *bint)
 	  b->thread = thread;
 	  b->condition_not_parsed = 0;
 	}
-
-      update_breakpoint_locations (b, sals);
+      expanded = expand_line_sal_maybe (sals.sals[0]);
+      update_breakpoint_locations (b, expanded);
 
       /* Now that this is re-enabled, check_duplicates
 	 can be used. */
--- gdb/testsuite/gdb.cp/mb-ctor.cc	(/work/mb_mainline/8_multiple_locations)	(revision 4755)
+++ gdb/testsuite/gdb.cp/mb-ctor.cc	(/work/mb_mainline/9_expansion)	(revision 4755)
@@ -0,0 +1,58 @@
+
+#include <stdio.h>
+
+class Base 
+{
+public:
+  Base(int k);
+  ~Base();
+  virtual void foo() {}
+private:
+  int k;
+};
+
+Base::Base(int k)
+{
+  this->k = k;
+}
+
+Base::~Base()
+{
+    printf("~Base\n");
+}
+
+class Derived : public virtual Base
+{
+public:
+  Derived(int i);
+  ~Derived();
+private:
+  int i;
+};
+
+Derived::Derived(int i) : Base(i)
+{
+  this->i = i;
+}
+
+Derived::~Derived()
+{
+    printf("~Derived\n");
+}
+
+class DeeplyDerived : public Derived
+{
+public:
+  DeeplyDerived(int i) : Base(i), Derived(i) {}
+};
+
+int main()
+{
+  /* Invokes the Derived ctor that constructs both
+     Derived and Base.  */
+  Derived d(7);
+  /* Invokes the Derived ctor that constructs only
+     Derived. Base is constructed separately by
+     DeeplyDerived's ctor.  */
+  DeeplyDerived dd(15);
+}
--- gdb/testsuite/gdb.cp/mb-ctor.exp	(/work/mb_mainline/8_multiple_locations)	(revision 4755)
+++ gdb/testsuite/gdb.cp/mb-ctor.exp	(/work/mb_mainline/9_expansion)	(revision 4755)
@@ -0,0 +1,93 @@
+#   Copyright 1988, 1990, 1991, 1992, 1994, 1995, 1996, 1997, 1998, 1999,
+#   2000, 2002, 2003, 2007 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 2 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, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@prep.ai.mit.edu
+
+# This file was written by Rob Savoye. (rob@cygnus.com)
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+
+#
+# test running programs
+#
+set prms_id 0
+set bug_id 0
+
+set testfile "mb-ctor"
+set srcfile ${testfile}.cc
+set binfile ${objdir}/${subdir}/${testfile}
+
+if [get_compiler_info ${binfile} "c++"] {
+    return -1
+}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++}] != "" } {
+     untested mb-ctor.exp
+     return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+# Set a breakpoint with multiple locations
+# and a condition.
+
+gdb_test "break 'Derived::Derived(int)'" \
+    "Breakpoint.*at.* file .*$srcfile, line.*\\(2 locations\\).*" \
+    "set-breakpoint at ctor"
+
+gdb_test "break 'Derived::~Derived()'" \
+    "Breakpoint.*at.* file .*$srcfile, line.*\\(2 locations\\).*" \
+    "set-breakpoint at ctor"
+
+gdb_run_cmd
+gdb_expect {
+    -re "Breakpoint \[0-9\]+,.*Derived.*i=7.*$gdb_prompt $" {
+	pass "run to breakpoint"
+    }
+    -re "$gdb_prompt $" {
+	fail "run to breakpoint"
+    }
+    timeout {
+	fail "run to breakpoint (timeout)"
+    }
+}
+
+gdb_test "continue" \
+    ".*Breakpoint.*Derived.*i=15.*" \
+    "run to breakpoint 2"
+
+gdb_test "continue" \
+    ".*Breakpoint.*~Derived.*" \
+    "run to breakpoint 3"
+
+gdb_test "continue" \
+    ".*Breakpoint.*~Derived.*" \
+    "run to breakpoint 4"
+
+gdb_test "continue" \
+    ".*exited normally.*" \
+    "run to exit"
+
+
+
--- gdb/testsuite/gdb.cp/mb-templates.cc	(/work/mb_mainline/8_multiple_locations)	(revision 4755)
+++ gdb/testsuite/gdb.cp/mb-templates.cc	(/work/mb_mainline/9_expansion)	(revision 4755)
@@ -0,0 +1,19 @@
+
+#include <iostream>
+using namespace std;
+
+template<class T>
+void foo(T i)
+{
+  std::cout << "hi\n"; // set breakpoint here
+}
+
+int main()
+{
+    foo<int>(0);
+    foo<double>(0);
+    foo<int>(1);
+    foo<double>(1);
+    foo<int>(2);
+    foo<double>(2);
+}
--- gdb/testsuite/gdb.cp/mb-templates.exp	(/work/mb_mainline/8_multiple_locations)	(revision 4755)
+++ gdb/testsuite/gdb.cp/mb-templates.exp	(/work/mb_mainline/9_expansion)	(revision 4755)
@@ -0,0 +1,168 @@
+#   Copyright 1988, 1990, 1991, 1992, 1994, 1995, 1996, 1997, 1998, 1999,
+#   2000, 2002, 2003, 2007 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 2 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, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@prep.ai.mit.edu
+
+# This file was written by Rob Savoye. (rob@cygnus.com)
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+
+#
+# test running programs
+#
+set prms_id 0
+set bug_id 0
+
+set testfile "mb-templates"
+set srcfile ${testfile}.cc
+set binfile ${objdir}/${subdir}/${testfile}
+
+if [get_compiler_info ${binfile} "c++"] {
+    return -1
+}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++}] != "" } {
+     untested mb-templates.exp
+     return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+set bp_location [gdb_get_line_number "set breakpoint here"]
+
+# Set a breakpoint with multiple locations
+# and a condition.
+
+gdb_test "break $srcfile:$bp_location if i==1" \
+    "Breakpoint.*at.* file .*$srcfile, line.*\\(2 locations\\).*" \
+    "initial condition: set breakpoint"
+
+gdb_run_cmd
+gdb_expect {
+    -re "Breakpoint \[0-9\]+,.*foo<int> \\(i=1\\).*$gdb_prompt $" {
+	pass "initial condition: run to breakpoint"
+    }
+    -re "$gdb_prompt $" {
+	fail "initial condition: run to breakpoint"
+    }
+    timeout {
+	fail "initial condition: run to breakpoint (timeout)"
+    }
+}
+
+gdb_test "continue" \
+    ".*Breakpoint.*foo<double> \\(i=1\\).*" \
+    "initial condition: run to breakpoint 2"
+
+# Set breakpoint with multiple locations.
+# Separately set the condition.
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+gdb_test "break $srcfile:$bp_location" \
+    "Breakpoint.*at.* file .*$srcfile, line.*\\(2 locations\\).*" \
+    "separate condition: set breakpoint"
+
+gdb_test "condition 1 i==1" "" \
+    "separate condition: set condition"
+    
+gdb_run_cmd
+gdb_expect {
+    -re "Breakpoint \[0-9\]+,.*foo<int> \\(i=1\\).*$gdb_prompt $" {
+	pass "separate condition: run to breakpoint"
+    }
+    -re "$gdb_prompt $" {
+	fail "separate condition: run to breakpoint"
+    }
+    timeout {
+	fail "separate condition: run to breakpoint (timeout)"
+    }
+}
+
+gdb_test "continue" \
+    ".*Breakpoint.*foo<double> \\(i=1\\).*" \
+    "separate condition: run to breakpoint 2"
+
+# Try disabling a single location. We also test
+# that at least in simple cases, the enable/disable
+# state of locations surive "run".
+gdb_test "disable 1.1" "" "disabling location: disable"
+
+gdb_run_cmd
+gdb_expect {
+    -re "Breakpoint \[0-9\]+,.*foo<double> \\(i=1\\).*$gdb_prompt $" {
+	pass "disabling location: run to breakpoint"
+    }
+    -re "$gdb_prompt $" {
+	fail "disabling location: run to breakpoint"
+    }
+    timeout {
+	fail "disabling location: run to breakpoint (timeout)"
+    }
+}
+
+# Try disabling entire breakpoint
+gdb_test "enable 1.1" "" "disabling location: enable"
+
+
+gdb_test "disable 1" "" "disable breakpoint: disable"
+
+gdb_run_cmd
+gdb_expect {
+    -re "Program exited normally.*$gdb_prompt $" {
+	pass "disable breakpoint: run to breakpoint"
+    }
+    -re "$gdb_prompt $" {
+	fail "disable breakpoint: run to breakpoint"
+    }
+    timeout {
+	fail "disable breakpoint: run to breakpoint (timeout)"
+    }
+}
+
+# Make sure breakpoint can be set on a specific instantion.
+delete_breakpoints
+gdb_test "break 'void foo<int>(int)'" ".*" \
+    "instantiation: set breakpoint"
+
+
+gdb_run_cmd
+gdb_expect {
+    -re ".*Breakpoint \[0-9\]+,.*foo<int> \\(i=0\\).*$gdb_prompt $" {
+	pass "instantiation: run to breakpoint"
+    }
+    -re "$gdb_prompt $" {
+	fail "instantiation: run to breakpoint"
+    }
+    timeout {
+	fail "instantiation: run to breakpoint (timeout)"
+    }
+}
+
+gdb_test "continue" \
+    ".*Breakpoint.*foo<int> \\(i=1\\).*" \
+    "instantiation: run to breakpoint 2"
+
--- gdb/linespec.c	(/work/mb_mainline/8_multiple_locations)	(revision 4755)
+++ gdb/linespec.c	(/work/mb_mainline/9_expansion)	(revision 4755)
@@ -963,6 +963,7 @@ decode_indirect (char **argptr)
   values.sals[0] = find_pc_line (pc, 0);
   values.sals[0].pc = pc;
   values.sals[0].section = find_pc_overlay (pc);
+  values.sals[0].explicit_pc = 1;
 
   return values;
 }
@@ -1633,6 +1634,7 @@ decode_all_digits (char **argptr, struct
   values.nelts = 1;
   if (need_canonical)
     build_canonical_line_spec (values.sals, NULL, canonical);
+  values.sals[0].explicit_line = 1;
   return values;
 }
 
--- gdb/symtab.c	(/work/mb_mainline/8_multiple_locations)	(revision 4755)
+++ gdb/symtab.c	(/work/mb_mainline/9_expansion)	(revision 4755)
@@ -691,6 +691,8 @@ init_sal (struct symtab_and_line *sal)
   sal->line = 0;
   sal->pc = 0;
   sal->end = 0;
+  sal->explicit_pc = 0;
+  sal->explicit_line = 0;
 }
 \f
 
@@ -4172,6 +4174,166 @@ symtab_observer_executable_changed (void
   set_main_name (NULL);
 }
 
+/* Helper to expand_line_sal below.  Appends new sal to SAL,
+   initializing it from SYMTAB, LINENO and PC.  */
+static void
+append_expanded_sal (struct symtabs_and_lines *sal,
+		     struct symtab *symtab,
+		     int lineno, CORE_ADDR pc)
+{
+  CORE_ADDR func_addr, func_end;
+  
+  sal->sals = xrealloc (sal->sals, 
+			sizeof (sal->sals[0]) 
+			* (sal->nelts + 1));
+  init_sal (sal->sals + sal->nelts);
+  sal->sals[sal->nelts].symtab = symtab;
+  sal->sals[sal->nelts].section = NULL;
+  sal->sals[sal->nelts].end = 0;
+  sal->sals[sal->nelts].line = lineno;  
+  sal->sals[sal->nelts].pc = pc;
+  ++sal->nelts;      
+}
+
+/* Compute a set of all sals in
+   the entire program that correspond to same file
+   and line as SAL and return those.  If there
+   are several sals that belong to the same block,
+   only one sal for the block is included in results.  */
+   
+struct symtabs_and_lines
+expand_line_sal (struct symtab_and_line sal)
+{
+  struct symtabs_and_lines ret, this_line;
+  int i, j;
+  struct objfile *objfile;
+  struct partial_symtab *psymtab;
+  struct symtab *symtab;
+  int lineno;
+  int deleted = 0;
+  struct block **blocks = NULL;
+  int *filter;
+
+  ret.nelts = 0;
+  ret.sals = NULL;
+
+  if (sal.symtab == NULL || sal.line == 0 || sal.pc != 0)
+    {
+      ret.sals = xmalloc (sizeof (struct symtab_and_line));
+      ret.sals[0] = sal;
+      ret.nelts = 1;
+      return ret;
+    }
+  else
+    {
+      struct linetable_entry *best_item = 0;
+      struct symtab *best_symtab = 0;
+      int exact = 0;
+
+      lineno = sal.line;
+
+      /* We meed to find all symtabs for a file which name
+	 is described by sal. We cannot just directly 
+	 iterate over symtabs, since a symtab might not be
+	 yet created. We also cannot iterate over psymtabs,
+	 calling PSYMTAB_TO_SYMTAB and working on that symtab,
+	 since PSYMTAB_TO_SYMTAB will return NULL for psymtab
+	 corresponding to an included file. Therefore, we do
+	 first pass over psymtabs, reading in those with
+	 the right name.  Then, we iterate over symtabs, knowing
+	 that all symtabs we're interested in are loaded.  */
+
+      ALL_PSYMTABS (objfile, psymtab)
+	{
+	  if (strcmp (sal.symtab->filename,
+		      psymtab->filename) == 0)
+	    PSYMTAB_TO_SYMTAB (psymtab);
+	}
+
+	 
+      /* For each symtab, we add all pcs to ret.sals. I'm actually
+	 not sure what to do if we have exact match in one symtab,
+	 and non-exact match on another symtab.
+      */
+      ALL_SYMTABS (objfile, symtab)
+	{
+	  if (strcmp (sal.symtab->filename,
+		      symtab->filename) == 0)
+	    {
+	      struct linetable *l;
+	      int len;
+	      l = LINETABLE (symtab);
+	      if (!l)
+		continue;
+	      len = l->nitems;
+
+	      for (j = 0; j < len; j++)
+		{
+		  struct linetable_entry *item = &(l->item[j]);
+
+		  if (item->line == lineno)
+		    {
+		      exact = 1;
+		      append_expanded_sal (&ret, symtab, lineno, item->pc);
+		    }      
+		  else if (!exact && item->line > lineno
+			   && (best_item == NULL || item->line < best_item->line))
+		  
+		    {
+		      best_item = item;
+		      best_symtab = symtab;
+		    }
+		}
+	    }
+	}
+      if (!exact && best_item)
+	append_expanded_sal (&ret, best_symtab, lineno, best_item->pc);
+    }
+
+  /* For optimized code, compiler can scatter one source line accross
+     disjoint ranges of PC values, even when no duplicate functions
+     or inline functions are involved.  For example, 'for (;;)' inside
+     non-template non-inline non-ctor-or-dtor function can result
+     in two PC ranges.  In this case, we don't want to set breakpoint
+     on first PC of each range.  To filter such cases, we use containing
+     blocks -- for each PC found above we see if there are other PCs
+     that are in the same block.  If yes, the other PCs are filtered out.  */  
+
+  filter = xmalloc (ret.nelts * sizeof (int));
+  blocks = xmalloc (ret.nelts * sizeof (struct block *));
+  for (i = 0; i < ret.nelts; ++i)
+    {
+      filter[i] = 1;
+      blocks[i] = block_for_pc (ret.sals[i].pc);
+    }
+
+  for (i = 0; i < ret.nelts; ++i)
+    if (blocks[i] != NULL)
+      for (j = i+1; j < ret.nelts; ++j)
+	if (blocks[j] == blocks[i])
+	  {
+	    filter[j] = 0;
+	    ++deleted;
+	    break;
+	  }
+  
+  {
+    struct symtab_and_line *final = 
+      xmalloc (sizeof (struct symtab_and_line) * (ret.nelts-deleted));
+    
+    for (i = 0, j = 0; i < ret.nelts; ++i)
+      if (filter[i])
+	final[j++] = ret.sals[i];
+    
+    ret.nelts -= deleted;
+    xfree (ret.sals);
+    ret.sals = final;
+  }
+
+  return ret;
+}
+
+
 void
 _initialize_symtab (void)
 {
--- gdb/symtab.h	(/work/mb_mainline/8_multiple_locations)	(revision 4755)
+++ gdb/symtab.h	(/work/mb_mainline/9_expansion)	(revision 4755)
@@ -1213,6 +1213,8 @@ struct symtab_and_line
 
   CORE_ADDR pc;
   CORE_ADDR end;
+  int explicit_pc;
+  int explicit_line;
 };
 
 extern void init_sal (struct symtab_and_line *sal);
@@ -1404,5 +1406,7 @@ struct symbol *lookup_global_symbol_from
 						  const domain_enum domain,
 						  struct symtab **symtab);
 
+extern struct symtabs_and_lines
+expand_line_sal (struct symtab_and_line sal);
 
 #endif /* !defined(SYMTAB_H) */

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [9/9] expand locations
  2007-09-07 23:14 [9/9] expand locations Vladimir Prus
@ 2007-09-08  0:21 ` Pedro Alves
  2007-09-08  1:41   ` Jim Blandy
  2007-09-08  3:42   ` Daniel Jacobowitz
  2007-09-08 11:50 ` Eli Zaretskii
  1 sibling, 2 replies; 7+ messages in thread
From: Pedro Alves @ 2007-09-08  0:21 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches


Just one boring question from the newbie:

Is the the below the correct thing to do put in the headers of the
new .exp files? Not just the GPL2, but also the dates, and the
credit (, and the bug mailing list)?

I understand that the test structure was probably based on
an existing test as template.


+#   Copyright 1988, 1990, 1991, 1992, 1994, 1995, 1996, 1997, 1998, 1999,
+#   2000, 2002, 2003, 2007 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 2 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, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@prep.ai.mit.edu
+
+# This file was written by Rob Savoye. (rob@cygnus.com)
+

Cheers,
Pedro Alves





^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [9/9] expand locations
  2007-09-08  0:21 ` Pedro Alves
@ 2007-09-08  1:41   ` Jim Blandy
  2007-09-08  3:42   ` Daniel Jacobowitz
  1 sibling, 0 replies; 7+ messages in thread
From: Jim Blandy @ 2007-09-08  1:41 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Vladimir Prus, gdb-patches


Pedro Alves <pedro_alves@portugalmail.pt> writes:
> Just one boring question from the newbie:
>
> Is the the below the correct thing to do put in the headers of the
> new .exp files? Not just the GPL2, but also the dates, and the
> credit (, and the bug mailing list)?

Ah, thanks.  All that does need to be updated to match the rest of
GDB.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [9/9] expand locations
  2007-09-08  0:21 ` Pedro Alves
  2007-09-08  1:41   ` Jim Blandy
@ 2007-09-08  3:42   ` Daniel Jacobowitz
  1 sibling, 0 replies; 7+ messages in thread
From: Daniel Jacobowitz @ 2007-09-08  3:42 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Vladimir Prus, gdb-patches

On Sat, Sep 08, 2007 at 01:20:56AM +0100, Pedro Alves wrote:
> 
> Just one boring question from the newbie:
> 
> Is the the below the correct thing to do put in the headers of the
> new .exp files? Not just the GPL2, but also the dates, and the
> credit (, and the bug mailing list)?
> 
> I understand that the test structure was probably based on
> an existing test as template.

Depending how much of the test was based on another, the dates may be
appropriate.  The contributed-by notice is not - and the mailing list
isn't either; we should really strip all the prep.ai.mit.edu
references one of these days.

There's more per-test boilerplate that can go too, e.g. the tracelevel
stuff and clearing prms_id.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [9/9] expand locations
  2007-09-07 23:14 [9/9] expand locations Vladimir Prus
  2007-09-08  0:21 ` Pedro Alves
@ 2007-09-08 11:50 ` Eli Zaretskii
  2007-09-23  8:22   ` Vladimir Prus
  1 sibling, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2007-09-08 11:50 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Sat, 8 Sep 2007 03:13:33 +0400
> 
> +     One case I'm not sure about is where originally we've
> +     set breakpoint at file:line. There were several PC values
> +     for that file:line, due to optimization, all in one block.
> +     We've picked on PC value. If "clear" is issued with another
> +     PC corresponding to the same file:line, what should we do?  */

I think we should ask the user what she meant.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [9/9] expand locations
  2007-09-08 11:50 ` Eli Zaretskii
@ 2007-09-23  8:22   ` Vladimir Prus
  2007-09-23 19:23     ` Eli Zaretskii
  0 siblings, 1 reply; 7+ messages in thread
From: Vladimir Prus @ 2007-09-23  8:22 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

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

On Saturday 08 September 2007 15:50:24 Eli Zaretskii wrote:
> > From: Vladimir Prus <vladimir@codesourcery.com>
> > Date: Sat, 8 Sep 2007 03:13:33 +0400
> > 
> > +     One case I'm not sure about is where originally we've
> > +     set breakpoint at file:line. There were several PC values
> > +     for that file:line, due to optimization, all in one block.
> > +     We've picked on PC value. If "clear" is issued with another
> > +     PC corresponding to the same file:line, what should we do?  */
> 
> I think we should ask the user what she meant.

I have pondered about this, and I no longer thing we have much choice,
or that we really need to ask the user.

The situation comment talks about is this:

	- User sets breakpoints at some line
	- Due to optimization, there are two PC corresponding to that line
	-- PC1 and PC2
	- With my patches, the first PC is selected.
	- User says 'clear PC2'.

What user just did does not seem a common use case. Typically, user will just use
'clear' (without parameter) after hitting a breakpoint.  And we set breakpoint
on PC1, so no problem. Another possible, though less likely case, is when user
looks at breakpoint table, notes PC, and says "clear PC". But the breakpoint
table will only list PC1. So, the only possibility for user to issue 'clear PC2'
is when user guessed that PC2 is also PC corresponding to original line. This
does not seem very important case to support.

I've clarified the comment to reflect this. 

I have also fixed license/copyright years on the new test files. The
revised patch is attached, OK?

- Volodya


 



[-- Attachment #2: mainline_9_expansion.ChangeLog --]
[-- Type: text/plain, Size: 694 bytes --]

	gdb/
	* breakpoint.c (remove_sal): New.
	(expand_line_sal_maybe): New.
	(create_breakpoints): Call expand_line_sal_maybe.
	(clear_command): Add comment.
	(breakpoint_re_set_one): Call expand_line_sal_maybe.
	* linespec.c (decode_indirect): Set explicit_pc to 1.
	(decode_all_digits): Set explicit_line to 1.
	(append_expanded_sal): New.
	(expand_line_sal): New.
	* linespec.h (expand_line_sal): Declare.
	* symtab.c (init_sal): Initialize explicit_pc 
	and explicit_line.
	* symtab.h (struct symtab_and_line): New fields
	explicit_pc and explicit_line.	
	
	gdb/testsuite/
	* gdb.cp/mb-ctor.cc: New.
	* gdb.cp/mb-ctor.exp: New.
	* gdb.cp/mb-templates.cc: New.
	* gdb.cp/mb-templates.exp: New.


[-- Attachment #3: mainline_9_expansion.diff --]
[-- Type: text/x-diff, Size: 21946 bytes --]

--- gdb/breakpoint.c	(/work/mb_mainline/8_multiple_locations)	(revision 4832)
+++ gdb/breakpoint.c	(/work/mb_mainline/9_expansion)	(revision 4832)
@@ -5184,6 +5184,128 @@ create_breakpoint (struct symtabs_and_li
   mention (b);
 }
 
+/* Remove element at INDEX_TO_REMOVE from SAL, shifting other
+   elements to fill the void space.  */
+static void remove_sal (struct symtabs_and_lines *sal, int index_to_remove)
+{
+  int i = index_to_remove+1;
+  int last_index = sal->nelts-1;
+
+  for (;i <= last_index; ++i)
+    sal->sals[i-1] = sal->sals[i];
+
+  --(sal->nelts);
+}
+
+/* If appropriate, obtains all sals that correspond
+   to the same file and line as SAL.  This is done
+   only if SAL does not have explicit PC and has
+   line and file information.  If we got just a single
+   expanded sal, return the original.
+
+   Otherwise, if SAL.explicit_line is not set, filter out 
+   all sals for which the name of enclosing function 
+   is different from SAL. This makes sure that if we have
+   breakpoint originally set in template instantiation, say
+   foo<int>(), we won't expand SAL to locations at the same
+   line in all existing instantiations of 'foo'.
+
+*/
+struct symtabs_and_lines
+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;
+
+  /* If we have explicit pc, don't expand.
+     If we have no line number, we can't expand.  */
+  if (sal.explicit_pc || sal.line == 0 || sal.symtab == NULL)
+    {
+      expanded.nelts = 1;
+      expanded.sals = xmalloc (sizeof (struct symtab_and_line));
+      expanded.sals[0] = sal;
+      return expanded;
+    }
+
+  sal.pc = 0;
+  find_pc_partial_function (original_pc, &original_function, NULL, NULL);
+  
+  expanded = expand_line_sal (sal);
+  if (expanded.nelts == 1)
+    {
+      /* We had one sal, we got one sal.  Without futher
+	 processing, just return the original sal.  */
+      xfree (expanded.sals);
+      expanded.nelts = 1;
+      expanded.sals = xmalloc (sizeof (struct symtab_and_line));
+      sal.pc = original_pc;
+      expanded.sals[0] = sal;
+      return expanded;      
+    }
+
+  if (!sal.explicit_line)
+    {
+      CORE_ADDR func_addr, func_end;
+      for (i = 0; i < expanded.nelts; ++i)
+	{
+	  CORE_ADDR pc = expanded.sals[i].pc;
+	  char *this_function;
+	  if (find_pc_partial_function (pc, &this_function, 
+					&func_addr, &func_end))
+	    {
+	      if (this_function && 
+		  strcmp (this_function, original_function) != 0)
+		{
+		  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
+		    expanded.sals[i].pc 
+		      = gdbarch_skip_prologue (current_gdbarch, pc);
+		}
+	    }
+	}
+    }
+
+  
+  if (expanded.nelts <= 1)
+    {
+      /* This is un ugly workaround. If we get zero
+       expanded sals then something is really wrong.
+      Fix that by returnign the original sal. */
+      xfree (expanded.sals);
+      expanded.nelts = 1;
+      expanded.sals = xmalloc (sizeof (struct symtab_and_line));
+      sal.pc = original_pc;
+      expanded.sals[0] = 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);
+    }
+
+  return expanded;
+}
+
 /* Add SALS.nelts breakpoints to the breakpoint table.  For each
    SALS.sal[i] breakpoint, include the corresponding ADDR_STRING[i]
    value.  COND_STRING, if not NULL, specified the condition to be
@@ -5214,11 +5336,10 @@ create_breakpoints (struct symtabs_and_l
   int i;
   for (i = 0; i < sals.nelts; ++i)
     {
-      struct symtabs_and_lines sals2;
-      sals2.sals = sals.sals + i;
-      sals2.nelts = 1;
+      struct symtabs_and_lines expanded = 
+	expand_line_sal_maybe (sals.sals[i]);
 
-      create_breakpoint (sals2, addr_string[i],
+      create_breakpoint (expanded, addr_string[i],
 			 cond_string, type, disposition,
 			 thread, ignore_count, from_tty,
 			 pending_bp);
@@ -6889,6 +7010,23 @@ clear_command (char *arg, int from_tty)
       default_match = 1;
     }
 
+  /* We don't call resolve_sal_pc here. That's not
+     as bad as it seems, because all existing breakpoints
+     typically have both file/line and pc set.  So, if
+     clear is given file/line, we can match this to existing
+     breakpoint without obtaining pc at all.
+
+     We only support clearing given the address explicitly 
+     present in breakpoint table.  Say, we've set breakpoint 
+     at file:line. There were several PC values for that file:line,
+     due to optimization, all in one block.
+     We've picked on PC value. If "clear" is issued with another
+     PC corresponding to the same file:line, the breakpoint won't
+     be cleared.  We probably can still clear the breakpoint, but 
+     since the other PC value is never presented to user, user
+     can only find it by guessing, and it does not seem important
+     to support that.  */
+
   /* For each line spec given, delete bps which correspond
      to it.  Do it in two passes, solely to preserve the current
      behavior that from_tty is forced true if we delete more than
@@ -7404,8 +7542,12 @@ update_breakpoint_locations (struct brea
       }
   }
 
-  if (existing_locations)
-    free_bp_location (existing_locations);
+  while (existing_locations)
+    {
+      struct bp_location *next = existing_locations->next;
+      free_bp_location (existing_locations);
+      existing_locations = next;
+    }
 }
 
 
@@ -7423,6 +7565,7 @@ breakpoint_re_set_one (void *bint)
   int not_found = 0;
   int *not_found_ptr = &not_found;
   struct symtabs_and_lines sals = {};
+  struct symtabs_and_lines expanded;
   char *s;
   enum enable_state save_enable;
   struct gdb_exception e;
@@ -7497,8 +7640,8 @@ breakpoint_re_set_one (void *bint)
 	  b->thread = thread;
 	  b->condition_not_parsed = 0;
 	}
-
-      update_breakpoint_locations (b, sals);
+      expanded = expand_line_sal_maybe (sals.sals[0]);
+      update_breakpoint_locations (b, expanded);
 
       /* Now that this is re-enabled, check_duplicates
 	 can be used. */
--- gdb/testsuite/gdb.cp/mb-ctor.cc	(/work/mb_mainline/8_multiple_locations)	(revision 4832)
+++ gdb/testsuite/gdb.cp/mb-ctor.cc	(/work/mb_mainline/9_expansion)	(revision 4832)
@@ -0,0 +1,58 @@
+
+#include <stdio.h>
+
+class Base 
+{
+public:
+  Base(int k);
+  ~Base();
+  virtual void foo() {}
+private:
+  int k;
+};
+
+Base::Base(int k)
+{
+  this->k = k;
+}
+
+Base::~Base()
+{
+    printf("~Base\n");
+}
+
+class Derived : public virtual Base
+{
+public:
+  Derived(int i);
+  ~Derived();
+private:
+  int i;
+};
+
+Derived::Derived(int i) : Base(i)
+{
+  this->i = i;
+}
+
+Derived::~Derived()
+{
+    printf("~Derived\n");
+}
+
+class DeeplyDerived : public Derived
+{
+public:
+  DeeplyDerived(int i) : Base(i), Derived(i) {}
+};
+
+int main()
+{
+  /* Invokes the Derived ctor that constructs both
+     Derived and Base.  */
+  Derived d(7);
+  /* Invokes the Derived ctor that constructs only
+     Derived. Base is constructed separately by
+     DeeplyDerived's ctor.  */
+  DeeplyDerived dd(15);
+}
--- gdb/testsuite/gdb.cp/mb-ctor.exp	(/work/mb_mainline/8_multiple_locations)	(revision 4832)
+++ gdb/testsuite/gdb.cp/mb-ctor.exp	(/work/mb_mainline/9_expansion)	(revision 4832)
@@ -0,0 +1,86 @@
+# Copyright 2007
+# 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 that breakpoints on C++ constructors work, despite the
+# fact that gcc generates several versions of constructor function.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+set testfile "mb-ctor"
+set srcfile ${testfile}.cc
+set binfile ${objdir}/${subdir}/${testfile}
+
+if [get_compiler_info ${binfile} "c++"] {
+    return -1
+}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++}] != "" } {
+     untested mb-ctor.exp
+     return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+# Set a breakpoint with multiple locations
+# and a condition.
+
+gdb_test "break 'Derived::Derived(int)'" \
+    "Breakpoint.*at.* file .*$srcfile, line.*\\(2 locations\\).*" \
+    "set-breakpoint at ctor"
+
+gdb_test "break 'Derived::~Derived()'" \
+    "Breakpoint.*at.* file .*$srcfile, line.*\\(2 locations\\).*" \
+    "set-breakpoint at ctor"
+
+gdb_run_cmd
+gdb_expect {
+    -re "Breakpoint \[0-9\]+,.*Derived.*i=7.*$gdb_prompt $" {
+	pass "run to breakpoint"
+    }
+    -re "$gdb_prompt $" {
+	fail "run to breakpoint"
+    }
+    timeout {
+	fail "run to breakpoint (timeout)"
+    }
+}
+
+gdb_test "continue" \
+    ".*Breakpoint.*Derived.*i=15.*" \
+    "run to breakpoint 2"
+
+gdb_test "continue" \
+    ".*Breakpoint.*~Derived.*" \
+    "run to breakpoint 3"
+
+gdb_test "continue" \
+    ".*Breakpoint.*~Derived.*" \
+    "run to breakpoint 4"
+
+gdb_test "continue" \
+    ".*exited normally.*" \
+    "run to exit"
+
+
+
--- gdb/testsuite/gdb.cp/mb-templates.cc	(/work/mb_mainline/8_multiple_locations)	(revision 4832)
+++ gdb/testsuite/gdb.cp/mb-templates.cc	(/work/mb_mainline/9_expansion)	(revision 4832)
@@ -0,0 +1,19 @@
+
+#include <iostream>
+using namespace std;
+
+template<class T>
+void foo(T i)
+{
+  std::cout << "hi\n"; // set breakpoint here
+}
+
+int main()
+{
+    foo<int>(0);
+    foo<double>(0);
+    foo<int>(1);
+    foo<double>(1);
+    foo<int>(2);
+    foo<double>(2);
+}
--- gdb/testsuite/gdb.cp/mb-templates.exp	(/work/mb_mainline/8_multiple_locations)	(revision 4832)
+++ gdb/testsuite/gdb.cp/mb-templates.exp	(/work/mb_mainline/9_expansion)	(revision 4832)
@@ -0,0 +1,161 @@
+# Copyright 2007
+# 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/>.
+
+# This test verifies that setting breakpoint on line in template
+# function will fire in all instantiations of that template.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+set testfile "mb-templates"
+set srcfile ${testfile}.cc
+set binfile ${objdir}/${subdir}/${testfile}
+
+if [get_compiler_info ${binfile} "c++"] {
+    return -1
+}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++}] != "" } {
+     untested mb-templates.exp
+     return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+set bp_location [gdb_get_line_number "set breakpoint here"]
+
+# Set a breakpoint with multiple locations
+# and a condition.
+
+gdb_test "break $srcfile:$bp_location if i==1" \
+    "Breakpoint.*at.* file .*$srcfile, line.*\\(2 locations\\).*" \
+    "initial condition: set breakpoint"
+
+gdb_run_cmd
+gdb_expect {
+    -re "Breakpoint \[0-9\]+,.*foo<int> \\(i=1\\).*$gdb_prompt $" {
+	pass "initial condition: run to breakpoint"
+    }
+    -re "$gdb_prompt $" {
+	fail "initial condition: run to breakpoint"
+    }
+    timeout {
+	fail "initial condition: run to breakpoint (timeout)"
+    }
+}
+
+gdb_test "continue" \
+    ".*Breakpoint.*foo<double> \\(i=1\\).*" \
+    "initial condition: run to breakpoint 2"
+
+# Set breakpoint with multiple locations.
+# Separately set the condition.
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+gdb_test "break $srcfile:$bp_location" \
+    "Breakpoint.*at.* file .*$srcfile, line.*\\(2 locations\\).*" \
+    "separate condition: set breakpoint"
+
+gdb_test "condition 1 i==1" "" \
+    "separate condition: set condition"
+    
+gdb_run_cmd
+gdb_expect {
+    -re "Breakpoint \[0-9\]+,.*foo<int> \\(i=1\\).*$gdb_prompt $" {
+	pass "separate condition: run to breakpoint"
+    }
+    -re "$gdb_prompt $" {
+	fail "separate condition: run to breakpoint"
+    }
+    timeout {
+	fail "separate condition: run to breakpoint (timeout)"
+    }
+}
+
+gdb_test "continue" \
+    ".*Breakpoint.*foo<double> \\(i=1\\).*" \
+    "separate condition: run to breakpoint 2"
+
+# Try disabling a single location. We also test
+# that at least in simple cases, the enable/disable
+# state of locations surive "run".
+gdb_test "disable 1.1" "" "disabling location: disable"
+
+gdb_run_cmd
+gdb_expect {
+    -re "Breakpoint \[0-9\]+,.*foo<double> \\(i=1\\).*$gdb_prompt $" {
+	pass "disabling location: run to breakpoint"
+    }
+    -re "$gdb_prompt $" {
+	fail "disabling location: run to breakpoint"
+    }
+    timeout {
+	fail "disabling location: run to breakpoint (timeout)"
+    }
+}
+
+# Try disabling entire breakpoint
+gdb_test "enable 1.1" "" "disabling location: enable"
+
+
+gdb_test "disable 1" "" "disable breakpoint: disable"
+
+gdb_run_cmd
+gdb_expect {
+    -re "Program exited normally.*$gdb_prompt $" {
+	pass "disable breakpoint: run to breakpoint"
+    }
+    -re "$gdb_prompt $" {
+	fail "disable breakpoint: run to breakpoint"
+    }
+    timeout {
+	fail "disable breakpoint: run to breakpoint (timeout)"
+    }
+}
+
+# Make sure breakpoint can be set on a specific instantion.
+delete_breakpoints
+gdb_test "break 'void foo<int>(int)'" ".*" \
+    "instantiation: set breakpoint"
+
+
+gdb_run_cmd
+gdb_expect {
+    -re ".*Breakpoint \[0-9\]+,.*foo<int> \\(i=0\\).*$gdb_prompt $" {
+	pass "instantiation: run to breakpoint"
+    }
+    -re "$gdb_prompt $" {
+	fail "instantiation: run to breakpoint"
+    }
+    timeout {
+	fail "instantiation: run to breakpoint (timeout)"
+    }
+}
+
+gdb_test "continue" \
+    ".*Breakpoint.*foo<int> \\(i=1\\).*" \
+    "instantiation: run to breakpoint 2"
+
--- gdb/linespec.c	(/work/mb_mainline/8_multiple_locations)	(revision 4832)
+++ gdb/linespec.c	(/work/mb_mainline/9_expansion)	(revision 4832)
@@ -963,6 +963,7 @@ decode_indirect (char **argptr)
   values.sals[0] = find_pc_line (pc, 0);
   values.sals[0].pc = pc;
   values.sals[0].section = find_pc_overlay (pc);
+  values.sals[0].explicit_pc = 1;
 
   return values;
 }
@@ -1633,6 +1634,7 @@ decode_all_digits (char **argptr, struct
   values.nelts = 1;
   if (need_canonical)
     build_canonical_line_spec (values.sals, NULL, canonical);
+  values.sals[0].explicit_line = 1;
   return values;
 }
 
--- gdb/symtab.c	(/work/mb_mainline/8_multiple_locations)	(revision 4832)
+++ gdb/symtab.c	(/work/mb_mainline/9_expansion)	(revision 4832)
@@ -691,6 +691,8 @@ init_sal (struct symtab_and_line *sal)
   sal->line = 0;
   sal->pc = 0;
   sal->end = 0;
+  sal->explicit_pc = 0;
+  sal->explicit_line = 0;
 }
 \f
 
@@ -4172,6 +4174,166 @@ symtab_observer_executable_changed (void
   set_main_name (NULL);
 }
 
+/* Helper to expand_line_sal below.  Appends new sal to SAL,
+   initializing it from SYMTAB, LINENO and PC.  */
+static void
+append_expanded_sal (struct symtabs_and_lines *sal,
+		     struct symtab *symtab,
+		     int lineno, CORE_ADDR pc)
+{
+  CORE_ADDR func_addr, func_end;
+  
+  sal->sals = xrealloc (sal->sals, 
+			sizeof (sal->sals[0]) 
+			* (sal->nelts + 1));
+  init_sal (sal->sals + sal->nelts);
+  sal->sals[sal->nelts].symtab = symtab;
+  sal->sals[sal->nelts].section = NULL;
+  sal->sals[sal->nelts].end = 0;
+  sal->sals[sal->nelts].line = lineno;  
+  sal->sals[sal->nelts].pc = pc;
+  ++sal->nelts;      
+}
+
+/* Compute a set of all sals in
+   the entire program that correspond to same file
+   and line as SAL and return those.  If there
+   are several sals that belong to the same block,
+   only one sal for the block is included in results.  */
+   
+struct symtabs_and_lines
+expand_line_sal (struct symtab_and_line sal)
+{
+  struct symtabs_and_lines ret, this_line;
+  int i, j;
+  struct objfile *objfile;
+  struct partial_symtab *psymtab;
+  struct symtab *symtab;
+  int lineno;
+  int deleted = 0;
+  struct block **blocks = NULL;
+  int *filter;
+
+  ret.nelts = 0;
+  ret.sals = NULL;
+
+  if (sal.symtab == NULL || sal.line == 0 || sal.pc != 0)
+    {
+      ret.sals = xmalloc (sizeof (struct symtab_and_line));
+      ret.sals[0] = sal;
+      ret.nelts = 1;
+      return ret;
+    }
+  else
+    {
+      struct linetable_entry *best_item = 0;
+      struct symtab *best_symtab = 0;
+      int exact = 0;
+
+      lineno = sal.line;
+
+      /* We meed to find all symtabs for a file which name
+	 is described by sal. We cannot just directly 
+	 iterate over symtabs, since a symtab might not be
+	 yet created. We also cannot iterate over psymtabs,
+	 calling PSYMTAB_TO_SYMTAB and working on that symtab,
+	 since PSYMTAB_TO_SYMTAB will return NULL for psymtab
+	 corresponding to an included file. Therefore, we do
+	 first pass over psymtabs, reading in those with
+	 the right name.  Then, we iterate over symtabs, knowing
+	 that all symtabs we're interested in are loaded.  */
+
+      ALL_PSYMTABS (objfile, psymtab)
+	{
+	  if (strcmp (sal.symtab->filename,
+		      psymtab->filename) == 0)
+	    PSYMTAB_TO_SYMTAB (psymtab);
+	}
+
+	 
+      /* For each symtab, we add all pcs to ret.sals. I'm actually
+	 not sure what to do if we have exact match in one symtab,
+	 and non-exact match on another symtab.
+      */
+      ALL_SYMTABS (objfile, symtab)
+	{
+	  if (strcmp (sal.symtab->filename,
+		      symtab->filename) == 0)
+	    {
+	      struct linetable *l;
+	      int len;
+	      l = LINETABLE (symtab);
+	      if (!l)
+		continue;
+	      len = l->nitems;
+
+	      for (j = 0; j < len; j++)
+		{
+		  struct linetable_entry *item = &(l->item[j]);
+
+		  if (item->line == lineno)
+		    {
+		      exact = 1;
+		      append_expanded_sal (&ret, symtab, lineno, item->pc);
+		    }      
+		  else if (!exact && item->line > lineno
+			   && (best_item == NULL || item->line < best_item->line))
+		  
+		    {
+		      best_item = item;
+		      best_symtab = symtab;
+		    }
+		}
+	    }
+	}
+      if (!exact && best_item)
+	append_expanded_sal (&ret, best_symtab, lineno, best_item->pc);
+    }
+
+  /* For optimized code, compiler can scatter one source line accross
+     disjoint ranges of PC values, even when no duplicate functions
+     or inline functions are involved.  For example, 'for (;;)' inside
+     non-template non-inline non-ctor-or-dtor function can result
+     in two PC ranges.  In this case, we don't want to set breakpoint
+     on first PC of each range.  To filter such cases, we use containing
+     blocks -- for each PC found above we see if there are other PCs
+     that are in the same block.  If yes, the other PCs are filtered out.  */  
+
+  filter = xmalloc (ret.nelts * sizeof (int));
+  blocks = xmalloc (ret.nelts * sizeof (struct block *));
+  for (i = 0; i < ret.nelts; ++i)
+    {
+      filter[i] = 1;
+      blocks[i] = block_for_pc (ret.sals[i].pc);
+    }
+
+  for (i = 0; i < ret.nelts; ++i)
+    if (blocks[i] != NULL)
+      for (j = i+1; j < ret.nelts; ++j)
+	if (blocks[j] == blocks[i])
+	  {
+	    filter[j] = 0;
+	    ++deleted;
+	    break;
+	  }
+  
+  {
+    struct symtab_and_line *final = 
+      xmalloc (sizeof (struct symtab_and_line) * (ret.nelts-deleted));
+    
+    for (i = 0, j = 0; i < ret.nelts; ++i)
+      if (filter[i])
+	final[j++] = ret.sals[i];
+    
+    ret.nelts -= deleted;
+    xfree (ret.sals);
+    ret.sals = final;
+  }
+
+  return ret;
+}
+
+
 void
 _initialize_symtab (void)
 {
--- gdb/symtab.h	(/work/mb_mainline/8_multiple_locations)	(revision 4832)
+++ gdb/symtab.h	(/work/mb_mainline/9_expansion)	(revision 4832)
@@ -1213,6 +1213,8 @@ struct symtab_and_line
 
   CORE_ADDR pc;
   CORE_ADDR end;
+  int explicit_pc;
+  int explicit_line;
 };
 
 extern void init_sal (struct symtab_and_line *sal);
@@ -1404,5 +1406,7 @@ struct symbol *lookup_global_symbol_from
 						  const domain_enum domain,
 						  struct symtab **symtab);
 
+extern struct symtabs_and_lines
+expand_line_sal (struct symtab_and_line sal);
 
 #endif /* !defined(SYMTAB_H) */

Property changes on: gdb
___________________________________________________________________
Name: svk:merge
 +d48a11ec-ee1c-0410-b3f5-c20844f99675:/work/mb_mainline/8_multiple_locations/gdb:4831


Property changes on: 
___________________________________________________________________
Name: svk:merge
  d48a11ec-ee1c-0410-b3f5-c20844f99675:/work/mb_mainline/5_per_loc_cond:4824
  d48a11ec-ee1c-0410-b3f5-c20844f99675:/work/mb_mainline/6_create_breakpoints_refactoring:4825
  d48a11ec-ee1c-0410-b3f5-c20844f99675:/work/mb_mainline/7_pending:4829
 +d48a11ec-ee1c-0410-b3f5-c20844f99675:/work/mb_mainline/8_multiple_locations:4831
  e7755896-6108-0410-9592-8049d3e74e28:/mirrors/gdb/trunk:182811


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [9/9] expand locations
  2007-09-23  8:22   ` Vladimir Prus
@ 2007-09-23 19:23     ` Eli Zaretskii
  0 siblings, 0 replies; 7+ messages in thread
From: Eli Zaretskii @ 2007-09-23 19:23 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Sun, 23 Sep 2007 12:21:40 +0400
> Cc: gdb-patches@sources.redhat.com
> 
> The revised patch is attached, OK?

Please fix this typo:

> +     We've picked on PC value. If "clear" is issued with another
                     ^^
You meant "one", right?

Also, please use two spaces after a period that ends a sentence.


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2007-09-23 19:23 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-09-07 23:14 [9/9] expand locations Vladimir Prus
2007-09-08  0:21 ` Pedro Alves
2007-09-08  1:41   ` Jim Blandy
2007-09-08  3:42   ` Daniel Jacobowitz
2007-09-08 11:50 ` Eli Zaretskii
2007-09-23  8:22   ` Vladimir Prus
2007-09-23 19:23     ` Eli Zaretskii

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox