public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases
@ 2014-01-31  9:48 Joel Brobecker
  2014-01-31 11:30 ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Joel Brobecker @ 2014-01-31  9:48 UTC (permalink / raw)
  To: gdb-patches

Hello,

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.

Another option is to port this patch to the gdb-7.7 branch to make it
deprecated in the 7.7 release, allowing us to remove support for these
commands from master now.

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.

Tested on x86-windows. Thoughts?

Thanks,
-- 
Joel

---
 gdb/NEWS          |  4 ++++
 gdb/windows-nat.c | 13 +++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 5062e02..14af602 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -51,6 +51,10 @@ qXfer:btrace:read's annex
   The qXfer:btrace:read packet supports a new annex 'delta' to read
   branch trace incrementally.
 
+* 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/windows-nat.c b/gdb/windows-nat.c
index 5bcb7b7..68a567b 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -2666,12 +2666,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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases
  2014-01-31  9:48 [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases Joel Brobecker
@ 2014-01-31 11:30 ` Eli Zaretskii
  2014-01-31 11:39   ` Joel Brobecker
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2014-01-31 11:30 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

> From: Joel Brobecker <brobecker@adacore.com>
> Date: Fri, 31 Jan 2014 13:48:26 +0400
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 5062e02..14af602 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -51,6 +51,10 @@ qXfer:btrace:read's annex
>    The qXfer:btrace:read packet supports a new annex 'delta' to read
>    branch trace incrementally.
>  
> +* 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.

OK for NEWS, but should we remove this command from gdb.texinfo?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases
  2014-01-31 11:30 ` Eli Zaretskii
@ 2014-01-31 11:39   ` Joel Brobecker
  2014-01-31 11:49     ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Joel Brobecker @ 2014-01-31 11:39 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

> > +* 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.
> 
> OK for NEWS, but should we remove this command from gdb.texinfo?

Thanks! I was planning on removing the command from gdb.texinfo
when it is actually deleted. But I can certainly do it now too!
If no one else comments on it, I'll go with your suggestion of
removing it now.

-- 
Joel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases
  2014-01-31 11:39   ` Joel Brobecker
@ 2014-01-31 11:49     ` Eli Zaretskii
  2014-01-31 12:22       ` Joel Brobecker
  2014-02-04 20:20       ` Pedro Alves
  0 siblings, 2 replies; 19+ messages in thread
From: Eli Zaretskii @ 2014-01-31 11:49 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

> Date: Fri, 31 Jan 2014 15:39:24 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> > > +* 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.
> > 
> > OK for NEWS, but should we remove this command from gdb.texinfo?
> 
> Thanks! I was planning on removing the command from gdb.texinfo
> when it is actually deleted. But I can certainly do it now too!
> If no one else comments on it, I'll go with your suggestion of
> removing it now.

I think we shouldn't advertise deprecated features.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases
  2014-01-31 11:49     ` Eli Zaretskii
@ 2014-01-31 12:22       ` Joel Brobecker
  2014-01-31 14:33         ` Eli Zaretskii
  2014-02-04 20:20       ` Pedro Alves
  1 sibling, 1 reply; 19+ messages in thread
From: Joel Brobecker @ 2014-01-31 12:22 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

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

> > Thanks! I was planning on removing the command from gdb.texinfo
> > when it is actually deleted. But I can certainly do it now too!
> > If no one else comments on it, I'll go with your suggestion of
> > removing it now.
> 
> I think we shouldn't advertise deprecated features.

Sure.

Upon grep'ing the manual, I found the following paragraph which
promotes a rather peculiar use of the command:

| 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.

I think that suggesting the loading of the DLL before getting it
actually mapped is a little iffy and that it's much better, IMO,
to ask the user to just run the program the same way we do it
on all other platforms. You can emulate that behavior using
the "symbol-file" command, but I don't think this was the intention
of that command and that we should validate it here.

With that in mind, and since I was almost done with the change,
if finished it with the assumption that this was going to be
an acceptable loss of functionality, I still went ahead with
the doco update. If people feel that this is not acceptable, then
my patch should be dropped, and I think the documentation should
be clarified about the dll-symbols command intended usage and
limitations (whoever opposed the removal can do it!?! ;-)).

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): Remove documentation of commands
        "add-shared-symbol-files" and "assf".
        (Cygwin Native): Remove documentation of the "dll-symbols"
        command.
        (Non-debug DLL Symbols): Remove reference to "dll-symbols"
        as a way to force the loading of symbols from a DLL.

