Re: RFC: allow breakpoints on labels

[Michael Snyder <msnyder at vmware dot com>]

Tom Tromey wrote:
> This patch fixes PR 8357.
>
> The bug is that while gcc emits dwarf for labels appearing in the user's
> source, there is no way to refer to these labels from gdb.  This patch
> changes the dwarf reader to put labels in the symbol table, and it
> changes linespecs to let the user specify a label in the current
> function.
>
> Built and regtested on x86-64 (compile farm).
>
> I'd appreciate comments.  This also needs a doc review.

Questions:
1) Labels will be in a separate name-space from other symbols (functions)?
2) The search order will find functions first, and only if
there is no conflicting function, find a label?  What if
there's a function and a label of the same name, and user
wants to specify the label?
3) There's no danger of our confusing a label with a
function somewhere downstream, and doing something silly
such as applying prologue_skip or skip_trampoline to a label?


> 2009-08-20  Tom Tromey  <tromey@redhat.com>
>
> 	PR breakpoints/8357:
> 	* symtab.h (domain_enum_tag) <LABEL_DOMAIN>: Update comment.
> 	* linespec.c (decode_line_1): Update comment.  Call decode_label.
> 	(decode_label): New function.
> 	(symbol_found): Handle LOC_LABEL.
> 	* dwarf2read.c (new_symbol) <DW_TAG_label>: Set symbol's type and
> 	domain.  Call add_symbol_to_list.
>
> 2009-08-20  Tom Tromey  <tromey@redhat.com>
>
> 	* gdb.texinfo (Specify Location): Document labels.
>
> 2009-08-20  Tom Tromey  <tromey@redhat.com>
>
> 	* gdb.base/label.exp: New file.
> 	* gdb.base/label.c: New file.
>
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 4016acc..7dcf51c 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -5783,6 +5783,13 @@ in the file @var{filename}.  You only need the file name with a
>  function name to avoid ambiguity when there are identically named
>  functions in different source files.
>  
> +@item @var{label}
> +Specifies the line at which the label named @var{label} appears.
> +@value{GDBN} searches for the label in the function corresponding to
> +the currently selected stack frame.  If there is no current selected
> +stack frame (for instance, if the inferior is not running), then
> +@value{GDBN} will not search for a label.
> +
>  @item *@var{address}
>  Specifies the program address @var{address}.  For line-oriented
>  commands, such as @code{list} and @code{edit}, this specifies a source
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 445bab8..21d6f9b 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -8327,7 +8327,10 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
>  	    {
>  	      SYMBOL_VALUE_ADDRESS (sym) = DW_ADDR (attr) + baseaddr;
>  	    }
> +	  SYMBOL_TYPE (sym) = objfile_type (objfile)->builtin_core_addr;
> +	  SYMBOL_DOMAIN (sym) = LABEL_DOMAIN;
>  	  SYMBOL_CLASS (sym) = LOC_LABEL;
> +	  add_symbol_to_list (sym, cu->list_in_scope);
>  	  break;
>  	case DW_TAG_subprogram:
>  	  /* SYMBOL_BLOCK_VALUE (sym) will be filled in later by
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index 3e943a1..4f7e797 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -123,6 +123,9 @@ static struct symtabs_and_lines decode_dollar (char *copy,
>  					       char ***canonical,
>  					       struct symtab *file_symtab);
>  
> +static int decode_label (char *copy, char ***canonical,
> +			 struct symtabs_and_lines *result);
> +
>  static struct symtabs_and_lines decode_variable (char *copy,
>  						 int funfirstline,
>  						 char ***canonical,
> @@ -640,6 +643,7 @@ See set/show multiple-symbol."));
>     FILE:LINENUM -- that line in that file.  PC returned is 0.
>     FUNCTION -- line number of openbrace of that function.
>     PC returned is the start of the function.
> +   LABEL -- a label in the current scope
>     VARIABLE -- line number of definition of that variable.
>     PC returned is 0.
>     FILE:FUNCTION -- likewise, but prefer functions in that file.
> @@ -873,6 +877,16 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
>      return decode_dollar (copy, funfirstline, default_symtab,
>  			  canonical, file_symtab);
>  
> +  /* Try the token as a label, but only if no file was specified,
> +     because we can only really find labels in the current scope.  */
> +
> +  if (!file_symtab)
> +    {
> +      struct symtabs_and_lines label_result;
> +      if (decode_label (copy, canonical, &label_result))
> +	return label_result;
> +    }
> +
>    /* Look up that token as a variable.
>       If file specified, use that file's per-file block to start with.  */
>  
> @@ -1708,6 +1722,27 @@ decode_dollar (char *copy, int funfirstline, struct symtab *default_symtab,
>  
>  
>  
> +/* A helper for decode_line_1 that tries to find a label.  The label
> +   is searched for in the current block.
> +   COPY is the name of the label to find.
> +   CANONICAL is the same as the "canonical" argument to decode_line_1.
> +   RESULT is a pointer to a symtabs_and_lines structure which will be
> +   filled in on success.
> +   This function returns 1 if a label was found, 0 otherwise.  */
> +
> +static int
> +decode_label (char *copy, char ***canonical, struct symtabs_and_lines *result)
> +{
> +  struct symbol *sym;
> +
> +  sym = lookup_symbol (copy, get_selected_block (0), LABEL_DOMAIN, 0);
> +
> +  if (sym != NULL)
> +    *result = symbol_found (0, canonical, copy, sym, NULL);
> +
> +  return sym != NULL;
> +}
> +
>  /* Decode a linespec that's a variable.  If FILE_SYMTAB is non-NULL,
>     look in that symtab's static variables first.  If NOT_FOUND_PTR is not NULL and
>     the function cannot be found, store boolean true in the location pointed to
> @@ -1787,7 +1822,7 @@ symbol_found (int funfirstline, char ***canonical, char *copy,
>      }
>    else
>      {
> -      if (funfirstline)
> +      if (funfirstline && SYMBOL_CLASS (sym) != LOC_LABEL)
>  	error (_("\"%s\" is not a function"), copy);
>        else if (SYMBOL_LINE (sym) != 0)
>  	{
> diff --git a/gdb/symtab.h b/gdb/symtab.h
> index 740d4e0..d425d6e 100644
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -378,8 +378,7 @@ typedef enum domain_enum_tag
>  
>    STRUCT_DOMAIN,
>  
> -  /* LABEL_DOMAIN may be used for names of labels (for gotos);
> -     currently it is not used and labels are not recorded at all.  */
> +  /* LABEL_DOMAIN may be used for names of labels (for gotos).  */
>  
>    LABEL_DOMAIN,
>  
> diff --git a/gdb/testsuite/gdb.base/label.c b/gdb/testsuite/gdb.base/label.c
> new file mode 100644
> index 0000000..f9cd8c3
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/label.c
> @@ -0,0 +1,21 @@
> +#include <stdio.h>
> +
> +int
> +main (int argc, char *argv)
> +{
> +  int i = 0;
> +  goto there;
> +
> +here:
> +  printf("not here\n");
> +  i = 1;
> +  
> +there:
> +  printf("but here\n");
> +  if (i == 0)
> +    goto here;
> +
> +done:
> +  return 0;
> +}
> +
> diff --git a/gdb/testsuite/gdb.base/label.exp b/gdb/testsuite/gdb.base/label.exp
> new file mode 100644
> index 0000000..9c4532c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/label.exp
> @@ -0,0 +1,66 @@
> +# Copyright 2009 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/>.
> +
> +if $tracelevel then {
> +    strace $tracelevel
> +}
> +
> +
> +if [is_remote target] then {
> +    return 0
> +}
> +
> +
> +#
> +# test running programs
> +#
> +set prms_id 0
> +set bug_id 0
> +
> +set testfile "label"
> +set srcfile ${testfile}.c
> +set binfile ${objdir}/${subdir}/${testfile}
> +
> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug nowarnings}] != "" } {
> +    untested label.exp
> +    return -1
> +}
> +
> +
> +gdb_exit
> +gdb_start
> +gdb_reinitialize_dir $srcdir/$subdir
> +gdb_load ${binfile}
> +
> +if {![runto_main]} {
> +  fail "label tests suppressed"
> +  return -1
> +}
> +
> +gdb_test "break here" \
> +  "Breakpoint.*at.*" \
> +  "breakpoint here"
> +
> +gdb_test "break there" \
> +  "Breakpoint.*at.*" \
> +  "breakpoint there"
> +
> +gdb_test "cont" \
> +  "Breakpoint 3,.*" \
> +  "continue to 'there'"
> +
> +gdb_test "cont" \
> +  "Breakpoint 2,.*" \
> +  "continue to 'here'"