public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2] Bug 17394: we cannot put a break-point at a global function for ASM file
@ 2014-09-17 11:53 Mihail-Marian Nistor
  2014-09-25 18:06 ` Keith Seitz
  0 siblings, 1 reply; 10+ messages in thread
From: Mihail-Marian Nistor @ 2014-09-17 11:53 UTC (permalink / raw)
  To: gdb-patches; +Cc: Mihail-Marian Nistor

We need to cover the following test case: the user wants to do an action
only for the function that was defined into a selected file name.
An example: the user wants to put a breakpoint only for functions "func"
that was defined in the file name "file.s"
    e.i. of gdb command line: b file.s:func
Due to the limitation that the GAS doesn't generate debug info for
functions/symbols, we cannot find the function information if we look only
in file symbtabs that was collected by using the file name specified by
the user.
We need to look into a global default symtab if we want to find minimal
information about functions that were written in the ASM file.
And after that, we need to select only functions that were defined into
the file name specified by the user.

gdb/ChangeLog
      2014-09-15  Mihail-Marian Nistor  <mihail.nistor@freescale.com>
      PR gdb/17394
	    * linespec.c (minsym_found): Add new "linespec_p" argument.
      Update all callers.
	    (maybe_same_source_file) New function.

gdb/testsuite/ChangeLog
      2014-09-15  Mihail-Marian Nistor  <mihail.nistor@freescale.com>
      PR gdb/17394
	    * gdb.arch/break-asm-x86-file0.s: New file.
	    * gdb.arch/break-asm-x86-file.c: New file.
	    * gdb.arch/break-asm-x86-file.exp: New file.

Signed-off-by: Mihail-Marian Nistor <mihail.nistor@freescale.com>
---
 gdb/linespec.c                                | 126 +++++++++++++++++++++++++-
 gdb/testsuite/gdb.arch/break-asm-x86-file.c   |  16 ++++
 gdb/testsuite/gdb.arch/break-asm-x86-file.exp |  39 ++++++++
 gdb/testsuite/gdb.arch/break-asm-x86-file0.s  |  15 +++
 4 files changed, 194 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.arch/break-asm-x86-file.c
 create mode 100644 gdb/testsuite/gdb.arch/break-asm-x86-file.exp
 create mode 100644 gdb/testsuite/gdb.arch/break-asm-x86-file0.s

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 8a2c8e3..2592f37 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -363,8 +363,11 @@ static void decode_digits_list_mode (struct linespec_state *self,
 				     struct symtabs_and_lines *values,
 				     struct symtab_and_line val);
 
+static int maybe_same_source_file (struct symtab_and_line *sal, linespec_p ls);
+
 static void minsym_found (struct linespec_state *self, struct objfile *objfile,
 			  struct minimal_symbol *msymbol,
+			  linespec_p ls,
 			  struct symtabs_and_lines *result);
 
 static int compare_symbols (const void *a, const void *b);
@@ -1628,6 +1631,80 @@ linespec_parse_basic (linespec_parser *parser)
 			 PARSER_RESULT (parser)->file_symtabs, name,
 			 &symbols, &minimal_symbols);
 
+  /* We need to cover the following test case: the user wants
+     to do an action only for the function that was defined
+     into a selected file name.
+     An example: the user wants to put a breakpoint only for
+     functions "func" that was defined in the file name "file.s"
+         e.i. of gdb command line: b file.s:func
+     Due to the limitation that the GAS doesn't generate debug
+     info for functions/symbols,we cannot find the function
+     information if we look only in file symbtabs that was collected
+     by using the file name specified by the user. We need to look
+     into a global default symtab if we want to find minimal
+     information about functions that were written in the ASM file.
+     And after that, we need to select only functions that
+     were defined into the file name specified by the user. We do this
+     action into the mimi_found in the minsym_found function */
+
+  /* Verify if the user has specified a file name that was used
+     to build the current file symtabs. */
+  if (PARSER_RESULT (parser)->source_filename != NULL)
+    {
+      VEC (symtab_ptr) *file_symtabs = NULL;
+      int ix;
+      struct symtab *elt;
+      int global_symtabs = 0;
+
+      /* Verify if we have already used the global default symtab
+         information to find the function/method. */
+      for (ix = 0;
+	   VEC_iterate (symtab_ptr, PARSER_RESULT (parser)->file_symtabs,
+			ix, elt);
+	   ++ix)
+	{
+	  VEC_safe_push (symtab_ptr, file_symtabs, elt);
+	  if (elt == NULL)
+	    global_symtabs = 1;
+	}
+
+      if (!global_symtabs)
+	VEC_safe_push (symtab_ptr, file_symtabs, NULL);
+
+      if (file_symtabs != NULL)
+	{
+	  int ims;
+	  bound_minimal_symbol_d *minsym;
+	  VEC (symbolp) *ignore_symbols;
+	  VEC (bound_minimal_symbol_d) *minimal_syms;
+
+	  find_linespec_symbols (PARSER_STATE (parser),
+				 file_symtabs, name,
+				 &ignore_symbols, &minimal_syms);
+
+	  /* We should ignore information about function/method that
+	     were found by using .debug_info information, because this
+	     information was already stored into the symbols vector. */
+	  if (ignore_symbols)
+	    VEC_free(symbolp, ignore_symbols);
+
+	  /* Copy information about function/method from minimal_syms
+	     to minimal_symbols vector. */
+	  if (minimal_syms != NULL)
+	    {
+	      for (ims = 0;
+		   VEC_iterate (bound_minimal_symbol_d, minimal_syms,
+				ims, minsym);
+		   ++ims)
+		VEC_safe_push (bound_minimal_symbol_d,
+			       minimal_symbols, minsym);
+
+		VEC_free (bound_minimal_symbol_d, minimal_syms);
+	    }
+	  VEC_free (symtab_ptr, file_symtabs);
+	}
+    }
+
   if (symbols != NULL || minimal_symbols != NULL)
     {
       PARSER_RESULT (parser)->function_symbols = symbols;
@@ -2059,7 +2136,7 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls)
 	    {
 	      pspace = elem->objfile->pspace;
 	      set_current_program_space (pspace);
-	      minsym_found (state, elem->objfile, elem->minsym, &sals);
+	      minsym_found (state, elem->objfile, elem->minsym, ls, &sals);
 	    }
 	}
     }
@@ -2337,6 +2414,13 @@ parse_linespec (linespec_parser *parser, const char **argptr)
   values = convert_linespec_to_sals (PARSER_STATE (parser),
 				     PARSER_RESULT (parser));
 
+  if (values.nelts == 0)
+    {
+      /* The symbol is not found. */
+      symbol_not_found_error (PARSER_RESULT (parser)->function_name,
+			      PARSER_RESULT (parser)->source_filename);
+    }
+
   return values;
 }
 
@@ -3409,12 +3493,49 @@ collect_symbols (struct symbol *sym, void *data)
   return 1; /* Continue iterating.  */
 }
 
+/* Check whether the user source file is the same as the source
+   file from SAL. If so, return 1.  Otherwise, return  0.  */
+static int
+maybe_same_source_file (struct symtab_and_line *sal, linespec_p ls)
+{
+  /* We know if the user has specified a source file name
+     by using the source_filename field from linespec. */
+  if (ls->source_filename != NULL)
+    {
+      int ix;
+      struct symtab *elt;
+      const char *fullname;
+
+      if (sal->symtab == NULL)
+	return 0;
+
+      fullname = symtab_to_fullname (sal->symtab);
+
+      for (ix = 0;
+	   VEC_iterate (symtab_ptr, ls->file_symtabs, ix, elt);
+	   ++ix)
+	{
+	  if (elt != NULL)
+	    {
+	      const char *name = symtab_to_fullname (elt);
+	      if (strcmp (fullname, name) == 0)
+		return 1;
+	    }
+	}
+
+      return 0;
+    }
+
+  return 1;
+}
+
 /* We've found a minimal symbol MSYMBOL in OBJFILE to associate with our
    linespec; return the SAL in RESULT.  */
 
 static void
 minsym_found (struct linespec_state *self, struct objfile *objfile,
 	      struct minimal_symbol *msymbol,
+	      linespec_p ls,
 	      struct symtabs_and_lines *result)
 {
   struct gdbarch *gdbarch = get_objfile_arch (objfile);
@@ -3434,7 +3555,8 @@ minsym_found (struct linespec_state *self, struct objfile *objfile,
   if (self->funfirstline)
     skip_prologue_sal (&sal);
 
-  if (maybe_add_address (self->addr_set, objfile->pspace, sal.pc))
+  if (maybe_add_address (self->addr_set, objfile->pspace, sal.pc)
+    && maybe_same_source_file (&sal, ls))
     add_sal_to_sals (self, result, &sal, MSYMBOL_NATURAL_NAME (msymbol), 0);
 }
 
diff --git a/gdb/testsuite/gdb.arch/break-asm-x86-file.c b/gdb/testsuite/gdb.arch/break-asm-x86-file.c
new file mode 100644
index 0000000..ebaaa72
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/break-asm-x86-file.c
@@ -0,0 +1,16 @@
+void func2();
+
+static func()
+{
+}
+
+void func1()
+{
+  func2();
+  func();
+}
+
+int main()
+{
+  func1();
+}
diff --git a/gdb/testsuite/gdb.arch/break-asm-x86-file.exp b/gdb/testsuite/gdb.arch/break-asm-x86-file.exp
new file mode 100644
index 0000000..db304c5
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/break-asm-x86-file.exp
@@ -0,0 +1,39 @@
+# Copyright 2012-2014 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/>.
+
+# Bug 17394
+# Test for break-point at a global function only for a selected ASM file.
+
+standard_testfile .c
+set execfile $testfile
+set asm_file break-asm-x86-file0.s
+
+if { ![istarget "i?86-*-*"] && ![istarget "x86_64-*-*"] } then {
+    verbose "Skipping ${testfile}."
+    return
+}
+
+if {[prepare_for_testing ${testfile}.exp $execfile \
+	 [list $asm_file $srcfile ] \
+	 {debug nowarnings optimize=-O0}]} {
+    untested "Skipping ${testfile}."
+    return
+}
+
+clean_restart $execfile
+
+gdb_test "break $asm_file:func" \
+    "Breakpoint 1 at 0x\[0-9a-fA-F\]+: file .*$asm_file, line 15\\\." \
+    "set a break-point at a global function only for a selected ASM file."
diff --git a/gdb/testsuite/gdb.arch/break-asm-x86-file0.s b/gdb/testsuite/gdb.arch/break-asm-x86-file0.s
new file mode 100644
index 0000000..f6dd0c0
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/break-asm-x86-file0.s
@@ -0,0 +1,15 @@
+	.section .text
+
+	.global _func2
+_func2:
+	.global func2
+	.type   func2, @function
+func2:
+	ret
+
+	.global _func
+_func:
+	.global func
+       .type   func, @function
+func:
+	ret
-- 
1.9.1

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

* Re: [PATCH v2] Bug 17394: we cannot put a break-point at a global function for ASM file
  2014-09-17 11:53 [PATCH v2] Bug 17394: we cannot put a break-point at a global function for ASM file Mihail-Marian Nistor
@ 2014-09-25 18:06 ` Keith Seitz
  2014-09-28 15:47   ` mihail.nistor
  0 siblings, 1 reply; 10+ messages in thread
From: Keith Seitz @ 2014-09-25 18:06 UTC (permalink / raw)
  To: Mihail-Marian Nistor, gdb-patches

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

Hi,

On 09/17/2014 04:53 AM, Mihail-Marian Nistor wrote:
> We need to cover the following test case: the user wants to do an action
> only for the function that was defined into a selected file name.
> An example: the user wants to put a breakpoint only for functions "func"
> that was defined in the file name "file.s"
>      e.i. of gdb command line: b file.s:func
> Due to the limitation that the GAS doesn't generate debug info for
> functions/symbols, we cannot find the function information if we look only
> in file symbtabs that was collected by using the file name specified by
> the user.
> We need to look into a global default symtab if we want to find minimal
> information about functions that were written in the ASM file.
> And after that, we need to select only functions that were defined into
> the file name specified by the user.

Thanks for the detailed explanation (and *especially* the test case) -- 
those have helped me immensely to understand the problem.

I spent some time looking into this patch/problem, and I am not quite 
sure the proposed solution is the right way to attack this.

My current feeling on this is: the behavior of the linespec parser (and 
coordinating routines) should not behave necessarily any different 
between the two use cases "break foo.c:func" and "break foo.s:func". 
Yet, the former works and the later does not.

So I did some digging. Maybe you have already discounted the approach I 
am going to suggest -- if so, I would be very interested in hearing 
about it.

When a user specifies a C/C++/Ada/Java/Asm/whatever filename, the 
linespec parser starts by building a list of file symtabs for the 
specified files.

All subsequent symbol searches are limited to results in those 
files/symtabs.

The problem here is that, as you point out, gas does not emit any symbol 
information for the .s file. Thus, we have a symtab for the file ("info 
sources" shows the file), but it contains no symbols.

When find_linespec_symbols is called in linespec_parse_basic, it calls 
find_function_symbols, which uses add_matching_symbols_to_info to 
collect all matching symbols.

That function does [pardon any mangled formatting]:

   for (ix = 0; VEC_iterate (symtab_ptr, info->file_symtabs, ix, elt); ++ix)
     {
       if (elt == NULL)
	{
	  iterate_over_all_matching_symtabs (info->state, name, VAR_DOMAIN,
					     collect_symbols, info,
					     pspace, 1);
	  search_minsyms_for_name (info, name, pspace);
	}
       else if (pspace == NULL || pspace == SYMTAB_PSPACE (elt))
	{
	  /* Program spaces that are executing startup should have
	     been filtered out earlier.  */
	  gdb_assert (!SYMTAB_PSPACE (elt)->executing_startup);
	  set_current_program_space (SYMTAB_PSPACE (elt));
	  iterate_over_file_blocks (elt, name, VAR_DOMAIN,
				    collect_symbols, info);
	}
     }

This iterates over the symtabs. In the failing use case, ELT is non-NULL 
(points to the symtab for the .s file), so it calls 
iterate_over_file_blocks. Herein is where the problem exists: it is 
assumed that if NAME exists, it must exist in the given symtab -- a 
reasonable assumption for "normal" (non-asm) cases. It never searches 
minimal symbols (or in the global default symtab).

This is where I think where the fix should start. While attempting to 
convince myself that approach is both appropriate and "correct," I've 
actually written a version of your patch which does this.

It is important to note that iterating over minsyms is fairly expensive, 
so in my patch, I've opted to only search minimal symbols for NAME if 
the symtab's language is language_asm and iterate_over_file_blocks 
returns no symbols. That should, hopefully, mitigate any performance 
impact this might have.

This is especially exasperated by the need to map the address of the 
minimal symbol back to its symtab. You'll see this (expensive) added 
complexity in add_minsym.

When all is said and done, though, when find_linespec_symbols returns, 
it will have collated the appropriate symbol from the .s file -- exactly 
the same way it would have if one had typed "break file.c:func".

What do you think about this? Does this fail on any use cases you have?

As for the test case, I would very much like to see this important 
functionality tested on every platform. I haven't tried it yet myself, 
but I see that some other tests in our test suite use some minimal 
assembler program which (presumably) runs on nearly every configuration. 
See, for example, gdb.dwarf2/dw2-anonymous-func.exp.
Your test example is simple enough that it should be fairly trivial to 
fixup.

In any case, I would move the test from gdb.arch to gdb.linespec, 
collecting it together with its linespec-specific test brethren.

Keith

[-- Attachment #2: 17394-proposed.patch --]
[-- Type: text/x-patch, Size: 4972 bytes --]

---
 gdb/linespec.c | 95 +++++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 71 insertions(+), 24 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 8a2c8e3..702ffb3 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -3446,6 +3446,9 @@ struct collect_minsyms
   /* The objfile we're examining.  */
   struct objfile *objfile;
 
+  /* Only search the given symtab, or NULL to search for all symbols.  */
+  struct symtab *symtab;
+
   /* The funfirstline setting from the initial call.  */
   int funfirstline;
 
@@ -3505,6 +3508,24 @@ add_minsym (struct minimal_symbol *minsym, void *d)
   mo.minsym = minsym;
   mo.objfile = info->objfile;
 
+  if (info->symtab != NULL)
+    {
+      CORE_ADDR pc;
+      struct symtab_and_line sal;
+      struct gdbarch *gdbarch = get_objfile_arch (info->objfile);
+
+      sal = find_pc_sect_line (MSYMBOL_VALUE_ADDRESS (info->objfile, minsym),
+			       NULL, 0);
+      sal.section = MSYMBOL_OBJ_SECTION (info->objfile, minsym);
+      pc
+	= gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc, &current_target);
+      if (pc != sal.pc)
+	sal = find_pc_sect_line (pc, NULL, 0);
+
+      if (info->symtab != sal.symtab)
+	return;
+    }
+
   /* Exclude data symbols when looking for breakpoint locations.   */
   if (!info->list_mode)
     switch (minsym->type)
@@ -3531,40 +3552,59 @@ add_minsym (struct minimal_symbol *minsym, void *d)
   VEC_safe_push (bound_minimal_symbol_d, info->msyms, &mo);
 }
 
-/* Search minimal symbols in all objfiles for NAME.  If SEARCH_PSPACE
+/* Search for minimal symbols called NAME.  If SEARCH_PSPACE
    is not NULL, the search is restricted to just that program
-   space.  */
+   space.
+
+   If SYMTAB is NULL, search all objfiles, otherwise
+   restrict results to the given SYMTAB.  */
 
 static void
 search_minsyms_for_name (struct collect_info *info, const char *name,
-			 struct program_space *search_pspace)
+			 struct program_space *search_pspace,
+			 struct symtab *symtab)
 {
-  struct objfile *objfile;
-  struct program_space *pspace;
+  struct collect_minsyms local;
+  struct cleanup *cleanup;
 
-  ALL_PSPACES (pspace)
-  {
-    struct collect_minsyms local;
-    struct cleanup *cleanup;
+  memset (&local, 0, sizeof (local));
+  local.funfirstline = info->state->funfirstline;
+  local.list_mode = info->state->list_mode;
+  local.symtab = symtab;
 
-    if (search_pspace != NULL && search_pspace != pspace)
-      continue;
-    if (pspace->executing_startup)
-      continue;
+  cleanup = make_cleanup (VEC_cleanup (bound_minimal_symbol_d), &local.msyms);
 
-    set_current_program_space (pspace);
+  if (symtab == NULL)
+    {
+      struct program_space *pspace;
+
+      ALL_PSPACES (pspace)
+      {
+	struct objfile *objfile;
 
-    memset (&local, 0, sizeof (local));
-    local.funfirstline = info->state->funfirstline;
-    local.list_mode = info->state->list_mode;
+	if (search_pspace != NULL && search_pspace != pspace)
+	  continue;
+	if (pspace->executing_startup)
+	  continue;
 
-    cleanup = make_cleanup (VEC_cleanup (bound_minimal_symbol_d),
-			    &local.msyms);
+	set_current_program_space (pspace);
 
-    ALL_OBJFILES (objfile)
+	ALL_OBJFILES (objfile)
+	{
+	  local.objfile = objfile;
+	  iterate_over_minimal_symbols (objfile, name, add_minsym, &local);
+	}
+      }
+    }
+  else
     {
-      local.objfile = objfile;
-      iterate_over_minimal_symbols (objfile, name, add_minsym, &local);
+      if (search_pspace == NULL || SYMTAB_PSPACE (symtab) == search_pspace)
+	{
+	  set_current_program_space (SYMTAB_PSPACE (symtab));
+	  local.objfile = symtab->objfile;
+	  iterate_over_minimal_symbols (local.objfile, name, add_minsym,
+					&local);
+	}
     }
 
     if (!VEC_empty (bound_minimal_symbol_d, local.msyms))
@@ -3597,7 +3637,6 @@ search_minsyms_for_name (struct collect_info *info, const char *name,
       }
 
     do_cleanups (cleanup);
-  }
 }
 
 /* A helper function to add all symbols matching NAME to INFO.  If
@@ -3619,7 +3658,7 @@ add_matching_symbols_to_info (const char *name,
 	  iterate_over_all_matching_symtabs (info->state, name, VAR_DOMAIN,
 					     collect_symbols, info,
 					     pspace, 1);
-	  search_minsyms_for_name (info, name, pspace);
+	  search_minsyms_for_name (info, name, pspace, NULL);
 	}
       else if (pspace == NULL || pspace == SYMTAB_PSPACE (elt))
 	{
@@ -3629,6 +3668,14 @@ add_matching_symbols_to_info (const char *name,
 	  set_current_program_space (SYMTAB_PSPACE (elt));
 	  iterate_over_file_blocks (elt, name, VAR_DOMAIN,
 				    collect_symbols, info);
+
+	  /* If no symbols were found and this symtab is in
+	     assembler, we might actually be looking for a
+	     label for which we don't have debug info.  Check
+	     for a minimal symbol in this case.  */
+	  if (VEC_empty (symbolp, info->result.symbols)
+	      && elt->language == language_asm)
+	    search_minsyms_for_name (info, name, pspace, elt);
 	}
     }
 }
-- 
1.9.3


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

* RE: [PATCH v2] Bug 17394: we cannot put a break-point at a global function for ASM file
  2014-09-25 18:06 ` Keith Seitz
@ 2014-09-28 15:47   ` mihail.nistor
  2014-09-28 16:08     ` mihail.nistor
  0 siblings, 1 reply; 10+ messages in thread
From: mihail.nistor @ 2014-09-28 15:47 UTC (permalink / raw)
  To: Keith Seitz, gdb-patches; +Cc: mihail.nistor

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

Hi Keith,

Thank you for the time you spent on this patch/problem. I have rewritten the test case to be run on the targets that support dwarf2_support. I have also changed the location of test case from
gdb.arch to gdb.linespec as you recommended.

Your changes look very well, I have one little observation regarding the filtering condition when we need to get information from minimal symbol.
Scenario: We can mix objects that were compiled/assembled by different vendors and the third party assembler provides debug information and it doesn't have this limitation. An application may contain more than one file that have the same name. We cannot put a breakpoint at a function in all objects that have the same name if an object that comes from ASM file has debug info and is linked before the objects assembled with the GNU-GAS. In the new test case I have tried to emulate the third party assembler behavior in the break-asm-file1.s.

	{
+ 	  int prev_len = VEC_length (symbolp, info->result.symbols);
	  /* Program spaces that are executing startup should have
	     been filtered out earlier.  */
	  gdb_assert (!SYMTAB_PSPACE (elt)->executing_startup);
	  set_current_program_space (SYMTAB_PSPACE (elt));
	  iterate_over_file_blocks (elt, name, VAR_DOMAIN,
				    collect_symbols, info);
-	  /* If no symbols were found and this symtab is in
+	  /* If no new symbols were found in this iteration and this symtab is in
	     assembler, we might actually be looking for a
	     label for which we don't have debug info.  Check
	     for a minimal symbol in this case.  */
-       if (VEC_empty (symbolp, info->result.symbols)
+	  if ((prev_len == VEC_length (symbolp, info->result.symbols))
	      && elt->language == language_asm)
	    search_minsyms_for_name (info, name, pspace, elt);

	}

Best regards,
Mihai
-----Original Message-----
From: Keith Seitz [mailto:keiths@redhat.com] 
Sent: Thursday, September 25, 2014 9:07 PM
To: Nistor Mihail-MNISTOR1; gdb-patches@sourceware.org
Subject: Re: [PATCH v2] Bug 17394: we cannot put a break-point at a global function for ASM file

Hi,

On 09/17/2014 04:53 AM, Mihail-Marian Nistor wrote:
> We need to cover the following test case: the user wants to do an 
> action only for the function that was defined into a selected file name.
> An example: the user wants to put a breakpoint only for functions "func"
> that was defined in the file name "file.s"
>      e.i. of gdb command line: b file.s:func Due to the limitation 
> that the GAS doesn't generate debug info for functions/symbols, we 
> cannot find the function information if we look only in file symbtabs 
> that was collected by using the file name specified by the user.
> We need to look into a global default symtab if we want to find 
> minimal information about functions that were written in the ASM file.
> And after that, we need to select only functions that were defined 
> into the file name specified by the user.

Thanks for the detailed explanation (and *especially* the test case) -- those have helped me immensely to understand the problem.

I spent some time looking into this patch/problem, and I am not quite sure the proposed solution is the right way to attack this.

My current feeling on this is: the behavior of the linespec parser (and coordinating routines) should not behave necessarily any different between the two use cases "break foo.c:func" and "break foo.s:func". 
Yet, the former works and the later does not.

So I did some digging. Maybe you have already discounted the approach I am going to suggest -- if so, I would be very interested in hearing about it.

When a user specifies a C/C++/Ada/Java/Asm/whatever filename, the linespec parser starts by building a list of file symtabs for the specified files.

All subsequent symbol searches are limited to results in those files/symtabs.

The problem here is that, as you point out, gas does not emit any symbol information for the .s file. Thus, we have a symtab for the file ("info sources" shows the file), but it contains no symbols.

When find_linespec_symbols is called in linespec_parse_basic, it calls find_function_symbols, which uses add_matching_symbols_to_info to collect all matching symbols.

That function does [pardon any mangled formatting]:

   for (ix = 0; VEC_iterate (symtab_ptr, info->file_symtabs, ix, elt); ++ix)
     {
       if (elt == NULL)
	{
	  iterate_over_all_matching_symtabs (info->state, name, VAR_DOMAIN,
					     collect_symbols, info,
					     pspace, 1);
	  search_minsyms_for_name (info, name, pspace);
	}
       else if (pspace == NULL || pspace == SYMTAB_PSPACE (elt))
	{
	  /* Program spaces that are executing startup should have
	     been filtered out earlier.  */
	  gdb_assert (!SYMTAB_PSPACE (elt)->executing_startup);
	  set_current_program_space (SYMTAB_PSPACE (elt));
	  iterate_over_file_blocks (elt, name, VAR_DOMAIN,
				    collect_symbols, info);
	}
     }

This iterates over the symtabs. In the failing use case, ELT is non-NULL (points to the symtab for the .s file), so it calls iterate_over_file_blocks. Herein is where the problem exists: it is assumed that if NAME exists, it must exist in the given symtab -- a reasonable assumption for "normal" (non-asm) cases. It never searches minimal symbols (or in the global default symtab).

This is where I think where the fix should start. While attempting to convince myself that approach is both appropriate and "correct," I've actually written a version of your patch which does this.

It is important to note that iterating over minsyms is fairly expensive, so in my patch, I've opted to only search minimal symbols for NAME if the symtab's language is language_asm and iterate_over_file_blocks returns no symbols. That should, hopefully, mitigate any performance impact this might have.

This is especially exasperated by the need to map the address of the minimal symbol back to its symtab. You'll see this (expensive) added complexity in add_minsym.

When all is said and done, though, when find_linespec_symbols returns, it will have collated the appropriate symbol from the .s file -- exactly the same way it would have if one had typed "break file.c:func".

What do you think about this? Does this fail on any use cases you have?

As for the test case, I would very much like to see this important functionality tested on every platform. I haven't tried it yet myself, but I see that some other tests in our test suite use some minimal assembler program which (presumably) runs on nearly every configuration. 
See, for example, gdb.dwarf2/dw2-anonymous-func.exp.
Your test example is simple enough that it should be fairly trivial to fixup.

In any case, I would move the test from gdb.arch to gdb.linespec, collecting it together with its linespec-specific test brethren.

Keith

[-- Attachment #2: bug_17394_new_test_case.patch --]
[-- Type: application/octet-stream, Size: 16708 bytes --]

From 9143a3043e3e837bfaac9f4a8c0d1afae63b716d Mon Sep 17 00:00:00 2001
From: Mihail-Marian Nistor <mihail.nistor@freescale.com>
Date: Sun, 28 Sep 2014 18:36:52 +0300
Subject: [PATCH] Bug 17394: we cannot put a break-point at a global function
 for ASM file

gdb/testsuite/ChangeLog
      2014-09-15  Mihail-Marian Nistor  <mihail.nistor@freescale.com>
      PR gdb/17394
	    * gdb.linespec/break-asm-x86-file0.s: New file.
	    * gdb.linespec/break-asm-x86-file1.s: New file.
	    * gdb.linespec/break-asm-x86-file.c: New file.
	    * gdb.linespec/break-asm-x86-file.exp: New file.

Signed-off-by: Mihail-Marian Nistor <mihail.nistor@freescale.com>
---
 gdb/testsuite/gdb.arch/break-asm-file.c   |  35 +++++
 gdb/testsuite/gdb.arch/break-asm-file.exp |  55 +++++++
 gdb/testsuite/gdb.arch/break-asm-file0.s  | 218 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.arch/break-asm-file1.s  | 244 ++++++++++++++++++++++++++++++
 4 files changed, 552 insertions(+)
 create mode 100644 gdb/testsuite/gdb.arch/break-asm-file.c
 create mode 100644 gdb/testsuite/gdb.arch/break-asm-file.exp
 create mode 100644 gdb/testsuite/gdb.arch/break-asm-file0.s
 create mode 100644 gdb/testsuite/gdb.arch/break-asm-file1.s

diff --git a/gdb/testsuite/gdb.arch/break-asm-file.c b/gdb/testsuite/gdb.arch/break-asm-file.c
new file mode 100644
index 0000000..525726b
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/break-asm-file.c
@@ -0,0 +1,35 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004-2014 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/>.  */
+
+void func3();
+void func2();
+
+static func()
+{
+}
+
+void func1()
+{
+  func3();
+  func2();
+  func();
+}
+
+int main()
+{
+  func1();
+}
diff --git a/gdb/testsuite/gdb.arch/break-asm-file.exp b/gdb/testsuite/gdb.arch/break-asm-file.exp
new file mode 100644
index 0000000..f1129c3
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/break-asm-file.exp
@@ -0,0 +1,55 @@
+# Copyright 2012-2014 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/>.
+
+# Bug 17394
+# Test for break-point at a function only for a selected ASM file.
+
+load_lib dwarf.exp 
+
+standard_testfile .c
+set execfile $testfile
+set asm_file1 break-asm-file1.s
+set asm_file0 break-asm-file0.s
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+if {[prepare_for_testing ${testfile}.exp $execfile \
+	 [list $srcfile $asm_file1 $asm_file0] \
+	 {debug nowarnings optimize=-O0}]} {
+    untested "Skipping ${testfile}."
+    return
+}
+
+clean_restart $execfile
+
+gdb_test "break a/$asm_file0:func" \
+    "Breakpoint 1 at 0x\[0-9a-fA-F\]+: file .*a/$asm_file0, line 7\\\." \
+    "set a break-point at a global function only for a selected ASM file."
+
+gdb_test "delete 1"
+
+gdb_test "break b/$asm_file0:func" \
+    "Breakpoint 2 at 0x\[0-9a-fA-F\]+: file .*b/$asm_file0, line 7\\\." \
+    "set a break-point at a function only for a selected ASM file."
+
+gdb_test "delete 2"
+
+gdb_test "break $asm_file0:func" \
+    "Breakpoint 3 at 0x\[0-9a-fA-F\]+: .*$asm_file0.*(2 locations).*" \
+    "set a break-point at function in all instances for a selected ASM file."
+
diff --git a/gdb/testsuite/gdb.arch/break-asm-file0.s b/gdb/testsuite/gdb.arch/break-asm-file0.s
new file mode 100644
index 0000000..3ac7767
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/break-asm-file0.s
@@ -0,0 +1,218 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004-2014 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/>.  */
+
+	.file "a/break-asm-file0.s"
+	.text
+.Lbegin_text1:
+	.globl _func2
+_func2:
+	.globl func2
+	.type func2, %function
+func2:
+.Lbegin_func2:
+	.int 0
+	.int 0
+.Lend_func2:
+	.size func2, .-func2
+	.globl _func
+_func:
+  .globl func
+	.type func, %function
+func:
+.Lbegin_func:
+	.file 1 "a/break-asm-file0.s"
+	.int 0
+	.int 0
+.Lend_func:
+	.size func, .-func
+.Lend_text1:
+
+/* Debug information */
+
+	.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	.Lend_text1			/* DW_AT_high_pc */
+	.4byte	.Lbegin_text1			/* DW_AT_low_pc */
+	.ascii	"a/break-asm-file0.s\0"			/* DW_AT_name */
+	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
+	.2byte	0x8001				/* DW_AT_language (Mips Assembler) */
+
+	.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	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x11			/* DW_AT_low_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	0x5			/* DW_FORM_data2 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	2			/* Abbrev code */
+	.uleb128	0x2e			/* DW_TAG_subprogram */
+	.byte		0			/* has_children */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.uleb128	0x3a			/* DW_AT_decl_file */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x3b			/* DW_AT_decl_line */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.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 */
+	.uleb128	0x40			/* DW_AT_frame_base */
+	.uleb128	0xa			/* DW_FORM_block1 */
+	.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 */
+
+	.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		"a/break-asm-file0.s\0"
+	.uleb128	0
+	.uleb128	0
+	.uleb128	0
+
+	.byte		0
+
+.Lline1_lines:
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func2
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 2 */
+
+	.byte		1	/* DW_LNS_copy */
+
+  .byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func2+1
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 3 */
+
+	.byte		1	/* DW_LNS_copy */
+	
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func
+
+  .byte		3	/* DW_LNS_advance_line */
+	.sleb128	4	/* ... to 7 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func+2
+
+  .byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 8 */
+
+	.byte		1	/* DW_LNS_copy */
+
+  .byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lend_func
+
+	.byte		0	/* DW_LNE_end_of_sequence */
+	.uleb128	1
+	.byte		1
+
+.Lline1_end:
diff --git a/gdb/testsuite/gdb.arch/break-asm-file1.s b/gdb/testsuite/gdb.arch/break-asm-file1.s
new file mode 100644
index 0000000..4375295
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/break-asm-file1.s
@@ -0,0 +1,244 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004-2014 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/>.  */
+
+	.text
+.Lbegin_text1:
+	.globl _func3
+_func3:
+	.globl func3
+	.type func3, %function
+func3:
+.Lbegin_func3:
+	.int 0
+	.int 0
+.Lend_func3:
+	.size func3, .-func3
+_func:
+	.type func, %function
+func:
+.Lbegin_func:
+	.int 0
+	.int 0
+.Lend_func:
+	.size func, .-func
+.Lend_text1:
+
+/* Debug information */
+
+	.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	.Lend_text1			/* DW_AT_high_pc */
+	.4byte	.Lbegin_text1			/* DW_AT_low_pc */
+	.ascii	"b/break-asm-file0.s\0"			/* DW_AT_name */
+	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
+	.2byte	0x8001				/* DW_AT_language (Mips Assembler) */
+
+	/* func3 */
+	.uleb128	2			/* Abbrev: DW_TAG_subprogram */
+	.byte		1			/* DW_AT_external */
+	.byte		1			/* DW_AT_decl_file */
+	.byte		2			/* DW_AT_decl_line */
+	.ascii		"func3\0"		/* DW_AT_name */
+	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
+	.4byte		.Lbegin_func3	/* DW_AT_low_pc */
+	.4byte		.Lend_func3		/* DW_AT_high_pc */
+	.byte		1			/* DW_AT_frame_base: length */
+	.byte		0x55			/* DW_AT_frame_base: DW_OP_reg5 */
+
+	/* func */
+	.uleb128	2			/* Abbrev: DW_TAG_subprogram */
+	.byte		0			/* DW_AT_external */
+	.byte		1			/* DW_AT_decl_file */
+	.byte		4			/* DW_AT_decl_line */
+	.ascii		"func\0"		/* DW_AT_name */
+	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
+	.4byte		.Lbegin_func	/* DW_AT_low_pc */
+	.4byte		.Lend_func		/* DW_AT_high_pc */
+	.byte		1			/* DW_AT_frame_base: length */
+	.byte		0x55			/* DW_AT_frame_base: DW_OP_reg5 */
+
+.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	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x11			/* DW_AT_low_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	0x5			/* DW_FORM_data2 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	2			/* Abbrev code */
+	.uleb128	0x2e			/* DW_TAG_subprogram */
+	.byte		0			/* has_children */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.uleb128	0x3a			/* DW_AT_decl_file */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x3b			/* DW_AT_decl_line */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.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 */
+	.uleb128	0x40			/* DW_AT_frame_base */
+	.uleb128	0xa			/* DW_FORM_block1 */
+	.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 */
+
+	.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		"b/break-asm-file0.s\0"
+	.uleb128	0
+	.uleb128	0
+	.uleb128	0
+
+	.byte		0
+
+.Lline1_lines:
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func3
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 2 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func3+1
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 3 */
+
+	.byte		1	/* DW_LNS_copy */
+	
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func
+
+  .byte		3	/* DW_LNS_advance_line */
+	.sleb128	4	/* ... to 7 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func+1
+
+  .byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 8 */
+
+	.byte		1	/* DW_LNS_copy */
+	
+  .byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lend_func
+	
+	.byte		0	/* DW_LNE_end_of_sequence */
+	.uleb128	1
+	.byte		1
+
+.Lline1_end:
-- 
1.9.1


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

* RE: [PATCH v2] Bug 17394: we cannot put a break-point at a global function for ASM file
  2014-09-28 15:47   ` mihail.nistor
@ 2014-09-28 16:08     ` mihail.nistor
  2014-09-30 19:45       ` Keith Seitz
  0 siblings, 1 reply; 10+ messages in thread
From: mihail.nistor @ 2014-09-28 16:08 UTC (permalink / raw)
  To: Keith Seitz, gdb-patches; +Cc: mihail.nistor

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

Hi Keith,

I forgot to move the test location from gdb.arch to gdb.linespec.
You will find enclosed the patch with the updated test location.

Best regards,
Mihai

-----Original Message-----
From: Nistor Mihail-MNISTOR1 
Sent: Sunday, September 28, 2014 6:47 PM
To: Keith Seitz; gdb-patches@sourceware.org
Cc: Nistor Mihail-MNISTOR1
Subject: RE: [PATCH v2] Bug 17394: we cannot put a break-point at a global function for ASM file

Hi Keith,

Thank you for the time you spent on this patch/problem. I have rewritten the test case to be run on the targets that support dwarf2_support. I have also changed the location of test case from gdb.arch to gdb.linespec as you recommended.

Your changes look very well, I have one little observation regarding the filtering condition when we need to get information from minimal symbol.
Scenario: We can mix objects that were compiled/assembled by different vendors and the third party assembler provides debug information and it doesn't have this limitation. An application may contain more than one file that have the same name. We cannot put a breakpoint at a function in all objects that have the same name if an object that comes from ASM file has debug info and is linked before the objects assembled with the GNU-GAS. In the new test case I have tried to emulate the third party assembler behavior in the break-asm-file1.s.

	{
+ 	  int prev_len = VEC_length (symbolp, info->result.symbols);
	  /* Program spaces that are executing startup should have
	     been filtered out earlier.  */
	  gdb_assert (!SYMTAB_PSPACE (elt)->executing_startup);
	  set_current_program_space (SYMTAB_PSPACE (elt));
	  iterate_over_file_blocks (elt, name, VAR_DOMAIN,
				    collect_symbols, info);
-	  /* If no symbols were found and this symtab is in
+	  /* If no new symbols were found in this iteration and this symtab is 
+in
	     assembler, we might actually be looking for a
	     label for which we don't have debug info.  Check
	     for a minimal symbol in this case.  */
-       if (VEC_empty (symbolp, info->result.symbols)
+	  if ((prev_len == VEC_length (symbolp, info->result.symbols))
	      && elt->language == language_asm)
	    search_minsyms_for_name (info, name, pspace, elt);

	}

Best regards,
Mihai
-----Original Message-----
From: Keith Seitz [mailto:keiths@redhat.com]
Sent: Thursday, September 25, 2014 9:07 PM
To: Nistor Mihail-MNISTOR1; gdb-patches@sourceware.org
Subject: Re: [PATCH v2] Bug 17394: we cannot put a break-point at a global function for ASM file

Hi,

On 09/17/2014 04:53 AM, Mihail-Marian Nistor wrote:
> We need to cover the following test case: the user wants to do an 
> action only for the function that was defined into a selected file name.
> An example: the user wants to put a breakpoint only for functions "func"
> that was defined in the file name "file.s"
>      e.i. of gdb command line: b file.s:func Due to the limitation 
> that the GAS doesn't generate debug info for functions/symbols, we 
> cannot find the function information if we look only in file symbtabs 
> that was collected by using the file name specified by the user.
> We need to look into a global default symtab if we want to find 
> minimal information about functions that were written in the ASM file.
> And after that, we need to select only functions that were defined 
> into the file name specified by the user.

Thanks for the detailed explanation (and *especially* the test case) -- those have helped me immensely to understand the problem.

I spent some time looking into this patch/problem, and I am not quite sure the proposed solution is the right way to attack this.

My current feeling on this is: the behavior of the linespec parser (and coordinating routines) should not behave necessarily any different between the two use cases "break foo.c:func" and "break foo.s:func". 
Yet, the former works and the later does not.

So I did some digging. Maybe you have already discounted the approach I am going to suggest -- if so, I would be very interested in hearing about it.

When a user specifies a C/C++/Ada/Java/Asm/whatever filename, the linespec parser starts by building a list of file symtabs for the specified files.

All subsequent symbol searches are limited to results in those files/symtabs.

The problem here is that, as you point out, gas does not emit any symbol information for the .s file. Thus, we have a symtab for the file ("info sources" shows the file), but it contains no symbols.

When find_linespec_symbols is called in linespec_parse_basic, it calls find_function_symbols, which uses add_matching_symbols_to_info to collect all matching symbols.

That function does [pardon any mangled formatting]:

   for (ix = 0; VEC_iterate (symtab_ptr, info->file_symtabs, ix, elt); ++ix)
     {
       if (elt == NULL)
	{
	  iterate_over_all_matching_symtabs (info->state, name, VAR_DOMAIN,
					     collect_symbols, info,
					     pspace, 1);
	  search_minsyms_for_name (info, name, pspace);
	}
       else if (pspace == NULL || pspace == SYMTAB_PSPACE (elt))
	{
	  /* Program spaces that are executing startup should have
	     been filtered out earlier.  */
	  gdb_assert (!SYMTAB_PSPACE (elt)->executing_startup);
	  set_current_program_space (SYMTAB_PSPACE (elt));
	  iterate_over_file_blocks (elt, name, VAR_DOMAIN,
				    collect_symbols, info);
	}
     }

This iterates over the symtabs. In the failing use case, ELT is non-NULL (points to the symtab for the .s file), so it calls iterate_over_file_blocks. Herein is where the problem exists: it is assumed that if NAME exists, it must exist in the given symtab -- a reasonable assumption for "normal" (non-asm) cases. It never searches minimal symbols (or in the global default symtab).

This is where I think where the fix should start. While attempting to convince myself that approach is both appropriate and "correct," I've actually written a version of your patch which does this.

It is important to note that iterating over minsyms is fairly expensive, so in my patch, I've opted to only search minimal symbols for NAME if the symtab's language is language_asm and iterate_over_file_blocks returns no symbols. That should, hopefully, mitigate any performance impact this might have.

This is especially exasperated by the need to map the address of the minimal symbol back to its symtab. You'll see this (expensive) added complexity in add_minsym.

When all is said and done, though, when find_linespec_symbols returns, it will have collated the appropriate symbol from the .s file -- exactly the same way it would have if one had typed "break file.c:func".

What do you think about this? Does this fail on any use cases you have?

As for the test case, I would very much like to see this important functionality tested on every platform. I haven't tried it yet myself, but I see that some other tests in our test suite use some minimal assembler program which (presumably) runs on nearly every configuration. 
See, for example, gdb.dwarf2/dw2-anonymous-func.exp.
Your test example is simple enough that it should be fairly trivial to fixup.

In any case, I would move the test from gdb.arch to gdb.linespec, collecting it together with its linespec-specific test brethren.

Keith

[-- Attachment #2: bug_17394_linespec_test_case.patch --]
[-- Type: application/octet-stream, Size: 16783 bytes --]

From 6e300dc431fa73b38b8f6d3b268810cbf3879576 Mon Sep 17 00:00:00 2001
From: Mihail-Marian Nistor <mihail.nistor@freescale.com>
Date: Sun, 28 Sep 2014 18:58:54 +0300
Subject: [PATCH] Bug 17394: we cannot put a break-point at a global function
 for ASM file

gdb/testsuite/ChangeLog
      2014-09-15  Mihail-Marian Nistor  <mihail.nistor@freescale.com>
      PR gdb/17394
      * gdb.linespec/break-asm-x86-file.c: New file.
      * gdb.linespec/break-asm-x86-file.exp: New file.
      * gdb.linespec/break-asm-x86-file0.s: New file.
      * gdb.linespec/break-asm-x86-file1.s: New file.

Signed-off-by: Mihail-Marian Nistor <mihail.nistor@freescale.com>
---
 gdb/testsuite/gdb.linespec/break-asm-file.c   |  35 ++++
 gdb/testsuite/gdb.linespec/break-asm-file.exp |  55 ++++++
 gdb/testsuite/gdb.linespec/break-asm-file0.s  | 218 +++++++++++++++++++++++
 gdb/testsuite/gdb.linespec/break-asm-file1.s  | 244 ++++++++++++++++++++++++++
 4 files changed, 552 insertions(+)
 create mode 100644 gdb/testsuite/gdb.linespec/break-asm-file.c
 create mode 100644 gdb/testsuite/gdb.linespec/break-asm-file.exp
 create mode 100644 gdb/testsuite/gdb.linespec/break-asm-file0.s
 create mode 100644 gdb/testsuite/gdb.linespec/break-asm-file1.s

diff --git a/gdb/testsuite/gdb.linespec/break-asm-file.c b/gdb/testsuite/gdb.linespec/break-asm-file.c
new file mode 100644
index 0000000..525726b
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/break-asm-file.c
@@ -0,0 +1,35 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004-2014 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/>.  */
+
+void func3();
+void func2();
+
+static func()
+{
+}
+
+void func1()
+{
+  func3();
+  func2();
+  func();
+}
+
+int main()
+{
+  func1();
+}
diff --git a/gdb/testsuite/gdb.linespec/break-asm-file.exp b/gdb/testsuite/gdb.linespec/break-asm-file.exp
new file mode 100644
index 0000000..f1129c3
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/break-asm-file.exp
@@ -0,0 +1,55 @@
+# Copyright 2012-2014 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/>.
+
+# Bug 17394
+# Test for break-point at a function only for a selected ASM file.
+
+load_lib dwarf.exp 
+
+standard_testfile .c
+set execfile $testfile
+set asm_file1 break-asm-file1.s
+set asm_file0 break-asm-file0.s
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+if {[prepare_for_testing ${testfile}.exp $execfile \
+	 [list $srcfile $asm_file1 $asm_file0] \
+	 {debug nowarnings optimize=-O0}]} {
+    untested "Skipping ${testfile}."
+    return
+}
+
+clean_restart $execfile
+
+gdb_test "break a/$asm_file0:func" \
+    "Breakpoint 1 at 0x\[0-9a-fA-F\]+: file .*a/$asm_file0, line 7\\\." \
+    "set a break-point at a global function only for a selected ASM file."
+
+gdb_test "delete 1"
+
+gdb_test "break b/$asm_file0:func" \
+    "Breakpoint 2 at 0x\[0-9a-fA-F\]+: file .*b/$asm_file0, line 7\\\." \
+    "set a break-point at a function only for a selected ASM file."
+
+gdb_test "delete 2"
+
+gdb_test "break $asm_file0:func" \
+    "Breakpoint 3 at 0x\[0-9a-fA-F\]+: .*$asm_file0.*(2 locations).*" \
+    "set a break-point at function in all instances for a selected ASM file."
+
diff --git a/gdb/testsuite/gdb.linespec/break-asm-file0.s b/gdb/testsuite/gdb.linespec/break-asm-file0.s
new file mode 100644
index 0000000..3ac7767
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/break-asm-file0.s
@@ -0,0 +1,218 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004-2014 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/>.  */
+
+	.file "a/break-asm-file0.s"
+	.text
+.Lbegin_text1:
+	.globl _func2
+_func2:
+	.globl func2
+	.type func2, %function
+func2:
+.Lbegin_func2:
+	.int 0
+	.int 0
+.Lend_func2:
+	.size func2, .-func2
+	.globl _func
+_func:
+  .globl func
+	.type func, %function
+func:
+.Lbegin_func:
+	.file 1 "a/break-asm-file0.s"
+	.int 0
+	.int 0
+.Lend_func:
+	.size func, .-func
+.Lend_text1:
+
+/* Debug information */
+
+	.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	.Lend_text1			/* DW_AT_high_pc */
+	.4byte	.Lbegin_text1			/* DW_AT_low_pc */
+	.ascii	"a/break-asm-file0.s\0"			/* DW_AT_name */
+	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
+	.2byte	0x8001				/* DW_AT_language (Mips Assembler) */
+
+	.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	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x11			/* DW_AT_low_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	0x5			/* DW_FORM_data2 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	2			/* Abbrev code */
+	.uleb128	0x2e			/* DW_TAG_subprogram */
+	.byte		0			/* has_children */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.uleb128	0x3a			/* DW_AT_decl_file */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x3b			/* DW_AT_decl_line */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.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 */
+	.uleb128	0x40			/* DW_AT_frame_base */
+	.uleb128	0xa			/* DW_FORM_block1 */
+	.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 */
+
+	.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		"a/break-asm-file0.s\0"
+	.uleb128	0
+	.uleb128	0
+	.uleb128	0
+
+	.byte		0
+
+.Lline1_lines:
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func2
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 2 */
+
+	.byte		1	/* DW_LNS_copy */
+
+  .byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func2+1
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 3 */
+
+	.byte		1	/* DW_LNS_copy */
+	
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func
+
+  .byte		3	/* DW_LNS_advance_line */
+	.sleb128	4	/* ... to 7 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func+2
+
+  .byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 8 */
+
+	.byte		1	/* DW_LNS_copy */
+
+  .byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lend_func
+
+	.byte		0	/* DW_LNE_end_of_sequence */
+	.uleb128	1
+	.byte		1
+
+.Lline1_end:
diff --git a/gdb/testsuite/gdb.linespec/break-asm-file1.s b/gdb/testsuite/gdb.linespec/break-asm-file1.s
new file mode 100644
index 0000000..4375295
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/break-asm-file1.s
@@ -0,0 +1,244 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004-2014 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/>.  */
+
+	.text
+.Lbegin_text1:
+	.globl _func3
+_func3:
+	.globl func3
+	.type func3, %function
+func3:
+.Lbegin_func3:
+	.int 0
+	.int 0
+.Lend_func3:
+	.size func3, .-func3
+_func:
+	.type func, %function
+func:
+.Lbegin_func:
+	.int 0
+	.int 0
+.Lend_func:
+	.size func, .-func
+.Lend_text1:
+
+/* Debug information */
+
+	.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	.Lend_text1			/* DW_AT_high_pc */
+	.4byte	.Lbegin_text1			/* DW_AT_low_pc */
+	.ascii	"b/break-asm-file0.s\0"			/* DW_AT_name */
+	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
+	.2byte	0x8001				/* DW_AT_language (Mips Assembler) */
+
+	/* func3 */
+	.uleb128	2			/* Abbrev: DW_TAG_subprogram */
+	.byte		1			/* DW_AT_external */
+	.byte		1			/* DW_AT_decl_file */
+	.byte		2			/* DW_AT_decl_line */
+	.ascii		"func3\0"		/* DW_AT_name */
+	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
+	.4byte		.Lbegin_func3	/* DW_AT_low_pc */
+	.4byte		.Lend_func3		/* DW_AT_high_pc */
+	.byte		1			/* DW_AT_frame_base: length */
+	.byte		0x55			/* DW_AT_frame_base: DW_OP_reg5 */
+
+	/* func */
+	.uleb128	2			/* Abbrev: DW_TAG_subprogram */
+	.byte		0			/* DW_AT_external */
+	.byte		1			/* DW_AT_decl_file */
+	.byte		4			/* DW_AT_decl_line */
+	.ascii		"func\0"		/* DW_AT_name */
+	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
+	.4byte		.Lbegin_func	/* DW_AT_low_pc */
+	.4byte		.Lend_func		/* DW_AT_high_pc */
+	.byte		1			/* DW_AT_frame_base: length */
+	.byte		0x55			/* DW_AT_frame_base: DW_OP_reg5 */
+
+.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	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x11			/* DW_AT_low_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	0x5			/* DW_FORM_data2 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	2			/* Abbrev code */
+	.uleb128	0x2e			/* DW_TAG_subprogram */
+	.byte		0			/* has_children */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.uleb128	0x3a			/* DW_AT_decl_file */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x3b			/* DW_AT_decl_line */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.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 */
+	.uleb128	0x40			/* DW_AT_frame_base */
+	.uleb128	0xa			/* DW_FORM_block1 */
+	.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 */
+
+	.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		"b/break-asm-file0.s\0"
+	.uleb128	0
+	.uleb128	0
+	.uleb128	0
+
+	.byte		0
+
+.Lline1_lines:
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func3
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 2 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func3+1
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 3 */
+
+	.byte		1	/* DW_LNS_copy */
+	
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func
+
+  .byte		3	/* DW_LNS_advance_line */
+	.sleb128	4	/* ... to 7 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func+1
+
+  .byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 8 */
+
+	.byte		1	/* DW_LNS_copy */
+	
+  .byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lend_func
+	
+	.byte		0	/* DW_LNE_end_of_sequence */
+	.uleb128	1
+	.byte		1
+
+.Lline1_end:
-- 
1.9.1


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

* Re: [PATCH v2] Bug 17394: we cannot put a break-point at a global function for ASM file
  2014-09-28 16:08     ` mihail.nistor
@ 2014-09-30 19:45       ` Keith Seitz
  2014-10-01 18:07         ` mihail.nistor
  2014-12-13 13:04         ` Joel Brobecker
  0 siblings, 2 replies; 10+ messages in thread
From: Keith Seitz @ 2014-09-30 19:45 UTC (permalink / raw)
  To: mihail.nistor, gdb-patches

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

On 09/28/2014 09:08 AM, mihail.nistor@freescale.com wrote:
> -----Original Message-----
> From: Nistor Mihail-MNISTOR1
> Sent: Sunday, September 28, 2014 6:47 PM
> To: Keith Seitz; gdb-patches@sourceware.org
> Cc: Nistor Mihail-MNISTOR1
> Subject: RE: [PATCH v2] Bug 17394: we cannot put a break-point at a global function for ASM file
>
> Your changes look very well, I have one little observation regarding the filtering condition when we need to get information from minimal symbol.
> Scenario: We can mix objects that were compiled/assembled by different vendors and the third party assembler provides debug information and it doesn't have this limitation. An application may contain more than one file that have the same name. We cannot put a breakpoint at a function in all objects that have the same name if an object that comes from ASM file has debug info and is linked before the objects assembled with the GNU-GAS. In the new test case I have tried to emulate the third party assembler behavior in the break-asm-file1.s.
>

That makes sense to me. FWIW, here is a combined patch which I think a 
maintainer should give a final look over.

Thank you for the patch!
Keith

gdb/ChangeLog:
2014-09-30  Keith Seitz  <keiths@redhat.com>
	    Mihail-Marian Nistor  <mihail.nistor@freescale.com>

	PR gdb/17394
	* linespec.c (struct collect_minsyms): Add new member `symtab'.
	(add_minsym): Handle cases where info.symtab is non-NULL.
	(search_minsyms_for_name): Add new parameter `symtab'.
	Handle limiting searches to a specific symtab.
	(add_matching_symtabs_to_info): Search through minimal symbols
	for language_asm files for which no new symbols are found.

gdb/testsuite/ChangeLog:
2014-09-30  Mihail-Marian Nistor  <mihail.nistor@freescale.com>

       PR gdb/17394
       * gdb.linespec/break-asm-file.c: New file.
       * gdb.linespec/break-asm-file.exp: New file.
       * gdb.linespec/break-asm-file0.s: New file.
       * gdb.linespec/break-asm-file1.s: New file.



[-- Attachment #2: 17394.patch --]
[-- Type: text/x-patch, Size: 20555 bytes --]

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 8a2c8e3..bd5df70 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -3446,6 +3446,9 @@ struct collect_minsyms
   /* The objfile we're examining.  */
   struct objfile *objfile;
 
+  /* Only search the given symtab, or NULL to search for all symbols.  */
+  struct symtab *symtab;
+
   /* The funfirstline setting from the initial call.  */
   int funfirstline;
 
@@ -3505,6 +3508,24 @@ add_minsym (struct minimal_symbol *minsym, void *d)
   mo.minsym = minsym;
   mo.objfile = info->objfile;
 
+  if (info->symtab != NULL)
+    {
+      CORE_ADDR pc;
+      struct symtab_and_line sal;
+      struct gdbarch *gdbarch = get_objfile_arch (info->objfile);
+
+      sal = find_pc_sect_line (MSYMBOL_VALUE_ADDRESS (info->objfile, minsym),
+			       NULL, 0);
+      sal.section = MSYMBOL_OBJ_SECTION (info->objfile, minsym);
+      pc
+	= gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc, &current_target);
+      if (pc != sal.pc)
+	sal = find_pc_sect_line (pc, NULL, 0);
+
+      if (info->symtab != sal.symtab)
+	return;
+    }
+
   /* Exclude data symbols when looking for breakpoint locations.   */
   if (!info->list_mode)
     switch (minsym->type)
@@ -3531,40 +3552,59 @@ add_minsym (struct minimal_symbol *minsym, void *d)
   VEC_safe_push (bound_minimal_symbol_d, info->msyms, &mo);
 }
 
-/* Search minimal symbols in all objfiles for NAME.  If SEARCH_PSPACE
+/* Search for minimal symbols called NAME.  If SEARCH_PSPACE
    is not NULL, the search is restricted to just that program
-   space.  */
+   space.
+
+   If SYMTAB is NULL, search all objfiles, otherwise
+   restrict results to the given SYMTAB.  */
 
 static void
 search_minsyms_for_name (struct collect_info *info, const char *name,
-			 struct program_space *search_pspace)
+			 struct program_space *search_pspace,
+			 struct symtab *symtab)
 {
-  struct objfile *objfile;
-  struct program_space *pspace;
+  struct collect_minsyms local;
+  struct cleanup *cleanup;
 
-  ALL_PSPACES (pspace)
-  {
-    struct collect_minsyms local;
-    struct cleanup *cleanup;
+  memset (&local, 0, sizeof (local));
+  local.funfirstline = info->state->funfirstline;
+  local.list_mode = info->state->list_mode;
+  local.symtab = symtab;
 
-    if (search_pspace != NULL && search_pspace != pspace)
-      continue;
-    if (pspace->executing_startup)
-      continue;
+  cleanup = make_cleanup (VEC_cleanup (bound_minimal_symbol_d), &local.msyms);
 
-    set_current_program_space (pspace);
+  if (symtab == NULL)
+    {
+      struct program_space *pspace;
 
-    memset (&local, 0, sizeof (local));
-    local.funfirstline = info->state->funfirstline;
-    local.list_mode = info->state->list_mode;
+      ALL_PSPACES (pspace)
+      {
+	struct objfile *objfile;
 
-    cleanup = make_cleanup (VEC_cleanup (bound_minimal_symbol_d),
-			    &local.msyms);
+	if (search_pspace != NULL && search_pspace != pspace)
+	  continue;
+	if (pspace->executing_startup)
+	  continue;
 
-    ALL_OBJFILES (objfile)
+	set_current_program_space (pspace);
+
+	ALL_OBJFILES (objfile)
+	{
+	  local.objfile = objfile;
+	  iterate_over_minimal_symbols (objfile, name, add_minsym, &local);
+	}
+      }
+    }
+  else
     {
-      local.objfile = objfile;
-      iterate_over_minimal_symbols (objfile, name, add_minsym, &local);
+      if (search_pspace == NULL || SYMTAB_PSPACE (symtab) == search_pspace)
+	{
+	  set_current_program_space (SYMTAB_PSPACE (symtab));
+	  local.objfile = symtab->objfile;
+	  iterate_over_minimal_symbols (local.objfile, name, add_minsym,
+					&local);
+	}
     }
 
     if (!VEC_empty (bound_minimal_symbol_d, local.msyms))
@@ -3597,7 +3637,6 @@ search_minsyms_for_name (struct collect_info *info, const char *name,
       }
 
     do_cleanups (cleanup);
-  }
 }
 
 /* A helper function to add all symbols matching NAME to INFO.  If
@@ -3619,16 +3658,26 @@ add_matching_symbols_to_info (const char *name,
 	  iterate_over_all_matching_symtabs (info->state, name, VAR_DOMAIN,
 					     collect_symbols, info,
 					     pspace, 1);
-	  search_minsyms_for_name (info, name, pspace);
+	  search_minsyms_for_name (info, name, pspace, NULL);
 	}
       else if (pspace == NULL || pspace == SYMTAB_PSPACE (elt))
 	{
+	  int prev_len = VEC_length (symbolp, info->result.symbols);
+
 	  /* Program spaces that are executing startup should have
 	     been filtered out earlier.  */
 	  gdb_assert (!SYMTAB_PSPACE (elt)->executing_startup);
 	  set_current_program_space (SYMTAB_PSPACE (elt));
 	  iterate_over_file_blocks (elt, name, VAR_DOMAIN,
 				    collect_symbols, info);
+
+	  /* If no new symbols were found in this iteration and this symtab
+	     is in assembler, we might actually be looking for a label for
+	     which we don't have debug info.  Check for a minimal symbol in
+	     this case.  */
+	  if (prev_len == VEC_length (symbolp, info->result.symbols)
+	      && elt->language == language_asm)
+	    search_minsyms_for_name (info, name, pspace, elt);
 	}
     }
 }
diff --git a/gdb/testsuite/gdb.linespec/break-asm-file.c b/gdb/testsuite/gdb.linespec/break-asm-file.c
new file mode 100644
index 0000000..525726b
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/break-asm-file.c
@@ -0,0 +1,35 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004-2014 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/>.  */
+
+void func3();
+void func2();
+
+static func()
+{
+}
+
+void func1()
+{
+  func3();
+  func2();
+  func();
+}
+
+int main()
+{
+  func1();
+}
diff --git a/gdb/testsuite/gdb.linespec/break-asm-file.exp b/gdb/testsuite/gdb.linespec/break-asm-file.exp
new file mode 100644
index 0000000..f1129c3
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/break-asm-file.exp
@@ -0,0 +1,55 @@
+# Copyright 2012-2014 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/>.
+
+# Bug 17394
+# Test for break-point at a function only for a selected ASM file.
+
+load_lib dwarf.exp 
+
+standard_testfile .c
+set execfile $testfile
+set asm_file1 break-asm-file1.s
+set asm_file0 break-asm-file0.s
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+if {[prepare_for_testing ${testfile}.exp $execfile \
+	 [list $srcfile $asm_file1 $asm_file0] \
+	 {debug nowarnings optimize=-O0}]} {
+    untested "Skipping ${testfile}."
+    return
+}
+
+clean_restart $execfile
+
+gdb_test "break a/$asm_file0:func" \
+    "Breakpoint 1 at 0x\[0-9a-fA-F\]+: file .*a/$asm_file0, line 7\\\." \
+    "set a break-point at a global function only for a selected ASM file."
+
+gdb_test "delete 1"
+
+gdb_test "break b/$asm_file0:func" \
+    "Breakpoint 2 at 0x\[0-9a-fA-F\]+: file .*b/$asm_file0, line 7\\\." \
+    "set a break-point at a function only for a selected ASM file."
+
+gdb_test "delete 2"
+
+gdb_test "break $asm_file0:func" \
+    "Breakpoint 3 at 0x\[0-9a-fA-F\]+: .*$asm_file0.*(2 locations).*" \
+    "set a break-point at function in all instances for a selected ASM file."
+
diff --git a/gdb/testsuite/gdb.linespec/break-asm-file0.s b/gdb/testsuite/gdb.linespec/break-asm-file0.s
new file mode 100644
index 0000000..3ac7767
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/break-asm-file0.s
@@ -0,0 +1,218 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004-2014 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/>.  */
+
+	.file "a/break-asm-file0.s"
+	.text
+.Lbegin_text1:
+	.globl _func2
+_func2:
+	.globl func2
+	.type func2, %function
+func2:
+.Lbegin_func2:
+	.int 0
+	.int 0
+.Lend_func2:
+	.size func2, .-func2
+	.globl _func
+_func:
+  .globl func
+	.type func, %function
+func:
+.Lbegin_func:
+	.file 1 "a/break-asm-file0.s"
+	.int 0
+	.int 0
+.Lend_func:
+	.size func, .-func
+.Lend_text1:
+
+/* Debug information */
+
+	.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	.Lend_text1			/* DW_AT_high_pc */
+	.4byte	.Lbegin_text1			/* DW_AT_low_pc */
+	.ascii	"a/break-asm-file0.s\0"			/* DW_AT_name */
+	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
+	.2byte	0x8001				/* DW_AT_language (Mips Assembler) */
+
+	.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	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x11			/* DW_AT_low_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	0x5			/* DW_FORM_data2 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	2			/* Abbrev code */
+	.uleb128	0x2e			/* DW_TAG_subprogram */
+	.byte		0			/* has_children */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.uleb128	0x3a			/* DW_AT_decl_file */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x3b			/* DW_AT_decl_line */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.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 */
+	.uleb128	0x40			/* DW_AT_frame_base */
+	.uleb128	0xa			/* DW_FORM_block1 */
+	.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 */
+
+	.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		"a/break-asm-file0.s\0"
+	.uleb128	0
+	.uleb128	0
+	.uleb128	0
+
+	.byte		0
+
+.Lline1_lines:
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func2
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 2 */
+
+	.byte		1	/* DW_LNS_copy */
+
+  .byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func2+1
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 3 */
+
+	.byte		1	/* DW_LNS_copy */
+	
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func
+
+  .byte		3	/* DW_LNS_advance_line */
+	.sleb128	4	/* ... to 7 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func+2
+
+  .byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 8 */
+
+	.byte		1	/* DW_LNS_copy */
+
+  .byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lend_func
+
+	.byte		0	/* DW_LNE_end_of_sequence */
+	.uleb128	1
+	.byte		1
+
+.Lline1_end:
diff --git a/gdb/testsuite/gdb.linespec/break-asm-file1.s b/gdb/testsuite/gdb.linespec/break-asm-file1.s
new file mode 100644
index 0000000..4375295
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/break-asm-file1.s
@@ -0,0 +1,244 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004-2014 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/>.  */
+
+	.text
+.Lbegin_text1:
+	.globl _func3
+_func3:
+	.globl func3
+	.type func3, %function
+func3:
+.Lbegin_func3:
+	.int 0
+	.int 0
+.Lend_func3:
+	.size func3, .-func3
+_func:
+	.type func, %function
+func:
+.Lbegin_func:
+	.int 0
+	.int 0
+.Lend_func:
+	.size func, .-func
+.Lend_text1:
+
+/* Debug information */
+
+	.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	.Lend_text1			/* DW_AT_high_pc */
+	.4byte	.Lbegin_text1			/* DW_AT_low_pc */
+	.ascii	"b/break-asm-file0.s\0"			/* DW_AT_name */
+	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
+	.2byte	0x8001				/* DW_AT_language (Mips Assembler) */
+
+	/* func3 */
+	.uleb128	2			/* Abbrev: DW_TAG_subprogram */
+	.byte		1			/* DW_AT_external */
+	.byte		1			/* DW_AT_decl_file */
+	.byte		2			/* DW_AT_decl_line */
+	.ascii		"func3\0"		/* DW_AT_name */
+	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
+	.4byte		.Lbegin_func3	/* DW_AT_low_pc */
+	.4byte		.Lend_func3		/* DW_AT_high_pc */
+	.byte		1			/* DW_AT_frame_base: length */
+	.byte		0x55			/* DW_AT_frame_base: DW_OP_reg5 */
+
+	/* func */
+	.uleb128	2			/* Abbrev: DW_TAG_subprogram */
+	.byte		0			/* DW_AT_external */
+	.byte		1			/* DW_AT_decl_file */
+	.byte		4			/* DW_AT_decl_line */
+	.ascii		"func\0"		/* DW_AT_name */
+	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
+	.4byte		.Lbegin_func	/* DW_AT_low_pc */
+	.4byte		.Lend_func		/* DW_AT_high_pc */
+	.byte		1			/* DW_AT_frame_base: length */
+	.byte		0x55			/* DW_AT_frame_base: DW_OP_reg5 */
+
+.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	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x11			/* DW_AT_low_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	0x5			/* DW_FORM_data2 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	2			/* Abbrev code */
+	.uleb128	0x2e			/* DW_TAG_subprogram */
+	.byte		0			/* has_children */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.uleb128	0x3a			/* DW_AT_decl_file */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x3b			/* DW_AT_decl_line */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.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 */
+	.uleb128	0x40			/* DW_AT_frame_base */
+	.uleb128	0xa			/* DW_FORM_block1 */
+	.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 */
+
+	.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		"b/break-asm-file0.s\0"
+	.uleb128	0
+	.uleb128	0
+	.uleb128	0
+
+	.byte		0
+
+.Lline1_lines:
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func3
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 2 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func3+1
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 3 */
+
+	.byte		1	/* DW_LNS_copy */
+	
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func
+
+  .byte		3	/* DW_LNS_advance_line */
+	.sleb128	4	/* ... to 7 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func+1
+
+  .byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 8 */
+
+	.byte		1	/* DW_LNS_copy */
+	
+  .byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lend_func
+	
+	.byte		0	/* DW_LNE_end_of_sequence */
+	.uleb128	1
+	.byte		1
+
+.Lline1_end:

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

* RE: [PATCH v2] Bug 17394: we cannot put a break-point at a global function for ASM file
  2014-09-30 19:45       ` Keith Seitz
@ 2014-10-01 18:07         ` mihail.nistor
  2014-12-13 13:04         ` Joel Brobecker
  1 sibling, 0 replies; 10+ messages in thread
From: mihail.nistor @ 2014-10-01 18:07 UTC (permalink / raw)
  To: Keith Seitz, gdb-patches

Thank you for everything. I'm looking forward to receiving the final approval so that the patch may be submitted.

Best regards,
Mihai

-----Original Message-----
From: Keith Seitz [mailto:keiths@redhat.com] 
Sent: Tuesday, September 30, 2014 10:45 PM
To: Nistor Mihail-MNISTOR1; gdb-patches@sourceware.org
Subject: Re: [PATCH v2] Bug 17394: we cannot put a break-point at a global function for ASM file

On 09/28/2014 09:08 AM, mihail.nistor@freescale.com wrote:
> -----Original Message-----
> From: Nistor Mihail-MNISTOR1
> Sent: Sunday, September 28, 2014 6:47 PM
> To: Keith Seitz; gdb-patches@sourceware.org
> Cc: Nistor Mihail-MNISTOR1
> Subject: RE: [PATCH v2] Bug 17394: we cannot put a break-point at a 
> global function for ASM file
>
> Your changes look very well, I have one little observation regarding the filtering condition when we need to get information from minimal symbol.
> Scenario: We can mix objects that were compiled/assembled by different vendors and the third party assembler provides debug information and it doesn't have this limitation. An application may contain more than one file that have the same name. We cannot put a breakpoint at a function in all objects that have the same name if an object that comes from ASM file has debug info and is linked before the objects assembled with the GNU-GAS. In the new test case I have tried to emulate the third party assembler behavior in the break-asm-file1.s.
>

That makes sense to me. FWIW, here is a combined patch which I think a maintainer should give a final look over.

Thank you for the patch!
Keith

gdb/ChangeLog:
2014-09-30  Keith Seitz  <keiths@redhat.com>
	    Mihail-Marian Nistor  <mihail.nistor@freescale.com>

	PR gdb/17394
	* linespec.c (struct collect_minsyms): Add new member `symtab'.
	(add_minsym): Handle cases where info.symtab is non-NULL.
	(search_minsyms_for_name): Add new parameter `symtab'.
	Handle limiting searches to a specific symtab.
	(add_matching_symtabs_to_info): Search through minimal symbols
	for language_asm files for which no new symbols are found.

gdb/testsuite/ChangeLog:
2014-09-30  Mihail-Marian Nistor  <mihail.nistor@freescale.com>

       PR gdb/17394
       * gdb.linespec/break-asm-file.c: New file.
       * gdb.linespec/break-asm-file.exp: New file.
       * gdb.linespec/break-asm-file0.s: New file.
       * gdb.linespec/break-asm-file1.s: New file.


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

* Re: [PATCH v2] Bug 17394: we cannot put a break-point at a global function for ASM file
  2014-09-30 19:45       ` Keith Seitz
  2014-10-01 18:07         ` mihail.nistor
@ 2014-12-13 13:04         ` Joel Brobecker
  2014-12-15 20:47           ` mihail.nistor
  1 sibling, 1 reply; 10+ messages in thread
From: Joel Brobecker @ 2014-12-13 13:04 UTC (permalink / raw)
  To: Keith Seitz; +Cc: mihail.nistor, gdb-patches

> gdb/ChangeLog:
> 2014-09-30  Keith Seitz  <keiths@redhat.com>
> 	    Mihail-Marian Nistor  <mihail.nistor@freescale.com>
> 
> 	PR gdb/17394
> 	* linespec.c (struct collect_minsyms): Add new member `symtab'.
> 	(add_minsym): Handle cases where info.symtab is non-NULL.
> 	(search_minsyms_for_name): Add new parameter `symtab'.
> 	Handle limiting searches to a specific symtab.
> 	(add_matching_symtabs_to_info): Search through minimal symbols
> 	for language_asm files for which no new symbols are found.
> 
> gdb/testsuite/ChangeLog:
> 2014-09-30  Mihail-Marian Nistor  <mihail.nistor@freescale.com>
> 
>       PR gdb/17394
>       * gdb.linespec/break-asm-file.c: New file.
>       * gdb.linespec/break-asm-file.exp: New file.
>       * gdb.linespec/break-asm-file0.s: New file.
>       * gdb.linespec/break-asm-file1.s: New file.

Overall, the patch looks good to me. Just a few trivial issues
hightlighted below, but the patch is otherwise approved.

> diff --git a/gdb/testsuite/gdb.linespec/break-asm-file.exp b/gdb/testsuite/gdb.linespec/break-asm-file.exp
> +
> +load_lib dwarf.exp 

Trailing space here...

> diff --git a/gdb/testsuite/gdb.linespec/break-asm-file0.s b/gdb/testsuite/gdb.linespec/break-asm-file0.s
[...]
> @@ -0,0 +1,218 @@
> +	.byte		1	/* DW_LNS_copy */
> +
> +  .byte		0	/* DW_LNE_set_address */

This file has a few .byte's where the formatting is inconsistent with
the rest. Not critical, but would you mind fixing those?


> +	.byte		1	/* DW_LNS_copy */
> +	

Trailing spaces...


> diff --git a/gdb/testsuite/gdb.linespec/break-asm-file1.s b/gdb/testsuite/gdb.linespec/break-asm-file1.s
[...]
> +	.byte		1	/* DW_LNS_copy */
> +	

Trailing spaces again...

> +	.byte		1	/* DW_LNS_copy */
> +	
Ditto...

> +	.4byte		.Lend_func
> +	

... and Ditto.

Thank you,
-- 
Joel

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

* RE: [PATCH v2] Bug 17394: we cannot put a break-point at a global function for ASM file
  2014-12-13 13:04         ` Joel Brobecker
@ 2014-12-15 20:47           ` mihail.nistor
  2014-12-20 16:33             ` Joel Brobecker
  0 siblings, 1 reply; 10+ messages in thread
From: mihail.nistor @ 2014-12-15 20:47 UTC (permalink / raw)
  To: Keith Seitz, Joel Brobecker; +Cc: gdb-patches

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

Thank you very much for your support.

You will find enclosed a new patch.

Thank you,
Mihai

-----Original Message-----
From: Joel Brobecker [mailto:brobecker@adacore.com] 
Sent: Saturday, December 13, 2014 3:04 PM
To: Keith Seitz
Cc: Nistor Mihail-MNISTOR1; gdb-patches@sourceware.org
Subject: Re: [PATCH v2] Bug 17394: we cannot put a break-point at a global function for ASM file

> gdb/ChangeLog:
> 2014-09-30  Keith Seitz  <keiths@redhat.com>
> 	    Mihail-Marian Nistor  <mihail.nistor@freescale.com>
> 
> 	PR gdb/17394
> 	* linespec.c (struct collect_minsyms): Add new member `symtab'.
> 	(add_minsym): Handle cases where info.symtab is non-NULL.
> 	(search_minsyms_for_name): Add new parameter `symtab'.
> 	Handle limiting searches to a specific symtab.
> 	(add_matching_symtabs_to_info): Search through minimal symbols
> 	for language_asm files for which no new symbols are found.
> 
> gdb/testsuite/ChangeLog:
> 2014-09-30  Mihail-Marian Nistor  <mihail.nistor@freescale.com>
> 
>       PR gdb/17394
>       * gdb.linespec/break-asm-file.c: New file.
>       * gdb.linespec/break-asm-file.exp: New file.
>       * gdb.linespec/break-asm-file0.s: New file.
>       * gdb.linespec/break-asm-file1.s: New file.

Overall, the patch looks good to me. Just a few trivial issues hightlighted below, but the patch is otherwise approved.

> diff --git a/gdb/testsuite/gdb.linespec/break-asm-file.exp 
> b/gdb/testsuite/gdb.linespec/break-asm-file.exp
> +
> +load_lib dwarf.exp

Trailing space here...

> diff --git a/gdb/testsuite/gdb.linespec/break-asm-file0.s 
> b/gdb/testsuite/gdb.linespec/break-asm-file0.s
[...]
> @@ -0,0 +1,218 @@
> +	.byte		1	/* DW_LNS_copy */
> +
> +  .byte		0	/* DW_LNE_set_address */

This file has a few .byte's where the formatting is inconsistent with the rest. Not critical, but would you mind fixing those?


> +	.byte		1	/* DW_LNS_copy */
> +	

Trailing spaces...


> diff --git a/gdb/testsuite/gdb.linespec/break-asm-file1.s 
> b/gdb/testsuite/gdb.linespec/break-asm-file1.s
[...]
> +	.byte		1	/* DW_LNS_copy */
> +	

Trailing spaces again...

> +	.byte		1	/* DW_LNS_copy */
> +	
Ditto...

> +	.4byte		.Lend_func
> +	

... and Ditto.

Thank you,
--
Joel

[-- Attachment #2: 17394_v3.patch --]
[-- Type: application/octet-stream, Size: 20543 bytes --]

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 82384ca..e1bd9b2 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -3448,6 +3448,9 @@ struct collect_minsyms
   /* The objfile we're examining.  */
   struct objfile *objfile;
 
+  /* Only search the given symtab, or NULL to search for all symbols.  */
+  struct symtab *symtab;
+
   /* The funfirstline setting from the initial call.  */
   int funfirstline;
 
@@ -3507,6 +3510,24 @@ add_minsym (struct minimal_symbol *minsym, void *d)
   mo.minsym = minsym;
   mo.objfile = info->objfile;
 
+  if (info->symtab != NULL)
+    {
+      CORE_ADDR pc;
+      struct symtab_and_line sal;
+      struct gdbarch *gdbarch = get_objfile_arch (info->objfile);
+
+      sal = find_pc_sect_line (MSYMBOL_VALUE_ADDRESS (info->objfile, minsym),
+			       NULL, 0);
+      sal.section = MSYMBOL_OBJ_SECTION (info->objfile, minsym);
+      pc
+	= gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc, &current_target);
+      if (pc != sal.pc)
+	sal = find_pc_sect_line (pc, NULL, 0);
+
+      if (info->symtab != sal.symtab)
+	return;
+    }
+
   /* Exclude data symbols when looking for breakpoint locations.   */
   if (!info->list_mode)
     switch (minsym->type)
@@ -3533,40 +3554,59 @@ add_minsym (struct minimal_symbol *minsym, void *d)
   VEC_safe_push (bound_minimal_symbol_d, info->msyms, &mo);
 }
 
-/* Search minimal symbols in all objfiles for NAME.  If SEARCH_PSPACE
+/* Search for minimal symbols called NAME.  If SEARCH_PSPACE
    is not NULL, the search is restricted to just that program
-   space.  */
+   space.
+
+   If SYMTAB is NULL, search all objfiles, otherwise
+   restrict results to the given SYMTAB.  */
 
 static void
 search_minsyms_for_name (struct collect_info *info, const char *name,
-			 struct program_space *search_pspace)
+			 struct program_space *search_pspace,
+			 struct symtab *symtab)
 {
-  struct objfile *objfile;
-  struct program_space *pspace;
+  struct collect_minsyms local;
+  struct cleanup *cleanup;
 
-  ALL_PSPACES (pspace)
-  {
-    struct collect_minsyms local;
-    struct cleanup *cleanup;
+  memset (&local, 0, sizeof (local));
+  local.funfirstline = info->state->funfirstline;
+  local.list_mode = info->state->list_mode;
+  local.symtab = symtab;
 
-    if (search_pspace != NULL && search_pspace != pspace)
-      continue;
-    if (pspace->executing_startup)
-      continue;
+  cleanup = make_cleanup (VEC_cleanup (bound_minimal_symbol_d), &local.msyms);
 
-    set_current_program_space (pspace);
+  if (symtab == NULL)
+    {
+      struct program_space *pspace;
 
-    memset (&local, 0, sizeof (local));
-    local.funfirstline = info->state->funfirstline;
-    local.list_mode = info->state->list_mode;
+      ALL_PSPACES (pspace)
+      {
+	struct objfile *objfile;
 
-    cleanup = make_cleanup (VEC_cleanup (bound_minimal_symbol_d),
-			    &local.msyms);
+	if (search_pspace != NULL && search_pspace != pspace)
+	  continue;
+	if (pspace->executing_startup)
+	  continue;
 
-    ALL_OBJFILES (objfile)
+	set_current_program_space (pspace);
+
+	ALL_OBJFILES (objfile)
+	{
+	  local.objfile = objfile;
+	  iterate_over_minimal_symbols (objfile, name, add_minsym, &local);
+	}
+      }
+    }
+  else
     {
-      local.objfile = objfile;
-      iterate_over_minimal_symbols (objfile, name, add_minsym, &local);
+      if (search_pspace == NULL || SYMTAB_PSPACE (symtab) == search_pspace)
+	{
+	  set_current_program_space (SYMTAB_PSPACE (symtab));
+	  local.objfile = symtab->objfile;
+	  iterate_over_minimal_symbols (local.objfile, name, add_minsym,
+					&local);
+	}
     }
 
     if (!VEC_empty (bound_minimal_symbol_d, local.msyms))
@@ -3599,7 +3639,6 @@ search_minsyms_for_name (struct collect_info *info, const char *name,
       }
 
     do_cleanups (cleanup);
-  }
 }
 
 /* A helper function to add all symbols matching NAME to INFO.  If
@@ -3621,16 +3660,26 @@ add_matching_symbols_to_info (const char *name,
 	  iterate_over_all_matching_symtabs (info->state, name, VAR_DOMAIN,
 					     collect_symbols, info,
 					     pspace, 1);
-	  search_minsyms_for_name (info, name, pspace);
+	  search_minsyms_for_name (info, name, pspace, NULL);
 	}
       else if (pspace == NULL || pspace == SYMTAB_PSPACE (elt))
 	{
+	  int prev_len = VEC_length (symbolp, info->result.symbols);
+
 	  /* Program spaces that are executing startup should have
 	     been filtered out earlier.  */
 	  gdb_assert (!SYMTAB_PSPACE (elt)->executing_startup);
 	  set_current_program_space (SYMTAB_PSPACE (elt));
 	  iterate_over_file_blocks (elt, name, VAR_DOMAIN,
 				    collect_symbols, info);
+
+	  /* If no new symbols were found in this iteration and this symtab
+	     is in assembler, we might actually be looking for a label for
+	     which we don't have debug info.  Check for a minimal symbol in
+	     this case.  */
+	  if (prev_len == VEC_length (symbolp, info->result.symbols)
+	      && elt->language == language_asm)
+	    search_minsyms_for_name (info, name, pspace, elt);
 	}
     }
 }
diff --git a/gdb/testsuite/gdb.linespec/break-asm-file.c b/gdb/testsuite/gdb.linespec/break-asm-file.c
new file mode 100644
index 0000000..525726b
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/break-asm-file.c
@@ -0,0 +1,35 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004-2014 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/>.  */
+
+void func3();
+void func2();
+
+static func()
+{
+}
+
+void func1()
+{
+  func3();
+  func2();
+  func();
+}
+
+int main()
+{
+  func1();
+}
diff --git a/gdb/testsuite/gdb.linespec/break-asm-file.exp b/gdb/testsuite/gdb.linespec/break-asm-file.exp
new file mode 100644
index 0000000..de1ed19
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/break-asm-file.exp
@@ -0,0 +1,55 @@
+# Copyright 2012-2014 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/>.
+
+# Bug 17394
+# Test for break-point at a function only for a selected ASM file.
+
+load_lib dwarf.exp
+
+standard_testfile .c
+set execfile $testfile
+set asm_file1 break-asm-file1.s
+set asm_file0 break-asm-file0.s
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+if {[prepare_for_testing ${testfile}.exp $execfile \
+	 [list $srcfile $asm_file1 $asm_file0] \
+	 {debug nowarnings optimize=-O0}]} {
+    untested "Skipping ${testfile}."
+    return
+}
+
+clean_restart $execfile
+
+gdb_test "break a/$asm_file0:func" \
+    "Breakpoint 1 at 0x\[0-9a-fA-F\]+: file .*a/$asm_file0, line 7\\\." \
+    "set a break-point at a global function only for a selected ASM file."
+
+gdb_test "delete 1"
+
+gdb_test "break b/$asm_file0:func" \
+    "Breakpoint 2 at 0x\[0-9a-fA-F\]+: file .*b/$asm_file0, line 7\\\." \
+    "set a break-point at a function only for a selected ASM file."
+
+gdb_test "delete 2"
+
+gdb_test "break $asm_file0:func" \
+    "Breakpoint 3 at 0x\[0-9a-fA-F\]+: .*$asm_file0.*(2 locations).*" \
+    "set a break-point at function in all instances for a selected ASM file."
+
diff --git a/gdb/testsuite/gdb.linespec/break-asm-file0.s b/gdb/testsuite/gdb.linespec/break-asm-file0.s
new file mode 100644
index 0000000..4d1176c
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/break-asm-file0.s
@@ -0,0 +1,218 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004-2014 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/>.  */
+
+	.file "a/break-asm-file0.s"
+	.text
+.Lbegin_text1:
+	.globl _func2
+_func2:
+	.globl func2
+	.type func2, %function
+func2:
+.Lbegin_func2:
+	.int 0
+	.int 0
+.Lend_func2:
+	.size func2, .-func2
+	.globl _func
+_func:
+  .globl func
+	.type func, %function
+func:
+.Lbegin_func:
+	.file 1 "a/break-asm-file0.s"
+	.int 0
+	.int 0
+.Lend_func:
+	.size func, .-func
+.Lend_text1:
+
+/* Debug information */
+
+	.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	.Lend_text1			/* DW_AT_high_pc */
+	.4byte	.Lbegin_text1			/* DW_AT_low_pc */
+	.ascii	"a/break-asm-file0.s\0"			/* DW_AT_name */
+	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
+	.2byte	0x8001				/* DW_AT_language (Mips Assembler) */
+
+	.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	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x11			/* DW_AT_low_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	0x5			/* DW_FORM_data2 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	2			/* Abbrev code */
+	.uleb128	0x2e			/* DW_TAG_subprogram */
+	.byte		0			/* has_children */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.uleb128	0x3a			/* DW_AT_decl_file */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x3b			/* DW_AT_decl_line */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.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 */
+	.uleb128	0x40			/* DW_AT_frame_base */
+	.uleb128	0xa			/* DW_FORM_block1 */
+	.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 */
+
+	.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		"a/break-asm-file0.s\0"
+	.uleb128	0
+	.uleb128	0
+	.uleb128	0
+
+	.byte		0
+
+.Lline1_lines:
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func2
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 2 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func2+1
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 3 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	4	/* ... to 7 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func+2
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 8 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lend_func
+
+	.byte		0	/* DW_LNE_end_of_sequence */
+	.uleb128	1
+	.byte		1
+
+.Lline1_end:
diff --git a/gdb/testsuite/gdb.linespec/break-asm-file1.s b/gdb/testsuite/gdb.linespec/break-asm-file1.s
new file mode 100644
index 0000000..36bcb86
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/break-asm-file1.s
@@ -0,0 +1,244 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004-2014 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/>.  */
+
+	.text
+.Lbegin_text1:
+	.globl _func3
+_func3:
+	.globl func3
+	.type func3, %function
+func3:
+.Lbegin_func3:
+	.int 0
+	.int 0
+.Lend_func3:
+	.size func3, .-func3
+_func:
+	.type func, %function
+func:
+.Lbegin_func:
+	.int 0
+	.int 0
+.Lend_func:
+	.size func, .-func
+.Lend_text1:
+
+/* Debug information */
+
+	.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	.Lend_text1			/* DW_AT_high_pc */
+	.4byte	.Lbegin_text1			/* DW_AT_low_pc */
+	.ascii	"b/break-asm-file0.s\0"			/* DW_AT_name */
+	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
+	.2byte	0x8001				/* DW_AT_language (Mips Assembler) */
+
+	/* func3 */
+	.uleb128	2			/* Abbrev: DW_TAG_subprogram */
+	.byte		1			/* DW_AT_external */
+	.byte		1			/* DW_AT_decl_file */
+	.byte		2			/* DW_AT_decl_line */
+	.ascii		"func3\0"		/* DW_AT_name */
+	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
+	.4byte		.Lbegin_func3	/* DW_AT_low_pc */
+	.4byte		.Lend_func3		/* DW_AT_high_pc */
+	.byte		1			/* DW_AT_frame_base: length */
+	.byte		0x55			/* DW_AT_frame_base: DW_OP_reg5 */
+
+	/* func */
+	.uleb128	2			/* Abbrev: DW_TAG_subprogram */
+	.byte		0			/* DW_AT_external */
+	.byte		1			/* DW_AT_decl_file */
+	.byte		4			/* DW_AT_decl_line */
+	.ascii		"func\0"		/* DW_AT_name */
+	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
+	.4byte		.Lbegin_func	/* DW_AT_low_pc */
+	.4byte		.Lend_func		/* DW_AT_high_pc */
+	.byte		1			/* DW_AT_frame_base: length */
+	.byte		0x55			/* DW_AT_frame_base: DW_OP_reg5 */
+
+.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	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x11			/* DW_AT_low_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	0x5			/* DW_FORM_data2 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	2			/* Abbrev code */
+	.uleb128	0x2e			/* DW_TAG_subprogram */
+	.byte		0			/* has_children */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.uleb128	0x3a			/* DW_AT_decl_file */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x3b			/* DW_AT_decl_line */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.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 */
+	.uleb128	0x40			/* DW_AT_frame_base */
+	.uleb128	0xa			/* DW_FORM_block1 */
+	.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 */
+
+	.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		"b/break-asm-file0.s\0"
+	.uleb128	0
+	.uleb128	0
+	.uleb128	0
+
+	.byte		0
+
+.Lline1_lines:
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func3
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 2 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func3+1
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 3 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	4	/* ... to 7 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func+1
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 8 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lend_func
+
+	.byte		0	/* DW_LNE_end_of_sequence */
+	.uleb128	1
+	.byte		1
+
+.Lline1_end:

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

* Re: [PATCH v2] Bug 17394: we cannot put a break-point at a global function for ASM file
  2014-12-15 20:47           ` mihail.nistor
@ 2014-12-20 16:33             ` Joel Brobecker
  2014-12-21  9:13               ` mihail.nistor
  0 siblings, 1 reply; 10+ messages in thread
From: Joel Brobecker @ 2014-12-20 16:33 UTC (permalink / raw)
  To: mihail.nistor; +Cc: Keith Seitz, gdb-patches

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

On Mon, Dec 15, 2014 at 08:47:19PM +0000, mihail.nistor@freescale.com wrote:
> Thank you very much for your support.
> 
> You will find enclosed a new patch.
[...]
> > gdb/ChangeLog:
> > 2014-09-30  Keith Seitz  <keiths@redhat.com>
> > 	    Mihail-Marian Nistor  <mihail.nistor@freescale.com>
> > 
> > 	PR gdb/17394
> > 	* linespec.c (struct collect_minsyms): Add new member `symtab'.
> > 	(add_minsym): Handle cases where info.symtab is non-NULL.
> > 	(search_minsyms_for_name): Add new parameter `symtab'.
> > 	Handle limiting searches to a specific symtab.
> > 	(add_matching_symtabs_to_info): Search through minimal symbols
> > 	for language_asm files for which no new symbols are found.
> > 
> > gdb/testsuite/ChangeLog:
> > 2014-09-30  Mihail-Marian Nistor  <mihail.nistor@freescale.com>
> > 
> >       PR gdb/17394
> >       * gdb.linespec/break-asm-file.c: New file.
> >       * gdb.linespec/break-asm-file.exp: New file.
> >       * gdb.linespec/break-asm-file0.s: New file.
> >       * gdb.linespec/break-asm-file1.s: New file.

Thank you. The patch is approved.

I have a few small additional remarks that came up following the
review:

The most important one that I almost tripped over, is the fact
that the patch no longer builds on today's master due to a change
in struct symtab. This goes to show that, when one asks for the patch
to be rebased, we should just go ahead and do not just that, but also
re-test it, even if the file that it touches hasn't changed in the
interim. I've made the required changes and re-done testing this
time around.

In the future, it would really help if you could send the commits
to us, rather than just the diff. What we're interesting in, in
addition to the diff, is the commit's revision log. It should
contain a detailed description of the problem you're trying to
solve, and how you're solving it (usually, we try to put the "why"
in the code). We try to do that at every iteration of the review,
so we can keep an eye on the contents of the revision log and make
sure it's complete and accurate. And another advantage is that
it allows me to push your patch in under 10 seconds rather than
having to apply a diff, make sure I don't miss some changes along
the way, commit, write the revision log, get your email address
(as commit author), etc.

Since there is a PR and testcase, and this patch has been delayed
quite a bit for lack of review, I will take care of creating
the commit this time.

Attached is the commit I just pushed.

-- 
Joel

[-- Attachment #2: 0001-gdb-17394-cannot-put-breakpoint-only-in-selected-ASM.patch --]
[-- Type: text/x-diff, Size: 25841 bytes --]

From 47c70b25c6d116243b21a139fcb957a34863f9e7 Mon Sep 17 00:00:00 2001
From: Mihail-Marian Nistor <mihail.nistor@freescale.com>
Date: Sat, 20 Dec 2014 11:04:44 -0500
Subject: [PATCH] gdb/17394: cannot put breakpoint only in selected ASM file.

This patch fixes a problem when trying to insert a breakpoint on
a specific symbol defined in a specific file, eg:

    break foo.c:func

This currently works for files in C/C++/Ada, etc, but doesn't always
work for Asm files. Analysis of the problem showed that this related
to a limitation in gas, which does not generate debug info for functions/
symbols.  Thus, we have a symtab for the file ("info sources" shows
the file), but it contains no symbols.

When find_linespec_symbols is called in linespec_parse_basic, it calls
find_function_symbols, which uses add_matching_symbols_to_info to
collect all matching symbols.

That function does [pardon any mangled formatting]:

  for (ix = 0; VEC_iterate (symtab_ptr, info->file_symtabs, ix, elt); ++ix)
    {
      if (elt == NULL)
        {
          iterate_over_all_matching_symtabs (info->state, name, VAR_DOMAIN,
                                             collect_symbols, info,
                                             pspace, 1);
          search_minsyms_for_name (info, name, pspace);
        }
      else if (pspace == NULL || pspace == SYMTAB_PSPACE (elt))
        {
          /* Program spaces that are executing startup should have
             been filtered out earlier.  */
          gdb_assert (!SYMTAB_PSPACE (elt)->executing_startup);
          set_current_program_space (SYMTAB_PSPACE (elt));
          iterate_over_file_blocks (elt, name, VAR_DOMAIN,
                                    collect_symbols, info);
        }
    }

This iterates over the symtabs. In the failing use case, ELT is
non-NULL (points to the symtab for the .s file), so it calls
iterate_over_file_blocks. Herein is where the problem exists: it is
assumed that if NAME exists, it must exist in the given symtab -- a
reasonable assumption for "normal" (non-asm) cases. It never searches
minimal symbols (or in the global default symtab).

This patch fixes the problem by doing so. It is important to note that
iterating over minsyms is fairly expensive, so this patch only adds
that extra search if the language is language_asm and
iterate_over_file_blocks returns no symbols.

gdb/ChangeLog:
2014-12-20  Keith Seitz  <keiths@redhat.com>
            Mihail-Marian Nistor  <mihail.nistor@freescale.com>

        PR gdb/17394
        * linespec.c (struct collect_minsyms): Add new member `symtab'.
        (add_minsym): Handle cases where info.symtab is non-NULL.
        (search_minsyms_for_name): Add new parameter `symtab'.
        Handle limiting searches to a specific symtab.
        (add_matching_symtabs_to_info): Search through minimal symbols
        for language_asm files for which no new symbols are found.

gdb/testsuite/ChangeLog:
2014-12-20  Mihail-Marian Nistor  <mihail.nistor@freescale.com>

        PR gdb/17394
        * gdb.linespec/break-asm-file.c: New file.
        * gdb.linespec/break-asm-file.exp: New file.
        * gdb.linespec/break-asm-file0.s: New file.
        * gdb.linespec/break-asm-file1.s: New file.
---
 gdb/ChangeLog                                 |  11 ++
 gdb/linespec.c                                |  97 +++++++---
 gdb/testsuite/ChangeLog                       |   8 +
 gdb/testsuite/gdb.linespec/break-asm-file.c   |  35 ++++
 gdb/testsuite/gdb.linespec/break-asm-file.exp |  55 ++++++
 gdb/testsuite/gdb.linespec/break-asm-file0.s  | 218 +++++++++++++++++++++++
 gdb/testsuite/gdb.linespec/break-asm-file1.s  | 244 ++++++++++++++++++++++++++
 7 files changed, 644 insertions(+), 24 deletions(-)
 create mode 100644 gdb/testsuite/gdb.linespec/break-asm-file.c
 create mode 100644 gdb/testsuite/gdb.linespec/break-asm-file.exp
 create mode 100644 gdb/testsuite/gdb.linespec/break-asm-file0.s
 create mode 100644 gdb/testsuite/gdb.linespec/break-asm-file1.s

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2f34a31..ca7c691 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2014-12-20  Keith Seitz  <keiths@redhat.com>
+	    Mihail-Marian Nistor  <mihail.nistor@freescale.com>
+
+	PR gdb/17394
+	* linespec.c (struct collect_minsyms): Add new member `symtab'.
+	(add_minsym): Handle cases where info.symtab is non-NULL.
+	(search_minsyms_for_name): Add new parameter `symtab'.
+	Handle limiting searches to a specific symtab.
+	(add_matching_symtabs_to_info): Search through minimal symbols
+	for language_asm files for which no new symbols are found.
+
 2014-12-19  Maciej W. Rozycki  <macro@codesourcery.com>
 	    Nigel Stephens  <nigel@mips.com>
 	    Chris Dearman  <chris@mips.com>
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 82384ca..ef4173c 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -3448,6 +3448,9 @@ struct collect_minsyms
   /* The objfile we're examining.  */
   struct objfile *objfile;
 
+  /* Only search the given symtab, or NULL to search for all symbols.  */
+  struct symtab *symtab;
+
   /* The funfirstline setting from the initial call.  */
   int funfirstline;
 
@@ -3507,6 +3510,24 @@ add_minsym (struct minimal_symbol *minsym, void *d)
   mo.minsym = minsym;
   mo.objfile = info->objfile;
 
+  if (info->symtab != NULL)
+    {
+      CORE_ADDR pc;
+      struct symtab_and_line sal;
+      struct gdbarch *gdbarch = get_objfile_arch (info->objfile);
+
+      sal = find_pc_sect_line (MSYMBOL_VALUE_ADDRESS (info->objfile, minsym),
+			       NULL, 0);
+      sal.section = MSYMBOL_OBJ_SECTION (info->objfile, minsym);
+      pc
+	= gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc, &current_target);
+      if (pc != sal.pc)
+	sal = find_pc_sect_line (pc, NULL, 0);
+
+      if (info->symtab != sal.symtab)
+	return;
+    }
+
   /* Exclude data symbols when looking for breakpoint locations.   */
   if (!info->list_mode)
     switch (minsym->type)
@@ -3533,40 +3554,59 @@ add_minsym (struct minimal_symbol *minsym, void *d)
   VEC_safe_push (bound_minimal_symbol_d, info->msyms, &mo);
 }
 
-/* Search minimal symbols in all objfiles for NAME.  If SEARCH_PSPACE
+/* Search for minimal symbols called NAME.  If SEARCH_PSPACE
    is not NULL, the search is restricted to just that program
-   space.  */
+   space.
+
+   If SYMTAB is NULL, search all objfiles, otherwise
+   restrict results to the given SYMTAB.  */
 
 static void
 search_minsyms_for_name (struct collect_info *info, const char *name,
-			 struct program_space *search_pspace)
+			 struct program_space *search_pspace,
+			 struct symtab *symtab)
 {
-  struct objfile *objfile;
-  struct program_space *pspace;
+  struct collect_minsyms local;
+  struct cleanup *cleanup;
 
-  ALL_PSPACES (pspace)
-  {
-    struct collect_minsyms local;
-    struct cleanup *cleanup;
+  memset (&local, 0, sizeof (local));
+  local.funfirstline = info->state->funfirstline;
+  local.list_mode = info->state->list_mode;
+  local.symtab = symtab;
 
-    if (search_pspace != NULL && search_pspace != pspace)
-      continue;
-    if (pspace->executing_startup)
-      continue;
+  cleanup = make_cleanup (VEC_cleanup (bound_minimal_symbol_d), &local.msyms);
 
-    set_current_program_space (pspace);
+  if (symtab == NULL)
+    {
+      struct program_space *pspace;
 
-    memset (&local, 0, sizeof (local));
-    local.funfirstline = info->state->funfirstline;
-    local.list_mode = info->state->list_mode;
+      ALL_PSPACES (pspace)
+      {
+	struct objfile *objfile;
 
-    cleanup = make_cleanup (VEC_cleanup (bound_minimal_symbol_d),
-			    &local.msyms);
+	if (search_pspace != NULL && search_pspace != pspace)
+	  continue;
+	if (pspace->executing_startup)
+	  continue;
 
-    ALL_OBJFILES (objfile)
+	set_current_program_space (pspace);
+
+	ALL_OBJFILES (objfile)
+	{
+	  local.objfile = objfile;
+	  iterate_over_minimal_symbols (objfile, name, add_minsym, &local);
+	}
+      }
+    }
+  else
     {
-      local.objfile = objfile;
-      iterate_over_minimal_symbols (objfile, name, add_minsym, &local);
+      if (search_pspace == NULL || SYMTAB_PSPACE (symtab) == search_pspace)
+	{
+	  set_current_program_space (SYMTAB_PSPACE (symtab));
+	  local.objfile = SYMTAB_OBJFILE(symtab);
+	  iterate_over_minimal_symbols (local.objfile, name, add_minsym,
+					&local);
+	}
     }
 
     if (!VEC_empty (bound_minimal_symbol_d, local.msyms))
@@ -3599,7 +3639,6 @@ search_minsyms_for_name (struct collect_info *info, const char *name,
       }
 
     do_cleanups (cleanup);
-  }
 }
 
 /* A helper function to add all symbols matching NAME to INFO.  If
@@ -3621,16 +3660,26 @@ add_matching_symbols_to_info (const char *name,
 	  iterate_over_all_matching_symtabs (info->state, name, VAR_DOMAIN,
 					     collect_symbols, info,
 					     pspace, 1);
-	  search_minsyms_for_name (info, name, pspace);
+	  search_minsyms_for_name (info, name, pspace, NULL);
 	}
       else if (pspace == NULL || pspace == SYMTAB_PSPACE (elt))
 	{
+	  int prev_len = VEC_length (symbolp, info->result.symbols);
+
 	  /* Program spaces that are executing startup should have
 	     been filtered out earlier.  */
 	  gdb_assert (!SYMTAB_PSPACE (elt)->executing_startup);
 	  set_current_program_space (SYMTAB_PSPACE (elt));
 	  iterate_over_file_blocks (elt, name, VAR_DOMAIN,
 				    collect_symbols, info);
+
+	  /* If no new symbols were found in this iteration and this symtab
+	     is in assembler, we might actually be looking for a label for
+	     which we don't have debug info.  Check for a minimal symbol in
+	     this case.  */
+	  if (prev_len == VEC_length (symbolp, info->result.symbols)
+	      && elt->language == language_asm)
+	    search_minsyms_for_name (info, name, pspace, elt);
 	}
     }
 }
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 518def1..1f08d80 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2014-12-20  Mihail-Marian Nistor  <mihail.nistor@freescale.com>
+
+	PR gdb/17394
+	* gdb.linespec/break-asm-file.c: New file.
+	* gdb.linespec/break-asm-file.exp: New file.
+	* gdb.linespec/break-asm-file0.s: New file.
+	* gdb.linespec/break-asm-file1.s: New file.
+
 2014-12-18  Nigel Stephens  <nigel@mips.com>
 	    Maciej W. Rozycki  <macro@codesourcery.com>
 
diff --git a/gdb/testsuite/gdb.linespec/break-asm-file.c b/gdb/testsuite/gdb.linespec/break-asm-file.c
new file mode 100644
index 0000000..525726b
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/break-asm-file.c
@@ -0,0 +1,35 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004-2014 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/>.  */
+
+void func3();
+void func2();
+
+static func()
+{
+}
+
+void func1()
+{
+  func3();
+  func2();
+  func();
+}
+
+int main()
+{
+  func1();
+}
diff --git a/gdb/testsuite/gdb.linespec/break-asm-file.exp b/gdb/testsuite/gdb.linespec/break-asm-file.exp
new file mode 100644
index 0000000..de1ed19
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/break-asm-file.exp
@@ -0,0 +1,55 @@
+# Copyright 2012-2014 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/>.
+
+# Bug 17394
+# Test for break-point at a function only for a selected ASM file.
+
+load_lib dwarf.exp
+
+standard_testfile .c
+set execfile $testfile
+set asm_file1 break-asm-file1.s
+set asm_file0 break-asm-file0.s
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+if {[prepare_for_testing ${testfile}.exp $execfile \
+	 [list $srcfile $asm_file1 $asm_file0] \
+	 {debug nowarnings optimize=-O0}]} {
+    untested "Skipping ${testfile}."
+    return
+}
+
+clean_restart $execfile
+
+gdb_test "break a/$asm_file0:func" \
+    "Breakpoint 1 at 0x\[0-9a-fA-F\]+: file .*a/$asm_file0, line 7\\\." \
+    "set a break-point at a global function only for a selected ASM file."
+
+gdb_test "delete 1"
+
+gdb_test "break b/$asm_file0:func" \
+    "Breakpoint 2 at 0x\[0-9a-fA-F\]+: file .*b/$asm_file0, line 7\\\." \
+    "set a break-point at a function only for a selected ASM file."
+
+gdb_test "delete 2"
+
+gdb_test "break $asm_file0:func" \
+    "Breakpoint 3 at 0x\[0-9a-fA-F\]+: .*$asm_file0.*(2 locations).*" \
+    "set a break-point at function in all instances for a selected ASM file."
+
diff --git a/gdb/testsuite/gdb.linespec/break-asm-file0.s b/gdb/testsuite/gdb.linespec/break-asm-file0.s
new file mode 100644
index 0000000..4d1176c
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/break-asm-file0.s
@@ -0,0 +1,218 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004-2014 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/>.  */
+
+	.file "a/break-asm-file0.s"
+	.text
+.Lbegin_text1:
+	.globl _func2
+_func2:
+	.globl func2
+	.type func2, %function
+func2:
+.Lbegin_func2:
+	.int 0
+	.int 0
+.Lend_func2:
+	.size func2, .-func2
+	.globl _func
+_func:
+  .globl func
+	.type func, %function
+func:
+.Lbegin_func:
+	.file 1 "a/break-asm-file0.s"
+	.int 0
+	.int 0
+.Lend_func:
+	.size func, .-func
+.Lend_text1:
+
+/* Debug information */
+
+	.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	.Lend_text1			/* DW_AT_high_pc */
+	.4byte	.Lbegin_text1			/* DW_AT_low_pc */
+	.ascii	"a/break-asm-file0.s\0"			/* DW_AT_name */
+	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
+	.2byte	0x8001				/* DW_AT_language (Mips Assembler) */
+
+	.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	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x11			/* DW_AT_low_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	0x5			/* DW_FORM_data2 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	2			/* Abbrev code */
+	.uleb128	0x2e			/* DW_TAG_subprogram */
+	.byte		0			/* has_children */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.uleb128	0x3a			/* DW_AT_decl_file */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x3b			/* DW_AT_decl_line */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.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 */
+	.uleb128	0x40			/* DW_AT_frame_base */
+	.uleb128	0xa			/* DW_FORM_block1 */
+	.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 */
+
+	.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		"a/break-asm-file0.s\0"
+	.uleb128	0
+	.uleb128	0
+	.uleb128	0
+
+	.byte		0
+
+.Lline1_lines:
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func2
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 2 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func2+1
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 3 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	4	/* ... to 7 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func+2
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 8 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lend_func
+
+	.byte		0	/* DW_LNE_end_of_sequence */
+	.uleb128	1
+	.byte		1
+
+.Lline1_end:
diff --git a/gdb/testsuite/gdb.linespec/break-asm-file1.s b/gdb/testsuite/gdb.linespec/break-asm-file1.s
new file mode 100644
index 0000000..36bcb86
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/break-asm-file1.s
@@ -0,0 +1,244 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004-2014 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/>.  */
+
+	.text
+.Lbegin_text1:
+	.globl _func3
+_func3:
+	.globl func3
+	.type func3, %function
+func3:
+.Lbegin_func3:
+	.int 0
+	.int 0
+.Lend_func3:
+	.size func3, .-func3
+_func:
+	.type func, %function
+func:
+.Lbegin_func:
+	.int 0
+	.int 0
+.Lend_func:
+	.size func, .-func
+.Lend_text1:
+
+/* Debug information */
+
+	.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	.Lend_text1			/* DW_AT_high_pc */
+	.4byte	.Lbegin_text1			/* DW_AT_low_pc */
+	.ascii	"b/break-asm-file0.s\0"			/* DW_AT_name */
+	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
+	.2byte	0x8001				/* DW_AT_language (Mips Assembler) */
+
+	/* func3 */
+	.uleb128	2			/* Abbrev: DW_TAG_subprogram */
+	.byte		1			/* DW_AT_external */
+	.byte		1			/* DW_AT_decl_file */
+	.byte		2			/* DW_AT_decl_line */
+	.ascii		"func3\0"		/* DW_AT_name */
+	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
+	.4byte		.Lbegin_func3	/* DW_AT_low_pc */
+	.4byte		.Lend_func3		/* DW_AT_high_pc */
+	.byte		1			/* DW_AT_frame_base: length */
+	.byte		0x55			/* DW_AT_frame_base: DW_OP_reg5 */
+
+	/* func */
+	.uleb128	2			/* Abbrev: DW_TAG_subprogram */
+	.byte		0			/* DW_AT_external */
+	.byte		1			/* DW_AT_decl_file */
+	.byte		4			/* DW_AT_decl_line */
+	.ascii		"func\0"		/* DW_AT_name */
+	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
+	.4byte		.Lbegin_func	/* DW_AT_low_pc */
+	.4byte		.Lend_func		/* DW_AT_high_pc */
+	.byte		1			/* DW_AT_frame_base: length */
+	.byte		0x55			/* DW_AT_frame_base: DW_OP_reg5 */
+
+.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	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x11			/* DW_AT_low_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	0x5			/* DW_FORM_data2 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	2			/* Abbrev code */
+	.uleb128	0x2e			/* DW_TAG_subprogram */
+	.byte		0			/* has_children */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.uleb128	0x3a			/* DW_AT_decl_file */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x3b			/* DW_AT_decl_line */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.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 */
+	.uleb128	0x40			/* DW_AT_frame_base */
+	.uleb128	0xa			/* DW_FORM_block1 */
+	.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 */
+
+	.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		"b/break-asm-file0.s\0"
+	.uleb128	0
+	.uleb128	0
+	.uleb128	0
+
+	.byte		0
+
+.Lline1_lines:
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func3
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 2 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func3+1
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 3 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	4	/* ... to 7 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lbegin_func+1
+
+	.byte		3	/* DW_LNS_advance_line */
+	.sleb128	1	/* ... to 8 */
+
+	.byte		1	/* DW_LNS_copy */
+
+	.byte		0	/* DW_LNE_set_address */
+	.uleb128	5
+	.byte		2
+	.4byte		.Lend_func
+
+	.byte		0	/* DW_LNE_end_of_sequence */
+	.uleb128	1
+	.byte		1
+
+.Lline1_end:
-- 
1.9.1


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

* RE: [PATCH v2] Bug 17394: we cannot put a break-point at a global function for ASM file
  2014-12-20 16:33             ` Joel Brobecker
@ 2014-12-21  9:13               ` mihail.nistor
  0 siblings, 0 replies; 10+ messages in thread
From: mihail.nistor @ 2014-12-21  9:13 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Keith Seitz, gdb-patches

Hello Joel,

Thank you very much for your help and guidance.
In the future I will do as you told me and I hope everything will go smoothly.

Thank you again,
Mihai

-----Original Message-----
From: Joel Brobecker [mailto:brobecker@adacore.com] 
Sent: Saturday, December 20, 2014 6:33 PM
To: Nistor Mihail-MNISTOR1
Cc: Keith Seitz; gdb-patches@sourceware.org
Subject: Re: [PATCH v2] Bug 17394: we cannot put a break-point at a global function for ASM file

On Mon, Dec 15, 2014 at 08:47:19PM +0000, mihail.nistor@freescale.com wrote:
> Thank you very much for your support.
> 
> You will find enclosed a new patch.
[...]
> > gdb/ChangeLog:
> > 2014-09-30  Keith Seitz  <keiths@redhat.com>
> > 	    Mihail-Marian Nistor  <mihail.nistor@freescale.com>
> > 
> > 	PR gdb/17394
> > 	* linespec.c (struct collect_minsyms): Add new member `symtab'.
> > 	(add_minsym): Handle cases where info.symtab is non-NULL.
> > 	(search_minsyms_for_name): Add new parameter `symtab'.
> > 	Handle limiting searches to a specific symtab.
> > 	(add_matching_symtabs_to_info): Search through minimal symbols
> > 	for language_asm files for which no new symbols are found.
> > 
> > gdb/testsuite/ChangeLog:
> > 2014-09-30  Mihail-Marian Nistor  <mihail.nistor@freescale.com>
> > 
> >       PR gdb/17394
> >       * gdb.linespec/break-asm-file.c: New file.
> >       * gdb.linespec/break-asm-file.exp: New file.
> >       * gdb.linespec/break-asm-file0.s: New file.
> >       * gdb.linespec/break-asm-file1.s: New file.

Thank you. The patch is approved.

I have a few small additional remarks that came up following the
review:

The most important one that I almost tripped over, is the fact that the patch no longer builds on today's master due to a change in struct symtab. This goes to show that, when one asks for the patch to be rebased, we should just go ahead and do not just that, but also re-test it, even if the file that it touches hasn't changed in the interim. I've made the required changes and re-done testing this time around.

In the future, it would really help if you could send the commits to us, rather than just the diff. What we're interesting in, in addition to the diff, is the commit's revision log. It should contain a detailed description of the problem you're trying to solve, and how you're solving it (usually, we try to put the "why"
in the code). We try to do that at every iteration of the review, so we can keep an eye on the contents of the revision log and make sure it's complete and accurate. And another advantage is that it allows me to push your patch in under 10 seconds rather than having to apply a diff, make sure I don't miss some changes along the way, commit, write the revision log, get your email address (as commit author), etc.

Since there is a PR and testcase, and this patch has been delayed quite a bit for lack of review, I will take care of creating the commit this time.

Attached is the commit I just pushed.

--
Joel

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

end of thread, other threads:[~2014-12-21  9:13 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-17 11:53 [PATCH v2] Bug 17394: we cannot put a break-point at a global function for ASM file Mihail-Marian Nistor
2014-09-25 18:06 ` Keith Seitz
2014-09-28 15:47   ` mihail.nistor
2014-09-28 16:08     ` mihail.nistor
2014-09-30 19:45       ` Keith Seitz
2014-10-01 18:07         ` mihail.nistor
2014-12-13 13:04         ` Joel Brobecker
2014-12-15 20:47           ` mihail.nistor
2014-12-20 16:33             ` Joel Brobecker
2014-12-21  9:13               ` mihail.nistor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).