-- 
Joel

[-- Attachment #2: 0001-Deprecate-windows-specific-dll-symbols-command-and-a.patch --]
[-- Type: text/x-diff, Size: 7101 bytes --]

From b21076d3544ac896573a4a51fa2950acca07c949 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): Remove documentation of commands
        "add-shared-symbol-files" and "assf".
        (Cygwin Native): Remove documentation of the "dll-symbols"
        command.
        (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 | 26 +-------------------------
 gdb/windows-nat.c   | 13 +++++++++----
 3 files changed, 14 insertions(+), 29 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 5062e02..14af602 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -51,6 +51,10 @@ qXfer:btrace:read's annex
   The qXfer:btrace:read packet supports a new annex 'delta' to read
   branch trace incrementally.
 
+* 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 af14286..4f84318 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -16919,19 +16919,6 @@ evaluation yields the address of the file's shared object file header.
 For this command to work, you must have used @code{symbol-file} or
 @code{exec-file} commands in advance.
 
-@kindex add-shared-symbol-files
-@kindex assf
-@item add-shared-symbol-files @var{library-file}
-@itemx assf @var{library-file}
-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}).
-@value{GDBN} automatically looks for shared libraries, however if
-@value{GDBN} does not find yours, you can invoke
-@code{add-shared-symbol-files}.  It takes one argument: the shared
-library's file name.  @code{assf} is a shorthand alias for
-@code{add-shared-symbol-files}.
-
 @kindex section
 @item section @var{section} @var{addr}
 The @code{section} command changes the base address of the named
@@ -19899,11 +19886,6 @@ selector for 32-bit programs and @code{$gs} for 64-bit programs).
 @item info dll
 This is a Cygwin-specific alias of @code{info shared}.
 
-@kindex dll-symbols
-@item dll-symbols
-This command loads symbols from a dll similarly to
-add-sym command but without the need to specify a base address.
-
 @kindex set cygwin-exceptions
 @cindex debugging the Cygwin DLL
 @cindex Cygwin DLL, debugging
@@ -19998,13 +19980,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 5bcb7b7..68a567b 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -2666,12 +2666,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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases
  2014-01-31 12:22       ` Joel Brobecker
@ 2014-01-31 14:33         ` Eli Zaretskii
  0 siblings, 0 replies; 19+ messages in thread
From: Eli Zaretskii @ 2014-01-31 14:33 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

> Date: Fri, 31 Jan 2014 16:22:03 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> | 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.
> 
> I think that suggesting the loading of the DLL before getting it
> actually mapped is a little iffy and that it's much better, IMO,
> to ask the user to just run the program the same way we do it
> on all other platforms.

Doesn't "sharedlibrary" have the same effect as dll-symbols in this
scenario?

> You can emulate that behavior using the "symbol-file" command, but I
> don't think this was the intention of that command and that we
> should validate it here.

Intention aside, the important question, I think, is whether we really
want to discourage such uses of "symbol-file", and if so, why.

> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -16919,19 +16919,6 @@ evaluation yields the address of the file's shared object file header.
>  For this command to work, you must have used @code{symbol-file} or
>  @code{exec-file} commands in advance.
>  
> -@kindex add-shared-symbol-files
> -@kindex assf
> -@item add-shared-symbol-files @var{library-file}
> -@itemx assf @var{library-file}
> -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}).
> -@value{GDBN} automatically looks for shared libraries, however if
> -@value{GDBN} does not find yours, you can invoke
> -@code{add-shared-symbol-files}.  It takes one argument: the shared
> -library's file name.  @code{assf} is a shorthand alias for
> -@code{add-shared-symbol-files}.
> -
>  @kindex section
>  @item section @var{section} @var{addr}
>  The @code{section} command changes the base address of the named
> @@ -19899,11 +19886,6 @@ selector for 32-bit programs and @code{$gs} for 64-bit programs).
>  @item info dll
>  This is a Cygwin-specific alias of @code{info shared}.
>  
> -@kindex dll-symbols
> -@item dll-symbols
> -This command loads symbols from a dll similarly to
> -add-sym command but without the need to specify a base address.
> -
>  @kindex set cygwin-exceptions
>  @cindex debugging the Cygwin DLL
>  @cindex Cygwin DLL, debugging
> @@ -19998,13 +19980,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.

I'm OK with this, provided that we really don't want users to know
about this use of "symbol-file".

Also, I'd like to hear at least from Chris and/or Corinna, as I don't
think I ever used dll-symbols myself.

Thanks.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases
  2014-01-31 11:49     ` Eli Zaretskii
  2014-01-31 12:22       ` Joel Brobecker
@ 2014-02-04 20:20       ` Pedro Alves
  2014-02-10 14:34         ` Joel Brobecker
  1 sibling, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2014-02-04 20:20 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Joel Brobecker, gdb-patches

On 01/31/2014 11:49 AM, Eli Zaretskii wrote:
>> > If no one else comments on it, I'll go with your suggestion of
>> > removing it now.
> I think we shouldn't advertise deprecated features.

I'm not sure I agree with that policy.  I mean, we shouldn't advertise
them, yes, and I think we do hide deprecated commands in the CLI
at places, IIRC, but as long as they exist, I think they should
be documented.  IMO, it'd be nicer to users who are currently using
such commands, and find out they're deprecated (because GDB warns) if
they go to the manual and find both the replacement they should be
using (and how), and the docu for the old command, so they can easily
map arguments, etc.  IOW, IMO, we should instead add a "This command is
deprecated; a better alternative is blah blah" note.  When I grep
for deprecate_cmd, and then look for the documentation of the
corresponding deprecated commands, I do find it.
E.g., "record restore", "disable tracepoint", "set remotebreak", etc.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases
  2014-02-04 20:20       ` Pedro Alves
@ 2014-02-10 14:34         ` Joel Brobecker
  2014-02-12 14:44           ` Pedro Alves
  0 siblings, 1 reply; 19+ messages in thread
From: Joel Brobecker @ 2014-02-10 14:34 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Eli Zaretskii, gdb-patches

> I'm not sure I agree with that policy.  I mean, we shouldn't advertise
> them, yes, and I think we do hide deprecated commands in the CLI
> at places, IIRC, but as long as they exist, I think they should
> be documented.  IMO, it'd be nicer to users who are currently using
> such commands, and find out they're deprecated (because GDB warns) if
> they go to the manual and find both the replacement they should be
> using (and how), and the docu for the old command, so they can easily
> map arguments, etc.  IOW, IMO, we should instead add a "This command is
> deprecated; a better alternative is blah blah" note.  When I grep
> for deprecate_cmd, and then look for the documentation of the
> corresponding deprecated commands, I do find it.
> E.g., "record restore", "disable tracepoint", "set remotebreak", etc.

My 2 cents: I think it's fine to either remove the documentation, or
else leave it as is. Adding more info as you suggest is indeed making
the documentation better, but only for a short period of time. During
that period of time, if people happen to read about the deprecated
command in the manual, and then actually use that function, they'll
immediately get a warning, the warning will tell them which command to
use in its place. On the other hand, if we excise the commands'
documentation and they are searching the users manual, the only
command they can find are the non-deprecated ones.

In that respect, I now tend to agree with Eli. But it's not a strong
opinion.

-- 
Joel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases
  2014-02-10 14:34         ` Joel Brobecker
@ 2014-02-12 14:44           ` Pedro Alves
  2014-02-12 17:17             ` Joel Brobecker
  0 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2014-02-12 14:44 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Eli Zaretskii, gdb-patches

On 02/10/2014 02:34 PM, Joel Brobecker wrote:

> My 2 cents: I think it's fine to either remove the documentation, or
> else leave it as is. Adding more info as you suggest is indeed making
> the documentation better, but only for a short period of time. 

"short period of time" here doesn't seem all that correct to me.
Sure, it's short if you track mainline.  But a user using a
release will be looking at the documentation of the gdb version
she is using.  In that perspective, the fact that mainline has
already changed is not what dictates how long the user sees
the documentation.  The next upgrade cycle does.

> During
> that period of time, if people happen to read about the deprecated
> command in the manual, and then actually use that function, they'll
> immediately get a warning, the warning will tell them which command to
> use in its place. On the other hand, if we excise the commands'
> documentation and they are searching the users manual, the only
> command they can find are the non-deprecated ones.
> 
> In that respect, I now tend to agree with Eli. But it's not a strong
> opinion.

Alright, so AIUI, the argument for deleting seems to be that we
should not advertise deprecated commands.

IOW, we should avoid that people reading the manual end up using
deprecated commands when a better alternative exists, and the
deprecated command might well be removed soon.

I completely agree that we should avoid that.  What I don't agree
is that deleting the documentation is the only way to do that.
I believe that adding a note to the command's documentation in
the manual saying the command is deprecated and pointing at the
better alternative would do just as well.

I'm of the opinion that as long as the command exists, it
should be documented.  Sure, by being deprecated, the user
should no longer rely on its existence, but it might take
a while for said user to actually upgrade to a gdb that
actually removes the command (if we ever do), and there's
no need IMO to make it more painful for the user to
look up documentation for the command she is actually
currently using right now to solve some hard to catch
bug in her application.  If we remove the documentation,
then she has to do look for a manual of an older gdb.

Note that old -> new command mapping might not be
trivial, it might need adjustment of command parameters,
and might well be that the new form is actually a sequence
of commands, not a single command.

(Note that I'm talking about policy here, not about
any specific command.)

So it seems like we have 3 possible policies:

 #1 - Leave the manual unchanged when we deprecate commands.  Delete
      the documentation at the same time the command is actually
      deleted.

 #2 - Always excise documentation for deprecated commands at the same
     time we do the deprecation.

 #3 - Mark the commands deprecated in the manual at the same time
      we mark them deprecated in the code.  Delete the documentation
      at the same time the command is actually deleted.

In my view, #1 is just a bad policy.  Having documentation
for deprecated commands behind _without_ a "deprecated,
use foo instead" note in them might lead users to find
the old command and start using them while newer better
alternatives exist.  I think everyone will agree to that.

And in my view, #3 is a superior policy than #2.  But
I'll accept #2, if that's what the group ends up preferring.

Whatever we end up deciding, I think we should document
the outcome as guideline somewhere in the internals
manual, and if we go with #2, then we it'd be good
to make a pass over the manual and remove all the
existing documentation for currently deprecated
commands.

Thanks,
-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases
  2014-02-12 14:44           ` Pedro Alves
@ 2014-02-12 17:17             ` Joel Brobecker
  2014-02-13 12:01               ` Joel Brobecker
  0 siblings, 1 reply; 19+ messages in thread
From: Joel Brobecker @ 2014-02-12 17:17 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Eli Zaretskii, gdb-patches

Hi Pedro,

> So it seems like we have 3 possible policies:
> 
>  #1 - Leave the manual unchanged when we deprecate commands.  Delete
>       the documentation at the same time the command is actually
>       deleted.
> 
>  #2 - Always excise documentation for deprecated commands at the same
>      time we do the deprecation.
> 
>  #3 - Mark the commands deprecated in the manual at the same time
>       we mark them deprecated in the code.  Delete the documentation
>       at the same time the command is actually deleted.
> 
> In my view, #1 is just a bad policy.  Having documentation
> for deprecated commands behind _without_ a "deprecated,
> use foo instead" note in them might lead users to find
> the old command and start using them while newer better
> alternatives exist.  I think everyone will agree to that.
> 
> And in my view, #3 is a superior policy than #2.  But
> I'll accept #2, if that's what the group ends up preferring.
> 
> Whatever we end up deciding, I think we should document
> the outcome as guideline somewhere in the internals
> manual, and if we go with #2, then we it'd be good
> to make a pass over the manual and remove all the
> existing documentation for currently deprecated
> commands.

I understand your reasoning, and can agree with you in the sense
that someone already using the deprecated command might want to
look some detail up in the GDB manual and not find it.

But I do not have a strong opininon on this, and whatever we end up
deciding is fine by me. Let's just decide now :). I will go with #3,
or else #2.

-- 
Joel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases
  2014-02-12 17:17             ` Joel Brobecker
