[RFA] [patch] 'info symbol' to print more info

[RFA] [patch] 'info symbol' to print more info

[Paul Pluzhnikov]

Greetings,

Currently, 'info symbol' and 'maintenance translate-address' print
just the symbol+offset:

(gdb) maint translate-address 0x2aaaac1f5880
exit+0
(gdb) info symbol 0x2aaaac1f5880
exit in section .text

That's nice, but when I have 100s of shared libraries loaded,
I want to know more.

Attached patch results in:

(gdb) maint translate-address 0x2aaaac1f5880
exit+0 section .text in /usr/lib64/libc.so.6
(gdb) info symbol 0x2aaaac1f5880
exit in section .text of /usr/lib64/libc.so.6

Regtested on x86_64 with no failures.

Ok to commit?

Thanks,

--
Paul Pluzhnikov

2008-11-14  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* printcmd.c (sym_info): Print object name.
	* maint.c (maintenance_translate_address): Likewise.
	

Index: gdb/maint.c
===================================================================
RCS file: /cvs/src/src/gdb/maint.c,v
retrieving revision 1.68
diff -u -p -u -r1.68 maint.c
--- gdb/maint.c	30 Oct 2008 20:35:30 -0000	1.68
+++ gdb/maint.c	14 Nov 2008 20:39:48 -0000
@@ -484,9 +484,18 @@ maintenance_translate_address (char *arg
     sym = lookup_minimal_symbol_by_pc (address);
 
   if (sym)
-    printf_filtered ("%s+%s\n",
-		     SYMBOL_PRINT_NAME (sym),
-		     pulongest (address - SYMBOL_VALUE_ADDRESS (sym)));
+    {
+      printf_filtered ("%s+%s",
+		       SYMBOL_PRINT_NAME (sym),
+		       pulongest (address - SYMBOL_VALUE_ADDRESS (sym)));
+      if ((sect = SYMBOL_OBJ_SECTION(sym)))
+	{
+	  printf_filtered (_(" section %s"), sect->the_bfd_section->name);
+	  if (sect->objfile && sect->objfile->name)
+	    printf_filtered (_(" in %s"), sect->objfile->name);
+	}
+      printf_filtered (_("\n"));
+    }
   else if (sect)
     printf_filtered (_("no symbol at %s:0x%s\n"),
 		     sect->the_bfd_section->name, paddr (address));
Index: gdb/printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.136
diff -u -p -u -r1.136 printcmd.c
--- gdb/printcmd.c	13 Nov 2008 22:26:15 -0000	1.136
+++ gdb/printcmd.c	14 Nov 2008 20:39:48 -0000
@@ -1026,6 +1026,8 @@ sym_info (char *arg, int from_tty)
 	  printf_filtered (_("%s overlay "),
 			   section_is_mapped (osect) ? "mapped" : "unmapped");
 	printf_filtered (_("section %s"), osect->the_bfd_section->name);
+	if (osect->objfile && osect->objfile->name)
+	  printf_filtered (_(" of %s"), osect->objfile->name);
 	printf_filtered ("\n");
       }
   }

Re: [RFA] [patch] 'info symbol' to print more info

[Michael Snyder]

Paul Pluzhnikov wrote:
> Greetings,
> 
> Currently, 'info symbol' and 'maintenance translate-address' print
> just the symbol+offset:
> 
> (gdb) maint translate-address 0x2aaaac1f5880
> exit+0
> (gdb) info symbol 0x2aaaac1f5880
> exit in section .text
> 
> That's nice, but when I have 100s of shared libraries loaded,
> I want to know more.
> 
> Attached patch results in:
> 
> (gdb) maint translate-address 0x2aaaac1f5880
> exit+0 section .text in /usr/lib64/libc.so.6
> (gdb) info symbol 0x2aaaac1f5880
> exit in section .text of /usr/lib64/libc.so.6
> 
> Regtested on x86_64 with no failures.
> 
> Ok to commit?

I like it!

Just one suggestion (and suggestion only) -- since the exec_file
is sort of the default/common case, do you think it would be a
good idea to check if it's the exec_file, and omit the objfile
information if so?


