Re: [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases

[Eli Zaretskii <eliz@gnu.org>]

> Date: Wed, 19 Feb 2014 10:43:27 +0100
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Pedro Alves <palves@redhat.com>, gdb-patches@sourceware.org
> 
> Attached is the new version of the patch, with the GDB manual now
> simply stating that the commands are deprecated instead of removing
> their documentation. Can you tell me if the changes look OK?

Yes, it's OK.

Thanks.


 I still
> kept the removal of the dubious recommendation of loading the DLL
> statements by hand, as I don't think we should be advertising that
> practice.
> 
> gdb/ChangeLog:
> 
>         * windows-nat.c (_initialize_windows_nat): Deprecate the
>         "dll-symbols" command.  Turn the "add-shared-symbol-files"
>         and "assf" aliases into commands, and deprecate them as well.
>         * NEWS: Add entry explaining that "dll-symbols" and its two
>         aliases are now deprecated.
> 
> gdb/doc/ChangeLog:
> 
>         * gdb.texinfo (Files): Document "add-shared-symbol-files"
>         and "assf" as being deprecated.
>         (Cygwin Native): Likewise for "dll-symbols".
>         (Non-debug DLL Symbols): Remove reference to "dll-symbols"
>         as a way to force the loading of symbols from a DLL.
> 
> Thank you,
> -- 
> Joel
> 
> [2:text/x-diff Hide Save:0001-Deprecate-windows-specific-dll-symbols-command-and-a.patch (6kB)]
> 
> >From b89a5f03be0fd9e70506f08f499051f2bf8e2d3d Mon Sep 17 00:00:00 2001
> From: Joel Brobecker <brobecker@adacore.com>
> Date: Fri, 31 Jan 2014 04:22:53 -0500
> Subject: [PATCH] Deprecate windows-specific dll-symbols command and aliases
> 
> The "dll-symbols" command, specific to native Windows platforms,
> gives the impression that the symbols were not loaded, first
> because it completes silently, and second because the "info shared"
> output does not get updated after the command completes:
> 
>     (gdb) dll-symbols C:\WINDOWS\syswow64\rpcrt4.dll
>     (gdb) info shared
>     From        To          Syms Read   Shared Object Library
>     [...]
>     0x77e51000  0x77ee2554  No          C:\WINDOWS\system32\rpcrt4.dll
> 
> (we exected the "Syms Read" column to read "Yes").
> 
> As far as I can tell, the symbols actually do get loaded, but completely
> independently from the solib framework, which explains the silent
> loading and the fact that the "Syms Read" column does not get updated.
> See windows-nat.c::safe_symbol_file_add_stub, which calls symbol_file_add
> instead of calling solib_add.
> 
> But, aside from the fact that the "Syms Read" status does not get
> updated, I also noticed that it does not take into account the DLL's
> actual load address when loading its symbols. As a result, I believe
> that we get it wrong if the DLL does not get loaded at the prefered
> address.
> 
> Rather than trying to fix this command, there does not seem to be
> a reason other than historical for having Windows-specific commands
> which essentially re-implements the "sharedlibrary" command. The
> command interface is slightly different (the latter takes a regexp
> rather than a plain filename), but it should be just as easy to use
> the "sharedlibrary" command, or its "share" alias, as usisng the
> "dll-symbols" command. For instance:
> 
>     (gdb) share rpcrt4.dll
>     Reading symbols from C:\WINDOWS\system32\rpcrt4.dll...(no debugging symbols found)...done.
>     Loaded symbols for C:\WINDOWS\system32\rpcrt4.dll
>     (gdb) info shared
>     From        To          Syms Read   Shared Object Library
>     [...]
>     0x77e51000  0x77ee2554  Yes (*)     C:\WINDOWS\system32\rpcrt4.dll
> 
> This patch therefore deprecates the "dll-symbols" command, as well
> as its two aliases "add-shared-symbol-files" and "assf", with a view
> of deleting them as soon as the 7.8 branch gets cut.
> 
> gdb/ChangeLog:
> 
> 	* windows-nat.c (_initialize_windows_nat): Deprecate the
> 	"dll-symbols" command.  Turn the "add-shared-symbol-files"
> 	and "assf" aliases into commands, and deprecate them as well.
> 	* NEWS: Add entry explaining that "dll-symbols" and its two
> 	aliases are now deprecated.
> 
> gdb/doc/ChangeLog:
> 
>         * gdb.texinfo (Files): Document "add-shared-symbol-files"
>         and "assf" as being deprecated.
>         (Cygwin Native): Likewise for "dll-symbols".
>         (Non-debug DLL Symbols): Remove reference to "dll-symbols"
>         as a way to force the loading of symbols from a DLL.
> ---
>  gdb/NEWS            |  4 ++++
>  gdb/doc/gdb.texinfo | 14 +++++++-------
>  gdb/windows-nat.c   | 13 +++++++++----
>  3 files changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index b54a414..eeb2f94 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -85,6 +85,10 @@ qXfer:btrace:read's annex
>  * New targets
>  PowerPC64 GNU/Linux little-endian	powerpc64le-*-linux*
>  
> +* The "dll-symbols" command, and its two aliases ("add-shared-symbol-files"
> +  and "assf"), have been deprecated.  Use the "sharedlibrary" command, or
> +  its alias "share", instead.
> +
>  *** Changes in GDB 7.7
>  
>  * Improved support for process record-replay and reverse debugging on
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 7dc1564..de5ac63 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -16923,6 +16923,9 @@ For this command to work, you must have used @code{symbol-file} or
>  @kindex assf
>  @item add-shared-symbol-files @var{library-file}
>  @itemx assf @var{library-file}
> +This command is deprecated and will be removed in future versions
> +of @value{GDBN}.  Use the @code{sharedlibrary} command instead.
> +
>  The @code{add-shared-symbol-files} command can currently be used only
>  in the Cygwin build of @value{GDBN} on MS-Windows OS, where it is an
>  alias for the @code{dll-symbols} command (@pxref{Cygwin Native}).
> @@ -19901,6 +19904,9 @@ This is a Cygwin-specific alias of @code{info shared}.
>  
>  @kindex dll-symbols
>  @item dll-symbols
> +This command is deprecated and will be removed in future versions
> +of @value{GDBN}.  Use the @code{sharedlibrary} command instead.
> +
>  This command loads symbols from a dll similarly to
>  add-sym command but without the need to specify a base address.
>  
> @@ -19998,13 +20004,7 @@ describes working with such symbols, known internally to @value{GDBN} as
>  Note that before the debugged program has started execution, no DLLs
>  will have been loaded.  The easiest way around this problem is simply to
>  start the program --- either by setting a breakpoint or letting the
> -program run once to completion.  It is also possible to force
> -@value{GDBN} to load a particular DLL before starting the executable ---
> -see the shared library information in @ref{Files}, or the
> -@code{dll-symbols} command in @ref{Cygwin Native}.  Currently,
> -explicitly loading symbols from a DLL with no debugging information will
> -cause the symbol names to be duplicated in @value{GDBN}'s lookup table,
> -which may adversely affect symbol lookup performance.
> +program run once to completion.
>  
>  @subsubsection DLL Name Prefixes
>  
> diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
> index 9212adf..3dbd3c2 100644
> --- a/gdb/windows-nat.c
> +++ b/gdb/windows-nat.c
> @@ -2671,12 +2671,17 @@ _initialize_windows_nat (void)
>    c = add_com ("dll-symbols", class_files, dll_symbol_command,
>  	       _("Load dll library symbols from FILE."));
>    set_cmd_completer (c, filename_completer);
> +  deprecate_cmd (c, "sharedlibrary");
>  
> -  add_com_alias ("sharedlibrary", "dll-symbols", class_alias, 1);
> -
> -  add_com_alias ("add-shared-symbol-files", "dll-symbols", class_alias, 1);
> +  c = add_com ("add-shared-symbol-files", class_files, dll_symbol_command,
> +	       _("Load dll library symbols from FILE."));
> +  set_cmd_completer (c, filename_completer);
> +  deprecate_cmd (c, "sharedlibrary");
>  
> -  add_com_alias ("assf", "dll-symbols", class_alias, 1);
> +  c = add_com ("assf", class_files, dll_symbol_command,
> +	       _("Load dll library symbols from FILE."));
> +  set_cmd_completer (c, filename_completer);
> +  deprecate_cmd (c, "sharedlibrary");
>  
>  #ifdef __CYGWIN__
>    add_setshow_boolean_cmd ("shell", class_support, &useshell, _("\
> -- 
> 1.8.3.2
>