@ 2014-02-13 12:01               ` Joel Brobecker
  2014-02-13 16:02                 ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Joel Brobecker @ 2014-02-13 12:01 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Eli Zaretskii, gdb-patches

Hi Eli,

Since you are the official Maintainer of the GDB documentation,
would you mind telling us what you think? I'd like us to make
a decision, so that I can redo the documentation that goes
with the patch.

Thank you!

> > So it seems like we have 3 possible policies:
> > 
> >  #1 - Leave the manual unchanged when we deprecate commands.  Delete
> >       the documentation at the same time the command is actually
> >       deleted.
> > 
> >  #2 - Always excise documentation for deprecated commands at the same
> >      time we do the deprecation.
> > 
> >  #3 - Mark the commands deprecated in the manual at the same time
> >       we mark them deprecated in the code.  Delete the documentation
> >       at the same time the command is actually deleted.
> > 
> > In my view, #1 is just a bad policy.  Having documentation
> > for deprecated commands behind _without_ a "deprecated,
> > use foo instead" note in them might lead users to find
> > the old command and start using them while newer better
> > alternatives exist.  I think everyone will agree to that.
> > 
> > And in my view, #3 is a superior policy than #2.  But
> > I'll accept #2, if that's what the group ends up preferring.
> > 
> > Whatever we end up deciding, I think we should document
> > the outcome as guideline somewhere in the internals
> > manual, and if we go with #2, then we it'd be good
> > to make a pass over the manual and remove all the
> > existing documentation for currently deprecated
> > commands.
> 
> I understand your reasoning, and can agree with you in the sense
> that someone already using the deprecated command might want to
> look some detail up in the GDB manual and not find it.
> 
> But I do not have a strong opininon on this, and whatever we end up
> deciding is fine by me. Let's just decide now :). I will go with #3,
> or else #2.
> 
> -- 
> Joel

-- 
Joel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases
  2014-02-13 12:01               ` Joel Brobecker
@ 2014-02-13 16:02                 ` Eli Zaretskii
  2014-02-13 16:43                   ` Joel Brobecker
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2014-02-13 16:02 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: palves, gdb-patches

> Date: Thu, 13 Feb 2014 16:01:03 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, gdb-patches@sourceware.org
> 
> Hi Eli,
> 
> Since you are the official Maintainer of the GDB documentation,
> would you mind telling us what you think?

I thought I already did, in the beginning of this thread.  Didn't I?

> I'd like us to make a decision, so that I can redo the documentation
> that goes with the patch.

What is the mechanism of making decisions when not everybody agrees?
If by majority, then it sounds like 2 out of 3 want/prefer alternative
#3.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases
  2014-02-13 16:02                 ` Eli Zaretskii
@ 2014-02-13 16:43                   ` Joel Brobecker
  2014-02-13 17:03                     ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Joel Brobecker @ 2014-02-13 16:43 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: palves, gdb-patches

> > Since you are the official Maintainer of the GDB documentation,
> > would you mind telling us what you think?
> 
> I thought I already did, in the beginning of this thread.  Didn't I?

Well, I thought that Pedro hadn't had a chance to expose his arguments,
yet, and maybe they might have affected your point of view?

> > I'd like us to make a decision, so that I can redo the documentation
> > that goes with the patch.
> 
> What is the mechanism of making decisions when not everybody agrees?
> If by majority, then it sounds like 2 out of 3 want/prefer alternative
> #3.

For myself, I'm happy either way, even if I tend to indeed prefer #3
(now).  I think Pedro had a soft preference also, but I will let him
confirm.  In this case, you are the Maintainer of the GDB documentation,
so I would tend to give you more weight in that decision.

Based on this reply, do I understand correctly that you still think
we should remove these commands from the manual?

-- 
Joel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases
  2014-02-13 16:43                   ` Joel Brobecker
@ 2014-02-13 17:03                     ` Eli Zaretskii
  2014-02-13 17:24                       ` Pedro Alves
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2014-02-13 17:03 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: palves, gdb-patches

> Date: Thu, 13 Feb 2014 20:43:07 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: palves@redhat.com, gdb-patches@sourceware.org
> 
> Based on this reply, do I understand correctly that you still think
> we should remove these commands from the manual?

I do.  But I won't fight over it.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases
  2014-02-13 17:03                     ` Eli Zaretskii
@ 2014-02-13 17:24                       ` Pedro Alves
  2014-02-13 17:40                         ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2014-02-13 17:24 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Joel Brobecker, gdb-patches

On 02/13/2014 05:03 PM, Eli Zaretskii wrote:
>> Date: Thu, 13 Feb 2014 20:43:07 +0400
>> From: Joel Brobecker <brobecker@adacore.com>
>> Cc: palves@redhat.com, gdb-patches@sourceware.org
>>
>> Based on this reply, do I understand correctly that you still think
>> we should remove these commands from the manual?
> 
> I do.  But I won't fight over it.

I wasn't trying to fight, only to get my point across.

In the end it's your decision, and I respect it.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases
  2014-02-13 17:24                       ` Pedro Alves
@ 2014-02-13 17:40                         ` Eli Zaretskii
  2014-02-19  9:43                           ` Joel Brobecker
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2014-02-13 17:40 UTC (permalink / raw)
  To: Pedro Alves; +Cc: brobecker, gdb-patches

> Date: Thu, 13 Feb 2014 17:24:10 +0000
> From: Pedro Alves <palves@redhat.com>
> CC: Joel Brobecker <brobecker@adacore.com>, gdb-patches@sourceware.org
> 
> On 02/13/2014 05:03 PM, Eli Zaretskii wrote:
> >> Date: Thu, 13 Feb 2014 20:43:07 +0400
> >> From: Joel Brobecker <brobecker@adacore.com>
> >> Cc: palves@redhat.com, gdb-patches@sourceware.org
> >>
> >> Based on this reply, do I understand correctly that you still think
> >> we should remove these commands from the manual?
> > 
> > I do.  But I won't fight over it.
> 
> I wasn't trying to fight, only to get my point across.
> 
> In the end it's your decision, and I respect it.

Let's go with your suggestion, then.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases
  2014-02-13 17:40                         ` Eli Zaretskii
@ 2014-02-19  9:43                           ` Joel Brobecker
  2014-02-19 16:44                             ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Joel Brobecker @ 2014-02-19  9:43 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Pedro Alves, gdb-patches

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

Hi Eli,

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? 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

[-- Attachment #2: 0001-Deprecate-windows-specific-dll-symbols-command-and-a.patch --]
[-- Type: text/x-diff, Size: 6563 bytes --]

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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases
  2014-02-19  9:43                           ` Joel Brobecker
@ 2014-02-19 16:44                             ` Eli Zaretskii
  2014-02-20  8:34                               ` Joel Brobecker
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2014-02-19 16:44 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: palves, gdb-patches

> 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
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases
  2014-02-19 16:44                             ` Eli Zaretskii
@ 2014-02-20  8:34                               ` Joel Brobecker
  0 siblings, 0 replies; 19+ messages in thread
From: Joel Brobecker @ 2014-02-20  8:34 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

> > 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.

Thanks, Eli. The patch is now in.

> > 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.
-- 
Joel

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2014-02-20  8:34 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-31  9:48 [Windows/RFA/commit] Deprecate windows-specific dll-symbols command and aliases Joel Brobecker
2014-01-31 11:30 ` Eli Zaretskii
2014-01-31 11:39   ` Joel Brobecker
2014-01-31 11:49     ` Eli Zaretskii
2014-01-31 12:22       ` Joel Brobecker
2014-01-31 14:33         ` Eli Zaretskii
2014-02-04 20:20       ` Pedro Alves
2014-02-10 14:34         ` Joel Brobecker
2014-02-12 14:44           ` Pedro Alves
2014-02-12 17:17             ` Joel Brobecker
2014-02-13 12:01               ` Joel Brobecker
2014-02-13 16:02                 ` Eli Zaretskii
2014-02-13 16:43                   ` Joel Brobecker
2014-02-13 17:03                     ` Eli Zaretskii
2014-02-13 17:24                       ` Pedro Alves
2014-02-13 17:40                         ` Eli Zaretskii
2014-02-19  9:43                           ` Joel Brobecker
2014-02-19 16:44                             ` Eli Zaretskii
2014-02-20  8:34                               ` Joel Brobecker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).