> 2008-11-14  Paul Pluzhnikov  <ppluzhnikov@google.com>
> 
> 	* printcmd.c (sym_info): Print object name.
> 	* maint.c (maintenance_translate_address): Likewise.
> 	
> 
> Index: gdb/maint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/maint.c,v
> retrieving revision 1.68
> diff -u -p -u -r1.68 maint.c
> --- gdb/maint.c	30 Oct 2008 20:35:30 -0000	1.68
> +++ gdb/maint.c	14 Nov 2008 20:39:48 -0000
> @@ -484,9 +484,18 @@ maintenance_translate_address (char *arg
>      sym = lookup_minimal_symbol_by_pc (address);
>  
>    if (sym)
> -    printf_filtered ("%s+%s\n",
> -		     SYMBOL_PRINT_NAME (sym),
> -		     pulongest (address - SYMBOL_VALUE_ADDRESS (sym)));
> +    {
> +      printf_filtered ("%s+%s",
> +		       SYMBOL_PRINT_NAME (sym),
> +		       pulongest (address - SYMBOL_VALUE_ADDRESS (sym)));
> +      if ((sect = SYMBOL_OBJ_SECTION(sym)))
> +	{
> +	  printf_filtered (_(" section %s"), sect->the_bfd_section->name);
> +	  if (sect->objfile && sect->objfile->name)
> +	    printf_filtered (_(" in %s"), sect->objfile->name);
> +	}
> +      printf_filtered (_("\n"));
> +    }
>    else if (sect)
>      printf_filtered (_("no symbol at %s:0x%s\n"),
>  		     sect->the_bfd_section->name, paddr (address));
> Index: gdb/printcmd.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/printcmd.c,v
> retrieving revision 1.136
> diff -u -p -u -r1.136 printcmd.c
> --- gdb/printcmd.c	13 Nov 2008 22:26:15 -0000	1.136
> +++ gdb/printcmd.c	14 Nov 2008 20:39:48 -0000
> @@ -1026,6 +1026,8 @@ sym_info (char *arg, int from_tty)
>  	  printf_filtered (_("%s overlay "),
>  			   section_is_mapped (osect) ? "mapped" : "unmapped");
>  	printf_filtered (_("section %s"), osect->the_bfd_section->name);
> +	if (osect->objfile && osect->objfile->name)
> +	  printf_filtered (_(" of %s"), osect->objfile->name);
>  	printf_filtered ("\n");
>        }
>    }

Re: [RFA] [patch] 'info symbol' to print more info

[Paul Pluzhnikov]

On Fri, Nov 14, 2008 at 1:44 PM, Michael Snyder <msnyder@vmware.com> wrote:

> Just one suggestion (and suggestion only) -- since the exec_file
> is sort of the default/common case, do you think it would be a
> good idea to check if it's the exec_file, and omit the objfile
> information if so?

I think there are two large classes of users:
- embedded people with no shared libraries and a single (statically
  linked) executable, and
- everybody else.

I've never been in the first camp. From the second camp, if the
symbol is in the main executable, I want to know that too.

E.g. if I do 'info symbol &malloc', and it tells me .text in a.out,
that may well be a very important clue (that something unusual is
happening).

I do agree that for the fully-static executable case the extra
verbiage may be somewhat annoying. Looking for a way to tell if
that is the case ... Would the test below be sufficient?

  if (ojbect_files->next)
   /* there is only one object, so don't print its name ... */

I also just noticed inconsistency between 'info symbol' and
'maintenance translate-address' wording:

  exit+0 section .text in /usr/lib64/libc.so.6
  exit in section .text of /usr/lib64/libc.so.6

I'll fix that in the final patch so they are the same.

Thanks,
-- 
Paul Pluzhnikov

Re: [RFA] [patch] 'info symbol' to print more info

[Eli Zaretskii]

> Cc: ppluzhnikov@google.com
> Date: Fri, 14 Nov 2008 12:46:17 -0800 (PST)
> From: ppluzhnikov@google.com (Paul Pluzhnikov)
> 
> Attached patch results in:
> 
> (gdb) maint translate-address 0x2aaaac1f5880
> exit+0 section .text in /usr/lib64/libc.so.6
> (gdb) info symbol 0x2aaaac1f5880
> exit in section .text of /usr/lib64/libc.so.6
> 
> Regtested on x86_64 with no failures.
> 
> Ok to commit?

Thanks, that's a nice feature.  Please also fix the places in the
manual where these two commands are described.

Re: [RFA] [patch] 'info symbol' to print more info

[Michael Snyder]

Paul Pluzhnikov wrote:
> On Fri, Nov 14, 2008 at 1:44 PM, Michael Snyder <msnyder@vmware.com> wrote:
> 
>> Just one suggestion (and suggestion only) -- since the exec_file
>> is sort of the default/common case, do you think it would be a
>> good idea to check if it's the exec_file, and omit the objfile
>> information if so?
> 
> I think there are two large classes of users:
> - embedded people with no shared libraries and a single (statically
>   linked) executable, and
> - everybody else.
> 
> I've never been in the first camp. From the second camp, if the
> symbol is in the main executable, I want to know that too.
> 
> E.g. if I do 'info symbol &malloc', and it tells me .text in a.out,
> that may well be a very important clue (that something unusual is
> happening).
> 
> I do agree that for the fully-static executable case the extra
> verbiage may be somewhat annoying. Looking for a way to tell if
> that is the case ... Would the test below be sufficient?
> 
>   if (ojbect_files->next)
>    /* there is only one object, so don't print its name ... */

Yeah, that's suitable.
Thanks.

Re: [RFA] [patch] 'info symbol' to print more info

[Paul Pluzhnikov]

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

On Fri, Nov 14, 2008 at 3:24 PM, Michael Snyder <msnyder@vmware.com> wrote:
> Paul Pluzhnikov wrote:

>>  if (ojbect_files->next)
>>   /* there is only one object, so don't print its name ... */

Obviously I meant the opposite of what I said :(

> Yeah, that's suitable.

A new patch attached.

On Fri, Nov 14, 2008 at 2:54 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> Please also fix the places in the
> manual where these two commands are described.

Done. Ok to commit?

Thanks,
-- 
Paul Pluzhnikov

2008-11-15  Paul Pluzhnikov  <ppluzhnikov@google.com>

        * objfiles.h: New MULTI_OBJFILE_P macro.
        * printcmd.c (sym_info): Print object name.
        * maint.c (maintenance_translate_address): Likewise.


doc/ChangeLog
2008-11-15  Paul Pluzhnikov  <ppluzhnikov@google.com>

        * gdb.texinfo (Symbols): Mention printing containing
	image name in "info symbol".
        (Maint translate-address): Likewise.

[-- Attachment #2: gdb-info-sym-20081115.txt --]
[-- Type: text/plain, Size: 3782 bytes --]

Index: maint.c
===================================================================
RCS file: /cvs/src/src/gdb/maint.c,v
retrieving revision 1.68
diff -u -p -u -r1.68 maint.c
--- maint.c	30 Oct 2008 20:35:30 -0000	1.68
+++ maint.c	15 Nov 2008 15:01:17 -0000
@@ -484,9 +484,19 @@ maintenance_translate_address (char *arg
     sym = lookup_minimal_symbol_by_pc (address);
 
   if (sym)
-    printf_filtered ("%s+%s\n",
-		     SYMBOL_PRINT_NAME (sym),
-		     pulongest (address - SYMBOL_VALUE_ADDRESS (sym)));
+    {
+      printf_filtered ("%s + %s",
+		       SYMBOL_PRINT_NAME (sym),
+		       pulongest (address - SYMBOL_VALUE_ADDRESS (sym)));
+      if ((sect = SYMBOL_OBJ_SECTION(sym)))
+	{
+	  printf_filtered (_(" in section %s"), sect->the_bfd_section->name);
+	  if (MULTI_OBJFILE_P ()
+	      && sect->objfile && sect->objfile->name)
+	    printf_filtered (_(" of %s"), sect->objfile->name);
+	}
+      printf_filtered (_("\n"));
+    }
   else if (sect)
     printf_filtered (_("no symbol at %s:0x%s\n"),
 		     sect->the_bfd_section->name, paddr (address));
Index: objfiles.h
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.h,v
retrieving revision 1.56
diff -u -p -u -r1.56 objfiles.h
--- objfiles.h	1 Oct 2008 17:21:06 -0000	1.56
+++ objfiles.h	15 Nov 2008 15:01:17 -0000
@@ -583,4 +583,8 @@ extern void *objfile_data (struct objfil
    uninitialized section index. */
 #define SECT_OFF_BSS(objfile) (objfile)->sect_index_bss
 
+/* Answer whether there is more than one object file loaded.  */
+
+#define MULTI_OBJFILE_P() (object_files && object_files->next)
+
 #endif /* !defined (OBJFILES_H) */
Index: printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.136
diff -u -p -u -r1.136 printcmd.c
--- printcmd.c	13 Nov 2008 22:26:15 -0000	1.136
+++ printcmd.c	15 Nov 2008 15:01:17 -0000
@@ -1026,6 +1026,9 @@ sym_info (char *arg, int from_tty)
 	  printf_filtered (_("%s overlay "),
 			   section_is_mapped (osect) ? "mapped" : "unmapped");
 	printf_filtered (_("section %s"), osect->the_bfd_section->name);
+	if (MULTI_OBJFILE_P ()
+	    && osect->objfile && osect->objfile->name)
+	  printf_filtered (_(" of %s"), osect->objfile->name);
 	printf_filtered ("\n");
       }
   }
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.532
diff -u -p -u -r1.532 gdb.texinfo
--- doc/gdb.texinfo	24 Oct 2008 21:04:22 -0000	1.532
+++ doc/gdb.texinfo	15 Nov 2008 15:01:18 -0000
@@ -11866,6 +11866,16 @@ _initialize_vx + 396 in section .text
 This is the opposite of the @code{info address} command.  You can use
 it to find out the name of a variable or a function given its address.
 
+For dynamically linked executables, the name of executable or shared
+library containing the symbol is also printed:
+
+@smallexample
+(@value{GDBP}) info symbol 0x400225
+_start + 5 in section .text of /tmp/a.out
+(@value{GDBP}) info symbol 0x2aaaac2811cf
+__read_nocancel + 6 in section .text of /usr/lib64/libc.so.6
+@end smallexample
+
 @kindex whatis
 @item whatis [@var{arg}]
 Print the data type of @var{arg}, which can be either an expression or
@@ -24608,6 +24618,10 @@ the symbol's location to the specified a
 the @code{info address} command (@pxref{Symbols}), except that this
 command also allows to find symbols in other sections.
 
+If section was not specified, the section in which the symbol was found
+is also printed. For dynamically linked executables, the name of
+executable or shared library containing the symbol is printed as well.
+
 @end table
 
 The following command is useful for non-interactive invocations of

Re: [RFA] [patch] 'info symbol' to print more info

[Andreas Schwab]

Paul Pluzhnikov <ppluzhnikov@google.com> writes:

> Index: maint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/maint.c,v
> retrieving revision 1.68
> diff -u -p -u -r1.68 maint.c
> --- maint.c	30 Oct 2008 20:35:30 -0000	1.68
> +++ maint.c	15 Nov 2008 15:01:17 -0000
> @@ -484,9 +484,19 @@ maintenance_translate_address (char *arg
>      sym = lookup_minimal_symbol_by_pc (address);
>  
>    if (sym)
> -    printf_filtered ("%s+%s\n",
> -		     SYMBOL_PRINT_NAME (sym),
> -		     pulongest (address - SYMBOL_VALUE_ADDRESS (sym)));
> +    {
> +      printf_filtered ("%s + %s",
> +		       SYMBOL_PRINT_NAME (sym),
> +		       pulongest (address - SYMBOL_VALUE_ADDRESS (sym)));
> +      if ((sect = SYMBOL_OBJ_SECTION(sym)))

Please move the assignment out of the condition.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [RFA] [patch] 'info symbol' to print more info

[Eli Zaretskii]

> Date: Sat, 15 Nov 2008 08:42:12 -0800
> From: Paul Pluzhnikov <ppluzhnikov@google.com>
> Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
> 
> On Fri, Nov 14, 2008 at 2:54 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > Please also fix the places in the
> > manual where these two commands are described.
> 
> Done. Ok to commit?

Yes, but please fix a minor gotcha I point out below.

> +      printf_filtered ("%s + %s",
> +		       SYMBOL_PRINT_NAME (sym),
> +		       pulongest (address - SYMBOL_VALUE_ADDRESS (sym)));
> +      if ((sect = SYMBOL_OBJ_SECTION(sym)))
> +	{
> +	  printf_filtered (_(" in section %s"), sect->the_bfd_section->name);
> +	  if (MULTI_OBJFILE_P ()
> +	      && sect->objfile && sect->objfile->name)
> +	    printf_filtered (_(" of %s"), sect->objfile->name);
> +	}
> +      printf_filtered (_("\n"));
> +    }

This partition of a phrase into fragments means trouble for
translators.  Not every language can break the sentence

  "foo + NNN in section .text of foobar.o"

into exactly these 3 parts, like you can in English.  In addition,
translating each part without seeing the whole sentence is very
difficult.

So please rewrite this part to not break the sentence, something like
this:

  if ((sect = SYMBOL_OBJ_SECTION (sym))
       && MULTI_OBJFILE_P () && sect->objfile && sect->objfile->name)
    printf_filtered ("%s + %s in section %s of %s\n",
		     SYMBOL_PRINT_NAME (sym),
		     pulongest (address - SYMBOL_VALUE_ADDRESS (sym)),
		     sect->the_bfd_section->name, sect->objfile->name);
  else if ((sect = SYMBOL_OBJ_SECTION (sym))
    printf_filtered ("%s + %s in section %s\n",
		     SYMBOL_PRINT_NAME (sym),
		     pulongest (address - SYMBOL_VALUE_ADDRESS (sym)),
		     sect->the_bfd_section->name);
  else
    printf_filtered ("%s + %s\n",
		     SYMBOL_PRINT_NAME (sym),
		     pulongest (address - SYMBOL_VALUE_ADDRESS (sym)));

>  	  printf_filtered (_("%s overlay "),
>  			   section_is_mapped (osect) ? "mapped" : "unmapped");
>  	printf_filtered (_("section %s"), osect->the_bfd_section->name);
> +	if (MULTI_OBJFILE_P ()
> +	    && osect->objfile && osect->objfile->name)
> +	  printf_filtered (_(" of %s"), osect->objfile->name);
>  	printf_filtered ("\n");

Same here.

> +If section was not specified, the section in which the symbol was found
> +is also printed. For dynamically linked executables, the name of
                  ^^
Two spaces after a period that ends a sentence, please.

Re: [RFA] [patch] 'info symbol' to print more info

[Paul Pluzhnikov]

On Sat, Nov 15, 2008 at 9:39 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> Yes, but please fix a minor gotcha I point out below.
>
>> +      printf_filtered ("%s + %s",
>> +                    SYMBOL_PRINT_NAME (sym),
>> +                    pulongest (address - SYMBOL_VALUE_ADDRESS (sym)));
>> +      if ((sect = SYMBOL_OBJ_SECTION(sym)))
>> +     {
>> +       printf_filtered (_(" in section %s"), sect->the_bfd_section->name);
>> +       if (MULTI_OBJFILE_P ()
>> +           && sect->objfile && sect->objfile->name)
>> +         printf_filtered (_(" of %s"), sect->objfile->name);
>> +     }
>> +      printf_filtered (_("\n"));
>> +    }
>
> This partition of a phrase into fragments means trouble for
> translators.  Not every language can break the sentence
>
>  "foo + NNN in section .text of foobar.o"
>
> into exactly these 3 parts, like you can in English.  In addition,
> translating each part without seeing the whole sentence is very
> difficult.
>
> So please rewrite this part to not break the sentence, something like
> this:
>
>  if ((sect = SYMBOL_OBJ_SECTION (sym))
>       && MULTI_OBJFILE_P () && sect->objfile && sect->objfile->name)
>    printf_filtered ("%s + %s in section %s of %s\n",
>                     SYMBOL_PRINT_NAME (sym),
>                     pulongest (address - SYMBOL_VALUE_ADDRESS (sym)),
>                     sect->the_bfd_section->name, sect->objfile->name);
>  else if ((sect = SYMBOL_OBJ_SECTION (sym))
>    printf_filtered ("%s + %s in section %s\n",
>                     SYMBOL_PRINT_NAME (sym),
>                     pulongest (address - SYMBOL_VALUE_ADDRESS (sym)),
>                     sect->the_bfd_section->name);
>  else
>    printf_filtered ("%s + %s\n",
>                     SYMBOL_PRINT_NAME (sym),
>                     pulongest (address - SYMBOL_VALUE_ADDRESS (sym)));


I did this; there are only 3 if/then/else cases ...

>
>>         printf_filtered (_("%s overlay "),
>>                          section_is_mapped (osect) ? "mapped" : "unmapped");
>>       printf_filtered (_("section %s"), osect->the_bfd_section->name);
>> +     if (MULTI_OBJFILE_P ()
>> +         && osect->objfile && osect->objfile->name)
>> +       printf_filtered (_(" of %s"), osect->objfile->name);
>>       printf_filtered ("\n");
>
> Same here.

But here, the message is already built up from 3 separate conditions,
and I am adding one more. What you are proposing then leads to a
chain of 16 if/then/else clauses. I don't think that's really
appropriate ...

>> +If section was not specified, the section in which the symbol was found
>> +is also printed. For dynamically linked executables, the name of
>                  ^^
> Two spaces after a period that ends a sentence, please.

Done.

Thanks,
-- 
Paul Pluzhnikov

Re: [RFA] [patch] 'info symbol' to print more info

[Joel Brobecker]

> But here, the message is already built up from 3 separate conditions,
> and I am adding one more. What you are proposing then leads to a
> chain of 16 if/then/else clauses. I don't think that's really
> appropriate ...

I haven't looked very closely at the details of the patch since Michael
and Eli already did, so I can't comment on the exact number of if
branches. But, generally speaking, we just don't have much choice if
we want to support i18n well.

That being said, I agree that 16 branches is a large number, and perhaps
we should let go of some of them. For instance, there was this discussion
about not printing the name of the objfile if MULTI_OBJFILE_P. If we
get rid of that, does it reduce the number of cases?  The idea itself
is nice, but perhaps code simplicity is more important in this case.
Are there any other cosmectic features that we can get rid of to
reduce the number of cases further?

-- 
Joel

Re: [RFA] [patch] 'info symbol' to print more info

[Paul Pluzhnikov]

On Sat, Nov 15, 2008 at 11:01 AM, Joel Brobecker <brobecker@adacore.com> wrote:
>
> > But here, the message is already built up from 3 separate conditions,
> > and I am adding one more. What you are proposing then leads to a
> > chain of 16 if/then/else clauses. I don't think that's really
> > appropriate ...
>
> I haven't looked very closely at the details of the patch since Michael
> and Eli already did, so I can't comment on the exact number of if
> branches.

Note that the patch merely adds one more clause to existing three.
What Andreas is asking me to do here is rewrite existing code ...

> But, generally speaking, we just don't have much choice if
> we want to support i18n well.

Maybe 'info symbol' is sufficiently obscure that we don't need to
support i18n well for it?

> That being said, I agree that 16 branches is a large number, and perhaps
> we should let go of some of them.

If we always print symbol offset (even when 0), that eliminates
one branch.

> For instance, there was this discussion
> about not printing the name of the objfile if MULTI_OBJFILE_P. If we
> get rid of that, does it reduce the number of cases?

I think the answer is no, because we still have to check for:

	    && osect->objfile && osect->objfile->name

> The idea itself
> is nice, but perhaps code simplicity is more important in this case.
> Are there any other cosmectic features that we can get rid of to
> reduce the number of cases further?

Except for the offset mentioned above, I don't see what else could
be cut.

Thanks,
--
Paul Pluzhnikov

Re: [RFA] [patch] 'info symbol' to print more info

[Daniel Jacobowitz]

This discussion seems to have wandered far afield from Paul's
reasonable patch :-(

On Mon, Nov 17, 2008 at 11:12:45AM -0800, Paul Pluzhnikov wrote:
> > For instance, there was this discussion
> > about not printing the name of the objfile if MULTI_OBJFILE_P. If we
> > get rid of that, does it reduce the number of cases?
> 
> I think the answer is no, because we still have to check for:
> 
> 	    && osect->objfile && osect->objfile->name

I suspect that can be an assert, or you could use "<unnamed objfile>".
If that helps.  I can't imagine where we'd get an objfile section
without an objfile.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] [patch] 'info symbol' to print more info

[Paul Pluzhnikov]

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

On Mon, Nov 17, 2008 at 11:35 AM, Daniel Jacobowitz <drow@false.org> wrote:
> This discussion seems to have wandered far afield from Paul's
> reasonable patch :-(

So here is another attempt. I did warn it would be ugly :(

-- 
Paul Pluzhnikov


2008-11-17  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* objfiles.h: New MULTI_OBJFILE_P macro.
	* printcmd.c (sym_info): Print object name.
	* maint.c (maintenance_translate_address): Likewise.
	

doc/ChangeLog
2008-11-17  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb.texinfo (Symbols): Mention printing containing
	image name in "info symbol".
	(Maint translate-address): Likewise.
	

testsuite/ChangeLog
2008-11-17  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb.base/sepsymtab.exp: Update for new 'info sym' format.

[-- Attachment #2: gdb-info-sym-20081117.txt --]
[-- Type: text/plain, Size: 7104 bytes --]

Index: maint.c
===================================================================
RCS file: /cvs/src/src/gdb/maint.c,v
retrieving revision 1.68
diff -u -p -u -r1.68 maint.c
--- maint.c	30 Oct 2008 20:35:30 -0000	1.68
+++ maint.c	17 Nov 2008 23:09:50 -0000
@@ -35,6 +35,7 @@
 #include "symfile.h"
 #include "objfiles.h"
 #include "value.h"
+#include "gdb_assert.h"
 
 #include "cli/cli-decode.h"
 
@@ -484,9 +485,32 @@ maintenance_translate_address (char *arg
     sym = lookup_minimal_symbol_by_pc (address);
 
   if (sym)
-    printf_filtered ("%s+%s\n",
-		     SYMBOL_PRINT_NAME (sym),
-		     pulongest (address - SYMBOL_VALUE_ADDRESS (sym)));
+    {
+      const char *symbol_name = SYMBOL_PRINT_NAME (sym);
+      const char *symbol_offset = pulongest (address - SYMBOL_VALUE_ADDRESS (sym));
+
+      sect = SYMBOL_OBJ_SECTION(sym);
+      if (sect != NULL)
+	{
+	  const char *section_name;
+	  const char *obj_name;
+
+	  gdb_assert (sect->the_bfd_section && sect->the_bfd_section->name);
+	  section_name = sect->the_bfd_section->name;
+
+	  gdb_assert (sect->objfile && sect->objfile->name);
+	  obj_name = sect->objfile->name;
+
+	  if (MULTI_OBJFILE_P ())
+	    printf_filtered (_("%s + %s in section %s of %s\n"),
+			     symbol_name, symbol_offset, section_name, obj_name);
+	  else
+	    printf_filtered (_("%s + %s in section %s\n"),
+			     symbol_name, symbol_offset, section_name);
+	}
+      else
+	printf_filtered (_("%s + %s\n"), symbol_name, symbol_offset);
+    }
   else if (sect)
     printf_filtered (_("no symbol at %s:0x%s\n"),
 		     sect->the_bfd_section->name, paddr (address));
Index: objfiles.h
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.h,v
retrieving revision 1.56
diff -u -p -u -r1.56 objfiles.h
--- objfiles.h	1 Oct 2008 17:21:06 -0000	1.56
+++ objfiles.h	17 Nov 2008 23:09:50 -0000
@@ -583,4 +583,8 @@ extern void *objfile_data (struct objfil
    uninitialized section index. */
 #define SECT_OFF_BSS(objfile) (objfile)->sect_index_bss
 
+/* Answer whether there is more than one object file loaded.  */
+
+#define MULTI_OBJFILE_P() (object_files && object_files->next)
+
 #endif /* !defined (OBJFILES_H) */
Index: printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.136
diff -u -p -u -r1.136 printcmd.c
--- printcmd.c	13 Nov 2008 22:26:15 -0000	1.136
+++ printcmd.c	17 Nov 2008 23:09:50 -0000
@@ -1012,21 +1012,51 @@ sym_info (char *arg, int from_tty)
 	&& sect_addr < obj_section_endaddr (osect)
 	&& (msymbol = lookup_minimal_symbol_by_pc_section (sect_addr, osect)))
       {
+	const char *obj_name, *mapped, *sec_name, *msym_name;
+
 	matches = 1;
 	offset = sect_addr - SYMBOL_VALUE_ADDRESS (msymbol);
-	if (offset)
-	  printf_filtered ("%s + %u in ",
-			   SYMBOL_PRINT_NAME (msymbol), offset);
+	mapped = section_is_mapped (osect) ? "mapped" : "unmapped";
+	sec_name = osect->the_bfd_section->name;
+	msym_name = SYMBOL_PRINT_NAME (msymbol);
+
+	gdb_assert (osect->objfile && osect->objfile->name);
+	obj_name = osect->objfile->name;
+
+	if (MULTI_OBJFILE_P ())
+	  if (pc_in_unmapped_range (addr, osect))
+	    if (section_is_overlay (osect))
+	      printf_filtered (_("%s + %u in load address range of "
+				 "%s overlay section %s of %s\n"),
+			       msym_name, offset,
+			       mapped, sec_name, obj_name);
+	    else
+	      printf_filtered (_("%s + %u in load address range of "
+				 "section %s of %s\n"),
+			       msym_name, offset, sec_name, obj_name);
+	  else
+	    if (section_is_overlay (osect))
+	      printf_filtered (_("%s + %u in %s overlay section %s of %s\n"),
+			       msym_name, offset, mapped, sec_name, obj_name);
+	    else
+	      printf_filtered (_("%s + %u in section %s of %s\n"),
+			       msym_name, offset, sec_name, obj_name);
 	else
-	  printf_filtered ("%s in ",
-			   SYMBOL_PRINT_NAME (msymbol));
-	if (pc_in_unmapped_range (addr, osect))
-	  printf_filtered (_("load address range of "));
-	if (section_is_overlay (osect))
-	  printf_filtered (_("%s overlay "),
-			   section_is_mapped (osect) ? "mapped" : "unmapped");
-	printf_filtered (_("section %s"), osect->the_bfd_section->name);
-	printf_filtered ("\n");
+	  if (pc_in_unmapped_range (addr, osect))
+	    if (section_is_overlay (osect))
+	      printf_filtered (_("%s + %u in load address range of %s overlay "
+				 "section %s\n"),
+			       msym_name, offset, mapped, sec_name);
+	    else
+	      printf_filtered (_("%s + %u in load address range of section %s\n"),
+			       msym_name, offset, sec_name);
+	  else
+	    if (section_is_overlay (osect))
+	      printf_filtered (_("%s + %u in %s overlay section %s\n"),
+			       msym_name, offset, mapped, sec_name);
+	    else
+	      printf_filtered (_("%s + %u in section %s\n"),
+			       msym_name, offset, sec_name);
       }
   }
   if (matches == 0)
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.533
diff -u -p -u -r1.533 gdb.texinfo
--- doc/gdb.texinfo	17 Nov 2008 16:43:33 -0000	1.533
+++ doc/gdb.texinfo	17 Nov 2008 23:09:51 -0000
@@ -11866,6 +11866,16 @@ _initialize_vx + 396 in section .text
 This is the opposite of the @code{info address} command.  You can use
 it to find out the name of a variable or a function given its address.
 
+For dynamically linked executables, the name of executable or shared
+library containing the symbol is also printed:
+
+@smallexample
+(@value{GDBP}) info symbol 0x400225
+_start + 5 in section .text of /tmp/a.out
+(@value{GDBP}) info symbol 0x2aaaac2811cf
+__read_nocancel + 6 in section .text of /usr/lib64/libc.so.6
+@end smallexample
+
 @kindex whatis
 @item whatis [@var{arg}]
 Print the data type of @var{arg}, which can be either an expression or
@@ -24621,6 +24631,10 @@ the symbol's location to the specified a
 the @code{info address} command (@pxref{Symbols}), except that this
 command also allows to find symbols in other sections.
 
+If section was not specified, the section in which the symbol was found
+is also printed.  For dynamically linked executables, the name of
+executable or shared library containing the symbol is printed as well.
+
 @end table
 
 The following command is useful for non-interactive invocations of
Index: testsuite/gdb.base/sepsymtab.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/sepsymtab.exp,v
retrieving revision 1.5
diff -u -p -u -r1.5 sepsymtab.exp
--- testsuite/gdb.base/sepsymtab.exp	1 Jan 2008 22:53:19 -0000	1.5
+++ testsuite/gdb.base/sepsymtab.exp	17 Nov 2008 23:33:07 -0000
@@ -45,7 +45,7 @@ gdb_load ${binfile}
 set command "info sym main"
 set command_regex [string_to_regexp $command]
 gdb_test_multiple "$command" "$command" {
-    -re "^${command_regex}\[\r\n\]+main in section \[^\r\n\]+\[\r\n\]+$gdb_prompt \$" {
+    -re "^${command_regex}\[\r\n\]+main \\+ 0 in section \[^\r\n\]+\[\r\n\]+$gdb_prompt \$" {
 	pass "$command"
     }
 }

Re: [RFA] [patch] 'info symbol' to print more info

[Eli Zaretskii]

> Date: Mon, 17 Nov 2008 15:36:16 -0800
> From: Paul Pluzhnikov <ppluzhnikov@google.com>
> 
> So here is another attempt. I did warn it would be ugly :(

Thanks.  I see nothing ugly in it, and I'm fine with this code.

> 2008-11-17  Paul Pluzhnikov  <ppluzhnikov@google.com>
> 
> 	* gdb.texinfo (Symbols): Mention printing containing
> 	image name in "info symbol".
> 	(Maint translate-address): Likewise.

This part is approved.

Re: [RFA] [patch] 'info symbol' to print more info

[Joel Brobecker]

> 2008-11-17  Paul Pluzhnikov  <ppluzhnikov@google.com>
> 
> 	* objfiles.h: New MULTI_OBJFILE_P macro.
> 	* printcmd.c (sym_info): Print object name.
> 	* maint.c (maintenance_translate_address): Likewise.

Just one comment.

> testsuite/ChangeLog
> 2008-11-17  Paul Pluzhnikov  <ppluzhnikov@google.com>
> 
> 	* gdb.base/sepsymtab.exp: Update for new 'info sym' format.

This part looks ok to me.

> +	mapped = section_is_mapped (osect) ? "mapped" : "unmapped";

This part needs to be marked for translation as well.

> +	if (MULTI_OBJFILE_P ())
> +	  if (pc_in_unmapped_range (addr, osect))
> +	    if (section_is_overlay (osect))
> +	      printf_filtered (_("%s + %u in load address range of "
> +				 "%s overlay section %s of %s\n"),
> +			       msym_name, offset,
> +			       mapped, sec_name, obj_name);

I just want to make sure that others would agree that the "%s overlay
section" would not cause any problem for the translators. I tried to
use French as an example, and although my translation is horrible
because I don't know the French terms for these technical concepts,
I think it still showed me that it worked:

  %s + %u par rapport au bloc d'addresse de la section overlay %s dans %s

where "mapped" would have been translated to "mappée".

Anyway, as far as I can tell, as long as "mapped" and "unmapped"
are marked for translation, the patch looks good to me.

-- 
Joel

Re: [RFA] [patch] 'info symbol' to print more info

[Paul Pluzhnikov]

On Tue, Nov 18, 2008 at 1:01 PM, Joel Brobecker <brobecker@adacore.com> wrote:

> Anyway, as far as I can tell, as long as "mapped" and "unmapped"
> are marked for translation, the patch looks good to me.

Done and checked in.

Thanks,
-- 
Paul Pluzhnikov