[PATCH] Revised display-linkage-name
Keith Seitz
keiths@redhat.com
Mon Jul 22 17:48:00 GMT 2013
On 07/17/2013 11:51 AM, Michael Eager wrote:
> BTW, "prepend" appears in several places in the docs. I replaced it this
> patch with prefix. If there is a concern that "prepend" sounds like
> jargon, then the other uses should be changed as well.
I will audit the manual. Maybe that's just my pedantic side showing through.
> diff --git a/gdb/annotate.c b/gdb/annotate.c
> index ccba5fe..84edeac 100644
> --- a/gdb/annotate.c
> +++ b/gdb/annotate.c
> @@ -582,6 +582,13 @@ breakpoint_changed (struct breakpoint *b)
> }
>
> void
> +annotate_linkage_name (void)
> +{
> + if (annotation_level == 2)
> + printf_filtered (("\n\032\032linkage_name\n"));
> +}
> +
This is still missing a (trivial) comment. [IIRC, we require comments
for *all* functions, even if they are pretty trivial.]
> +void
> _initialize_annotate (void)
> {
> observer_attach_breakpoint_created (breakpoint_changed);
> diff --git a/gdb/annotate.h b/gdb/annotate.h
> index 72c4f19..33a9a0e 100644
> --- a/gdb/annotate.h
> +++ b/gdb/annotate.h
> @@ -98,5 +98,7 @@ extern void annotate_elt_rep_end (void);
> extern void annotate_elt (void);
> extern void annotate_array_section_end (void);
>
> +extern void annotate_linkage_name (void);
> +
> extern void (*deprecated_annotate_signalled_hook) (void);
> extern void (*deprecated_annotate_signal_hook) (void);
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 4d09b30..e40af59 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -5704,6 +5704,34 @@ print_breakpoint_location (struct breakpoint *b,
> ui_out_text (uiout, "in ");
> ui_out_field_string (uiout, "func",
> SYMBOL_PRINT_NAME (sym));
> + if (display_linkage_name)
I believe we are now asking all these types of checks to be explicit "!=
NULL".
> + {
> + struct bound_minimal_symbol msymbol =
Can you double-check this? GIT is showing me that there is extra
whitespace at the end of the (above) line.
> + lookup_minimal_symbol_by_pc (loc->address);
Need a blank line here.
> + if (msymbol.minsym != NULL
> + && strcmp (SYMBOL_LINKAGE_NAME (msymbol.minsym),
> + SYMBOL_PRINT_NAME (sym)) != 0)
> + {
> + ui_out_text (uiout, " [");
> +
> + if (strlen (SYMBOL_LINKAGE_NAME (msymbol.minsym))
> + > display_linkage_name_len)
This indentation looks off. Can you double-check this?
> + {
> + char *lname = alloca (display_linkage_name_len + 4);
> +
> + strncpy (lname, SYMBOL_LINKAGE_NAME (msymbol.minsym),
> + display_linkage_name_len);
> + lname[display_linkage_name_len] = '\0';
> + strcat (lname, "...");
> + ui_out_field_string (uiout, "linkage-name", lname);
> + }
> + else
> + ui_out_field_string (uiout, "linkage-name",
> + SYMBOL_LINKAGE_NAME (msymbol.minsym));
> +
From gdbint.info: "Any two or more lines in code should be wrapped in
braces [...]". See '(gdbint.info)Coding Standards'.
> + ui_out_text (uiout, "]");
> + }
> + }
> ui_out_text (uiout, " ");
> ui_out_wrap_hint (uiout, wrap_indent_at_field (uiout, "what"));
> ui_out_text (uiout, "at ");
> diff --git a/gdb/defs.h b/gdb/defs.h
> index 014d7d4..0d10a0d 100644
> --- a/gdb/defs.h
> +++ b/gdb/defs.h
> @@ -336,6 +336,7 @@ extern int build_address_symbolic (struct gdbarch *,
> CORE_ADDR addr,
> int do_demangle,
> char **name,
> + char **linkname,
GIT is showing extra whitespace at the end of this line, too.
> int *offset,
> char **filename,
> int *line,
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index fae54e4..4f48c4c 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -15933,8 +15933,31 @@ line 1574.
> @}
> (@value{GDBP})
> @end smallexample
> -@end table
>
> +@kindex set display-linkage-name
> +@cindex list linkage names
> +@item set display-linkage-name
> +@itemx show display-linkage-name
> +Control displaying linker symbol names for functions.
> +
> +The default is @code{off}, which means that @value{GDBN} will only
> +display the function name used in the source. When @code{on}, @value{GDBN}
> +will also display the linkage name for the symbol name within brackets if it is
> +different from the name in the source.
There appears to be some more whitespace at the end of this line.
> +
> +The linkage name is the name used by the linker for a symbol. This may be
> +the same as the symbol's name in the source, or may be different if the compiler
> +adds a prefix to the name (for example, an underscore) or modifies it, such
> +as by C@t{++} name mangling.
> +
> +This is different from "set print asm-demangle on" which only displays
> +the linkage name for C@t{++} symbols and does not display the source name.
> +
> +@kindex show display-linkage-name-len
> +@itemx show display-linkage-name-len @var{len}
> +Set the maximum number of characters of linkage name to display. The
> +@code{show} command displays the current setting. The default is @code{20}.
> +@end table
>
> @node Altering
> @chapter Altering Execution
> @@ -29250,6 +29273,9 @@ is not.
> @item what
> Some extra data, the exact contents of which are type-dependent.
>
> +@item linkage-name
> +The name used by the linker for this function.
> +
> @end table
>
> For example, here is what the output of @code{-break-insert}
> diff --git a/gdb/printcmd.c b/gdb/printcmd.c
> index 99d4dba..e3b8c21 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -51,6 +51,7 @@
> #include "cli/cli-utils.h"
> #include "format.h"
> #include "source.h"
> +#include "top.h"
>
> #ifdef TUI
> #include "tui/tui.h" /* For tui_active et al. */
> @@ -569,17 +570,19 @@ print_address_symbolic (struct gdbarch *gdbarch, CORE_ADDR addr,
> int do_demangle, char *leadin)
> {
> char *name = NULL;
> + char *linkname = NULL;
> char *filename = NULL;
> int unmapped = 0;
> int offset = 0;
> int line = 0;
>
> - /* Throw away both name and filename. */
> + /* Throw away name, linkname, and filename. */
> struct cleanup *cleanup_chain = make_cleanup (free_current_contents, &name);
> make_cleanup (free_current_contents, &filename);
> + make_cleanup (free_current_contents, &linkname);
>
> - if (build_address_symbolic (gdbarch, addr, do_demangle, &name, &offset,
> - &filename, &line, &unmapped))
> + if (build_address_symbolic (gdbarch, addr, do_demangle, &name, &linkname,
> + &offset, &filename, &line, &unmapped))
> {
> do_cleanups (cleanup_chain);
> return 0;
> @@ -591,6 +594,28 @@ print_address_symbolic (struct gdbarch *gdbarch, CORE_ADDR addr,
> else
> fputs_filtered ("<", stream);
> fputs_filtered (name, stream);
> +
> + /* Print linkage name after source name if requested and different. */
> + if (display_linkage_name
> + && linkname != NULL && strcmp (name, linkname) != 0)
> + {
> + fputs_filtered (" [", stream);
> +
> + if (strlen (linkname) > display_linkage_name_len)
> + {
> + char *lname = alloca (display_linkage_name_len + 4);
> +
> + strncpy (lname, linkname, display_linkage_name_len);
> + lname[display_linkage_name_len] = '\0';
> + strcat (lname, "...");
> + fputs_filtered (lname, stream);
> + }
> + else
> + fputs_filtered (linkname, stream);
> +
> + fputs_filtered ("]", stream);
> + }
> +
> if (offset != 0)
> fprintf_filtered (stream, "+%u", (unsigned int) offset);
>
> @@ -623,6 +648,7 @@ build_address_symbolic (struct gdbarch *gdbarch,
> CORE_ADDR addr, /* IN */
> int do_demangle, /* IN */
> char **name, /* OUT */
> + char **linkname, /* OUT */
> int *offset, /* OUT */
> char **filename, /* OUT */
> int *line, /* OUT */
> @@ -637,6 +663,10 @@ build_address_symbolic (struct gdbarch *gdbarch,
> /* Let's say it is mapped (not unmapped). */
> *unmapped = 0;
>
> + /* Let's say the link name is the same as the symbol name. */
> + if (linkname)
"!= NULL" and there appears to be extra whitespace at the end of this line.
> + *linkname = 0;
> +
s/0/NULL/g . From '(gdbint.info)Coding Standards': "Zero constant (0) is
not interchangeable with a null pointer constant (NULL) anywhere."
> /* Determine if the address is in an overlay, and whether it is
> mapped. */
> if (overlay_debugging)
> @@ -740,6 +770,13 @@ build_address_symbolic (struct gdbarch *gdbarch,
> *line = sal.line;
> }
> }
> +
> + /* If we have both symbol names and they are different, let caller know. */
> + if (msymbol != NULL && symbol != NULL
> + && strcmp (SYMBOL_LINKAGE_NAME (msymbol), SYMBOL_LINKAGE_NAME (symbol)) != 0)
> + if (linkname)
> + *linkname = xstrdup (SYMBOL_LINKAGE_NAME (msymbol));
The "strcmp..." line exceeds 80 columns.
This reads oddly. I think it would be better to merge the two tests,
adding the "linkname != NULL" before the strcmp:
if (linkname != NULL && msymbol != NULL && symbol != NULL
&& strcmp ...)
There's little point to doing the strcmp if the linkage name is not
required.
> +
> return 0;
> }
>
> diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
> index f960b08..6b48140 100644
> --- a/gdb/python/py-frame.c
> +++ b/gdb/python/py-frame.c
> @@ -131,7 +131,7 @@ frapy_name (PyObject *self, PyObject *args)
> {
> FRAPY_REQUIRE_VALID (self, frame);
>
> - find_frame_funname (frame, &name, &lang, NULL);
> + find_frame_funname (frame, &name, NULL, &lang, NULL);
> }
>
> if (except.reason < 0)
> diff --git a/gdb/stack.c b/gdb/stack.c
> index 313d57f..4bc8824 100644
> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -55,6 +55,7 @@
> #include "psymtab.h"
> #include "symfile.h"
> #include "python/python.h"
> +#include "top.h"
>
> void (*deprecated_selected_frame_level_changed_hook) (int);
>
> @@ -1000,15 +1001,19 @@ get_last_displayed_sal (struct symtab_and_line *sal)
>
>
> /* Attempt to obtain the FUNNAME, FUNLANG and optionally FUNCP of the function
> - corresponding to FRAME. FUNNAME needs to be freed by the caller. */
> + corresponding to FRAME. FUNNAME needs to be freed by the caller.
> + Set LINKNAME to linkage name (symbol used by linker) if LINKNAME is non-null
> + and linkage name differs from source name. */
>
> void
> find_frame_funname (struct frame_info *frame, char **funname,
> - enum language *funlang, struct symbol **funcp)
> + const char **linkname, enum language *funlang,
> + struct symbol **funcp)
> {
> struct symbol *func;
>
> *funname = NULL;
> + *linkname = NULL;
linkname could be NULL; *linkname would be bad. :-)
> *funlang = language_unknown;
> if (funcp)
> *funcp = NULL;
> @@ -1045,6 +1050,12 @@ find_frame_funname (struct frame_info *frame, char **funname,
> memset (&msymbol, 0, sizeof (msymbol));
>
> if (msymbol.minsym != NULL
> + && strcmp (SYMBOL_LINKAGE_NAME (msymbol.minsym),
> + SYMBOL_LINKAGE_NAME (func)) != 0)
> + if (linkname)
> + *linkname = SYMBOL_LINKAGE_NAME (msymbol.minsym);
> +
Same comment about merging these two tests.
> + if (msymbol.minsym != NULL
> && (SYMBOL_VALUE_ADDRESS (msymbol.minsym)
> > BLOCK_START (SYMBOL_BLOCK_VALUE (func))))
> {
> @@ -1102,6 +1113,7 @@ print_frame (struct frame_info *frame, int print_level,
> struct gdbarch *gdbarch = get_frame_arch (frame);
> struct ui_out *uiout = current_uiout;
> char *funname = NULL;
> + const char *linkname = NULL;
> enum language funlang = language_unknown;
> struct ui_file *stb;
> struct cleanup *old_chain, *list_chain;
> @@ -1115,7 +1127,7 @@ print_frame (struct frame_info *frame, int print_level,
> stb = mem_fileopen ();
> old_chain = make_cleanup_ui_file_delete (stb);
>
> - find_frame_funname (frame, &funname, &funlang, &func);
> + find_frame_funname (frame, &funname, &linkname, &funlang, &func);
> make_cleanup (xfree, funname);
>
> annotate_frame_begin (print_level ? frame_relative_level (frame) : 0,
> @@ -1147,9 +1159,33 @@ print_frame (struct frame_info *frame, int print_level,
> fprintf_symbol_filtered (stb, funname ? funname : "??",
> funlang, DMGL_ANSI);
> ui_out_field_stream (uiout, "func", stb);
> +
> + /* Print linkage name after source name if requested and different. */
> + if ((display_linkage_name || ui_out_is_mi_like_p (uiout))
> + && linkname != NULL && strcmp (funname, linkname) != 0)
> + {
> + annotate_linkage_name ();
> + ui_out_text (uiout, " [");
> +
> + if (strlen (linkname) > display_linkage_name_len)
> + {
> + char *lname = alloca (display_linkage_name_len + 4);
> +
> + strncpy (lname, linkname, display_linkage_name_len);
> + lname[display_linkage_name_len] = '\0';
> + strcat (lname, "...");
> + ui_out_text (uiout, lname);
> + }
> + else
> + ui_out_text (uiout, linkname);
> +
> + ui_out_text (uiout, "]");
> + ui_out_field_stream (uiout, "linkage_name", stb);
> + }
> +
> ui_out_wrap_hint (uiout, " ");
> annotate_frame_args ();
> -
> +
More whitespace here.
> ui_out_text (uiout, " (");
> if (print_args)
> {
> diff --git a/gdb/testsuite/gdb.cp/display-linkage-name.exp b/gdb/testsuite/gdb.cp/display-linkage-name.exp
> new file mode 100644
> index 0000000..6cde8af
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/display-linkage-name.exp
> @@ -0,0 +1,129 @@
> +# Copyright 2013 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see<http://www.gnu.org/licenses/>.
> +
> +# This file was written by Michael Eager (eager@eagercon.com).
> +
> +if { [skip_cplus_tests] } { continue }
> +
> +standard_testfile .cc
> +
> +if {[get_compiler_info "c++"]} {
> + return -1
> +}
> +
> +if { [prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}] } {
> + return -1
> +}
> +
> +# set break at functions
> +
> +set bp1 [gdb_get_line_number "set breakpoint 1 here"]
> +set bp2 [gdb_get_line_number "set breakpoint 2 here"]
> +
> +if {![gdb_breakpoint "foo"]} {
> + fail "set breakpoint foo"
> +}
> +
> +if {![gdb_breakpoint "fun_with_a_long_name"]} {
> + fail "set breakpoint fun_with_a_long_name"
> +}
> +
> +########################
> +# Test with display-linkage-name off
> +
> +gdb_test_no_output "set display-linkage-name off" ""
> +
> +gdb_test "info break" \
> + "Num Type\[ \]+Disp Enb Address\[\t \]+What.*
> +1\[\t \]+breakpoint keep y.* in foo(.*) at .*$srcfile:$bp1.*
> +2\[\t \]+breakpoint keep y.* in fun_with_a_long_name(.*) at .*$srcfile:$bp2.*" \
> + "info breakpoint - display off"
> +
> +gdb_run_cmd
> +set test "break at fun_with_a_long_name"
There's extra whitespace at the end of this line.
> +gdb_test_multiple "continue" $test {
> + -re "Breakpoint 2, fun_with_a_long_name (.*) at.*$srcfile.$bp2.*$gdb_prompt $" {
> + pass $test
> + break
> + }
> +}
You can use gdb_continue_to_breakpoint to simplify the above.
[gdb_continue_to_breakpoint takes an optional location pattern argument.]
> +set bttable "#0 foo (.*) at.*\[\r\n\]"
> +append bttable "#1 $hex in fun_with_a_long_name (.*) at .*$srcfile:.*\[\r\n\]"
> +append bttable "#2 $hex in goo (.*) at .*$srcfile:.*\[\r\n\]"
> +append bttable "#3 $hex in main (.*) at .*$srcfile:.*"
> +
> +gdb_test "backtrace" $bttable "backtrace - display off"
> +
> +########################
> +# Test with display-linkage-name on
> +
> +gdb_test_no_output "set display-linkage-name on" ""
> +
> +gdb_test "info break" \
> + "Num Type\[ \]+Disp Enb Address\[\t \]+What.*
> +1\[\t \]+breakpoint keep y.* in foo(.*) \\\[_Z3fooPKc\\\] at .*$srcfile:$bp1.*
> +2\[\t \]+breakpoint keep y.* in fun_with_a_long_name(.*) \\\[_Z20fun_with_a_long_\.\.\.\\\] at .*$srcfile:$bp2.*" \
> + "info breakpoint - display on"
> +
> +gdb_run_cmd
> +set test "break at fun_with_a_long_name - display on"
There is extra trailing whitespace here, too.
> +gdb_test_multiple "continue" $test {
> + -re "Breakpoint 2, fun_with_a_long_name \\\[_Z20fun_with_a_long_...\\\] (.*) at.*$srcfile.$bp2.*$gdb_prompt $" {
> + pass $test
> + break
> + }
> +}
gdb_continue_to_breakpoint
> +
> +set bttable "#0 foo \\\[_Z3fooPKc\\\] (.*) at.*\[\r\n\]"
> +append bttable "#1 $hex in fun_with_a_long_name \\\[_Z20fun_with_a_long_\.\.\.\\\] (.*) at .*$srcfile:.*\[\r\n\]"
> +append bttable "#2 $hex in goo \\\[_Z3goov\\\] (.*) at .*$srcfile:.*\[\r\n\]"
> +append bttable "#3 $hex in main (.*) at .*$srcfile:.*"
> +
> +gdb_test "backtrace" $bttable "backtrace - display on"
> +
> +########################
> +# Test set/show display-linkage-name-len
> +
> +gdb_test "show display-linkage-name-len" \
> + "Length of linkage names \\(symbol used by linker\\) to be displayed is 20."
> +
> +gdb_test_no_output "set display-linkage-name-len 10" ""
> +
> +gdb_test "show display-linkage-name-len" \
> + "Length of linkage names \\(symbol used by linker\\) to be displayed is 10."
> +
> +gdb_test "info break" \
> + "Num Type\[ \]+Disp Enb Address\[\t \]+What.*
> +1\[\t \]+breakpoint keep y.* in foo(.*) \\\[_Z3fooPKc\\\] at .*$srcfile:$bp1.*
> +2\[\t \]+breakpoint keep y.* in fun_with_a_long_name(.*) \\\[_Z20fun_wi\.\.\.\\\] at .*$srcfile:$bp2.*" \
> + "info breakpoint - display 10"
> +
> +gdb_run_cmd
> +set test "break at fun_with_a_long_name - display 10"
trailing whitespace
> +gdb_test_multiple "continue" $test {
> + -re "Breakpoint 2, fun_with_a_long_name \\\[_Z20fun_wi\.\.\.\\\] (.*) at .*$srcfile:$bp2.*$gdb_prompt $" {
> + pass $test
> + break
> + }
> +}
gdb_continue_to_breakpoint
> +
> +set bttable "#0 foo \\\[_Z3fooPKc\\\] (.*) at.*\[\r\n\]"
> +append bttable "#1 $hex in fun_with_a_long_name \\\[_Z20fun_wi\.\.\.\\\] (.*) at .*$srcfile:.*\[\r\n\]"
> +append bttable "#2 $hex in goo \\\[_Z3goov\\\] (.*) at .*$srcfile:.*\[\r\n\]"
> +append bttable "#3 $hex in main (.*) at .*$srcfile:.*"
> +
> +gdb_test "backtrace" $bttable "backtrace - display 10"
> +
> diff --git a/gdb/top.c b/gdb/top.c
> index 46faaa7..c0f6f25 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -287,6 +287,29 @@ quit_cover (void)
> quit_command ((char *) 0, 0);
> }
> #endif /* defined SIGHUP */
> +
> +/* Flag for whether we want to print linkage name for functions.
> + Length of linkage name to print. */
> +
> +int display_linkage_name = 0; /* Default is no. */
> +int display_linkage_name_len = 20; /* Default is first 20 chars. */
> +
> +static void
> +show_display_linkage_name (struct ui_file *file, int from_tty,
> + struct cmd_list_element *c, const char *value)
> +{
> + fprintf_filtered (file, _("\
> +Whether to display linkage name (symbol used by linker) of functions is %s.\n"),
> + value);
> +}
> +static void
> +show_display_linkage_name_len (struct ui_file *file, int from_tty,
> + struct cmd_list_element *c, const char *value)
> +{
> + fprintf_filtered (file, _("\
> +Length of linkage names (symbol used by linker) to be displayed is %s.\n"),
> + value);
> +}
>
> /* Line number we are currently in, in a file which is being sourced. */
> /* NOTE 1999-04-29: This variable will be static again, once we modify
> @@ -1809,7 +1832,22 @@ Use \"on\" to enable the notification, and \"off\" to disable it."),
> When set, GDB uses the specified path to search for data files."),
> set_gdb_datadir, NULL,
> &setlist,
> - &showlist);
> + &showlist);
Unnecessary whitespace change?
> +
> + add_setshow_boolean_cmd ("display-linkage-name", class_support, &display_linkage_name, _("\
> +Set whether to display linkage name (symbol used by linker) for functions."), _("\
> +Show whether to display linkage name (symbol used by linker) for functions."), NULL,
> + NULL,
> + show_display_linkage_name,
> + &setlist, &showlist);
> +
> + add_setshow_zinteger_cmd ("display-linkage-name-len", class_support, &display_linkage_name_len, _("\
> +Set number of characters of linkage name to display."), _("\
> +Show number of characters of linkage name to display."), NULL,
> + NULL,
> + show_display_linkage_name_len,
> + &setlist, &showlist);
> +
> }
I think with these minor things fixed, a global maintainer should give a
final review (and approval).
Keith
More information about the Gdb-patches
mailing list