Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Keith Seitz <keiths@redhat.com>
To: Pedro Alves <palves@redhat.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH 1/2] Fix "list ambiguous_variable"
Date: Wed, 06 Sep 2017 18:41:00 -0000	[thread overview]
Message-ID: <ce2d9562-7d1a-1e87-e2cf-abdb18a57fef@redhat.com> (raw)
In-Reply-To: <1504550858-27936-2-git-send-email-palves@redhat.com>

On 09/04/2017 11:47 AM, Pedro Alves wrote:
> The "list" command allows specifying the name of variables as
> argument, not just functions, so that users can type "list
> a_global_variable".
> 
> That support is a broken when it comes to ambiguous locations though.

Very nice!

FWIW, I have only one trivial nit (below).

Keith

PS. Out of curiosity, I hacked up a test program with multiple symbols named "ambiguous," both functions and variables. Well done!

(gdb) set listsize 3
(gdb) list ambiguous
file: "amb1.c", line number: 5, symbol: "ambiguous"
4	
5	static int ambiguous = 0;
6	
file: "amb2.c", line number: 3, symbol: "ambiguous"
2	ambiguous (void)
3	{
4	  return 0;
file: "amb3.c", line number: 3, symbol: "ambiguous"
2	ambiguous (void)
3	{
4	  return 0;
file: "amb4.c", line number: 1, symbol: "ambiguous"
1	static int ambiguous = 0;
2	
3	int


> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index 4801808..d72d19d 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -4318,35 +4318,45 @@ minsym_found (struct linespec_state *self, struct objfile *objfile,
>    CORE_ADDR pc;
>    struct symtab_and_line sal;
>  
> -  sal = find_pc_sect_line (MSYMBOL_VALUE_ADDRESS (objfile, msymbol),
> -			   (struct obj_section *) 0, 0);
> -  sal.section = MSYMBOL_OBJ_SECTION (objfile, msymbol);
> +  if (msymbol_is_text (msymbol))
> +    {
> +      sal = find_pc_sect_line (MSYMBOL_VALUE_ADDRESS (objfile, msymbol),
> +			       (struct obj_section *) 0, 0);
> +      sal.section = MSYMBOL_OBJ_SECTION (objfile, msymbol);
>  
> -  /* The minimal symbol might point to a function descriptor;
> -     resolve it to the actual code address instead.  */
> -  pc = gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc, &current_target);
> -  if (pc != sal.pc)
> -    sal = find_pc_sect_line (pc, NULL, 0);
> +      /* The minimal symbol might point to a function descriptor;
> +	 resolve it to the actual code address instead.  */
> +      pc = gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc, &current_target);

This line exceeds the 80-char line length limit.

> +      if (pc != sal.pc)
> +	sal = find_pc_sect_line (pc, NULL, 0);
>  
> -  if (self->funfirstline)
> -    {
> -      if (sal.symtab != NULL
> -	  && (COMPUNIT_LOCATIONS_VALID (SYMTAB_COMPUNIT (sal.symtab))
> -	      || SYMTAB_LANGUAGE (sal.symtab) == language_asm))
> +      if (self->funfirstline)
>  	{
> -	  /* If gdbarch_convert_from_func_ptr_addr does not apply then
> -	     sal.SECTION, sal.LINE&co. will stay correct from above.
> -	     If gdbarch_convert_from_func_ptr_addr applies then
> -	     sal.SECTION is cleared from above and sal.LINE&co. will
> -	     stay correct from the last find_pc_sect_line above.  */
> -	  sal.pc = MSYMBOL_VALUE_ADDRESS (objfile, msymbol);
> -	  sal.pc = gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc,
> -						       &current_target);
> -	  if (gdbarch_skip_entrypoint_p (gdbarch))
> -	    sal.pc = gdbarch_skip_entrypoint (gdbarch, sal.pc);
> +	  if (sal.symtab != NULL
> +	      && (COMPUNIT_LOCATIONS_VALID (SYMTAB_COMPUNIT (sal.symtab))
> +		  || SYMTAB_LANGUAGE (sal.symtab) == language_asm))
> +	    {
> +	      /* If gdbarch_convert_from_func_ptr_addr does not apply then
> +		 sal.SECTION, sal.LINE&co. will stay correct from above.
> +		 If gdbarch_convert_from_func_ptr_addr applies then
> +		 sal.SECTION is cleared from above and sal.LINE&co. will
> +		 stay correct from the last find_pc_sect_line above.  */
> +	      sal.pc = MSYMBOL_VALUE_ADDRESS (objfile, msymbol);
> +	      sal.pc = gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc,
> +							   &current_target);
> +	      if (gdbarch_skip_entrypoint_p (gdbarch))
> +		sal.pc = gdbarch_skip_entrypoint (gdbarch, sal.pc);
> +	    }
> +	  else
> +	    skip_prologue_sal (&sal);
>  	}
> -      else
> -	skip_prologue_sal (&sal);
> +    }
> +  else
> +    {
> +      sal.objfile = objfile;
> +      sal.pc = MSYMBOL_VALUE_ADDRESS (objfile, msymbol);
> +      sal.pspace = current_program_space;
> +      sal.section = MSYMBOL_OBJ_SECTION (objfile, msymbol);
>      }
>  
>    if (maybe_add_address (self->addr_set, objfile->pspace, sal.pc))


  reply	other threads:[~2017-09-06 18:41 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-04 18:47 [PATCH 0/2] Make "list ambiguous" show symbol names too Pedro Alves
2017-09-04 18:47 ` [PATCH 1/2] Fix "list ambiguous_variable" Pedro Alves
2017-09-06 18:41   ` Keith Seitz [this message]
2017-09-20 15:25     ` Pedro Alves
2017-10-16 15:03       ` Simon Marchi
2017-11-25  7:40         ` ppc64 regression: " Jan Kratochvil
2017-11-26 16:38           ` Ulrich Weigand
2017-11-29 19:08             ` [PATCH] Fix setting-breakpoints regression on PPC64 (function descriptors) (was: Re: ppc64 regression: [PATCH 1/2] Fix "list ambiguous_variable") Pedro Alves
2017-11-29 19:20               ` [PATCH] Fix setting-breakpoints regression on PPC64 (function descriptors) (was: Re: ppc64 regression: [PATCH 1/2] Fix "list amb Ulrich Weigand
2017-11-29 19:28                 ` Pedro Alves
2017-12-08  9:44               ` [PATCH] Fix setting-breakpoints regression on PPC64 (function descriptors) Yao Qi
2017-12-08 11:34                 ` Pedro Alves
2017-12-08 16:57                   ` Yao Qi
2017-09-04 18:47 ` [PATCH 2/2] Make "list ambiguous" show symbol names too Pedro Alves
2017-09-06 18:43   ` Keith Seitz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ce2d9562-7d1a-1e87-e2cf-abdb18a57fef@redhat.com \
    --to=keiths@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox