public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2] [PR gdb/27614] gdb-add-index fails on symlinks.
@ 2021-03-26 17:11 Lancelot SIX
  2021-03-26 18:53 ` Simon Marchi
  0 siblings, 1 reply; 3+ messages in thread
From: Lancelot SIX @ 2021-03-26 17:11 UTC (permalink / raw)
  To: gdb-patches; +Cc: Lancelot SIX

Since V1:
        - Replace '&>/dev/null' with '>/dev/null 2>&1'

--

PR 27614 shows that gdb-add-index fails to generate the index when its
argument is a symlink.

The following one liner illustrates the reported problem:

        $ echo 'int main(){}'|gcc -g -x c -;ln -s a.out symlink;gdb-add-index symlink
        gdb-add-index: No index was created for symlink
        gdb-add-index: [Was there no debuginfo? Was there already an index?]
        $ ls -l
        -rwxr-xr-x 1 25712 Mar 19 23:05 a.out*
        -rw------- 1  8277 Mar 19 23:05 a.out.gdb-index
        lrwxrwxrwx 1     5 Mar 19 23:05 symlink -> a.out*

GDB generates the .gdb-index file with a name that matches the name of
the actual program (a.out.gdb-index here), not the symlink that
references it.  The remaining of the script is looking for a file named
after the provided argument (would be 'symlink.gdb-index' in our
example).

To fix this, this commit proposes to use 'readlink' when available.  This
command is not part of the standard POSIX tools, so this fix might not
work in all situations.  It will at least provide an error message to
the user.

There are no straightforward solution to go around the problem.  The
.gdb-index, .debug_names and .debug_str files could be generated and
then identified by matching *.gdb-index, *.debug_names or *.debug_str.
This would be problematic if other files where to match this pattern.
Generating those index files in a dedicated directory raise the problem
of generating a unique dirname, and mkdtemp is not a standard POSIX
command so we cannot rely on it either.  It could also be possible to
use something like "gdb --batch -nx -iex 'set auto-load no' -iex 'file
$file' -iex 'maint info bfds" and parse its output, but I am not sure
how future-proof this is.

Long story short, I went for the simple option that fixes the reported
issue most of the time instead of trying to come with something
too complicated for limited added-value.  I can change that if
required.

gdb/ChangeLog:

	PR gdb/27614
	* contrib/gdb-add-index.sh: Fix when called with a symlink as an
	argument.

gdb/testsuite/ChangeLog:

	PR gdb/27614
	* gdb.dwarf2/gdb-add-index-symlink.exp: New test.
---
 gdb/contrib/gdb-add-index.sh                  | 15 +++++-
 .../gdb.dwarf2/gdb-add-index-symlink.exp      | 47 +++++++++++++++++++
 2 files changed, 61 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/gdb-add-index-symlink.exp

diff --git a/gdb/contrib/gdb-add-index.sh b/gdb/contrib/gdb-add-index.sh
index 60287b97fba..48a85136a4c 100755
--- a/gdb/contrib/gdb-add-index.sh
+++ b/gdb/contrib/gdb-add-index.sh
@@ -35,7 +35,20 @@ if test $# != 1; then
     exit 1
 fi
 
-file="$1"
+if test -L "$1"; then
+    # This script will most likely fail if the argument is a symlink to a
+    # program.  In order to avoid that, try to follow the symlink.  However,
+    # readlink is not a standard POSIX tool so we have to check if it is
+    # available.
+    if command -v readlink >/dev/null 2>&1; then
+	file="$(readlink -e "$1")"
+    else
+	echo "$myname: unable to follow link: $1" 1>&2
+	exit 1
+    fi
+else
+    file="$1"
+fi
 
 if test ! -r "$file"; then
     echo "$myname: unable to access: $file" 1>&2
diff --git a/gdb/testsuite/gdb.dwarf2/gdb-add-index-symlink.exp b/gdb/testsuite/gdb.dwarf2/gdb-add-index-symlink.exp
new file mode 100644
index 00000000000..eaeddec0250
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/gdb-add-index-symlink.exp
@@ -0,0 +1,47 @@
+# Copyright 2021 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/>.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2.
+if {![dwarf2_support]} {
+    return 0
+}
+
+standard_testfile main.c
+
+if { [prepare_for_testing "failed to prepare" "${testfile}" \
+	  [list ${srcfile}]] } {
+    return -1
+}
+
+set symlink [file dirname $binfile]/symlink
+
+if { ![file exists $symlink] } {
+    file link -symbolic $symlink $binfile
+}
+
+if { [ensure_gdb_index $symlink] == -1 } {
+    fail "Unable to call gdb-add-index with a symlink to a symfile"
+    return -1
+}
+
+# Ok, we have a copy of $binfile with an index.
+# Restart gdb and verify the index was used.
+
+clean_restart $symlink
+gdb_test "mt print objfiles ${testfile}" \
+    "(gdb_index|debug_names).*" \
+    "index used"
-- 
2.30.1


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

* Re: [PATCH v2] [PR gdb/27614] gdb-add-index fails on symlinks.
  2021-03-26 17:11 [PATCH v2] [PR gdb/27614] gdb-add-index fails on symlinks Lancelot SIX
@ 2021-03-26 18:53 ` Simon Marchi
  2021-03-26 19:44   ` Lancelot SIX
  0 siblings, 1 reply; 3+ messages in thread
From: Simon Marchi @ 2021-03-26 18:53 UTC (permalink / raw)
  To: Lancelot SIX, gdb-patches

On 2021-03-26 1:11 p.m., Lancelot SIX via Gdb-patches wrote:
> Since V1:
>         - Replace '&>/dev/null' with '>/dev/null 2>&1'
> 
> --
> 
> PR 27614 shows that gdb-add-index fails to generate the index when its
> argument is a symlink.
> 
> The following one liner illustrates the reported problem:
> 
>         $ echo 'int main(){}'|gcc -g -x c -;ln -s a.out symlink;gdb-add-index symlink
>         gdb-add-index: No index was created for symlink
>         gdb-add-index: [Was there no debuginfo? Was there already an index?]
>         $ ls -l
>         -rwxr-xr-x 1 25712 Mar 19 23:05 a.out*
>         -rw------- 1  8277 Mar 19 23:05 a.out.gdb-index
>         lrwxrwxrwx 1     5 Mar 19 23:05 symlink -> a.out*
> 
> GDB generates the .gdb-index file with a name that matches the name of
> the actual program (a.out.gdb-index here), not the symlink that
> references it.  The remaining of the script is looking for a file named
> after the provided argument (would be 'symlink.gdb-index' in our
> example).
> 
> To fix this, this commit proposes to use 'readlink' when available.  This
> command is not part of the standard POSIX tools, so this fix might not
> work in all situations.  It will at least provide an error message to
> the user.
> 
> There are no straightforward solution to go around the problem.  The
> .gdb-index, .debug_names and .debug_str files could be generated and
> then identified by matching *.gdb-index, *.debug_names or *.debug_str.
> This would be problematic if other files where to match this pattern.
> Generating those index files in a dedicated directory raise the problem
> of generating a unique dirname, and mkdtemp is not a standard POSIX
> command so we cannot rely on it either.  It could also be possible to
> use something like "gdb --batch -nx -iex 'set auto-load no' -iex 'file
> $file' -iex 'maint info bfds" and parse its output, but I am not sure
> how future-proof this is.
> 
> Long story short, I went for the simple option that fixes the reported
> issue most of the time instead of trying to come with something
> too complicated for limited added-value.  I can change that if
> required.
> 
> gdb/ChangeLog:
> 
> 	PR gdb/27614
> 	* contrib/gdb-add-index.sh: Fix when called with a symlink as an
> 	argument.
> 
> gdb/testsuite/ChangeLog:
> 
> 	PR gdb/27614
> 	* gdb.dwarf2/gdb-add-index-symlink.exp: New test.
> ---
>  gdb/contrib/gdb-add-index.sh                  | 15 +++++-
>  .../gdb.dwarf2/gdb-add-index-symlink.exp      | 47 +++++++++++++++++++
>  2 files changed, 61 insertions(+), 1 deletion(-)
>  create mode 100644 gdb/testsuite/gdb.dwarf2/gdb-add-index-symlink.exp
> 
> diff --git a/gdb/contrib/gdb-add-index.sh b/gdb/contrib/gdb-add-index.sh
> index 60287b97fba..48a85136a4c 100755
> --- a/gdb/contrib/gdb-add-index.sh
> +++ b/gdb/contrib/gdb-add-index.sh
> @@ -35,7 +35,20 @@ if test $# != 1; then
>      exit 1
>  fi
>  
> -file="$1"
> +if test -L "$1"; then
> +    # This script will most likely fail if the argument is a symlink to a
> +    # program.  In order to avoid that, try to follow the symlink.  However,
> +    # readlink is not a standard POSIX tool so we have to check if it is
> +    # available.
> +    if command -v readlink >/dev/null 2>&1; then
> +	file="$(readlink -e "$1")"

BSDs' readlink doesn't have the -e option, so it would probably derail
from here (should we also check the exit status of readlink to make sure
it worked?  did you try with a symlink pointing to a non-existent file /
with a non-existent component?).

Would it work with just `readlink <file>`?

Simon


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

* Re: [PATCH v2] [PR gdb/27614] gdb-add-index fails on symlinks.
  2021-03-26 18:53 ` Simon Marchi
@ 2021-03-26 19:44   ` Lancelot SIX
  0 siblings, 0 replies; 3+ messages in thread
From: Lancelot SIX @ 2021-03-26 19:44 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

Le Fri, Mar 26, 2021 at 02:53:12PM -0400, Simon Marchi a écrit :
> BSDs' readlink doesn't have the -e option, so it would probably derail
> from here (should we also check the exit status of readlink to make sure
> it worked?  did you try with a symlink pointing to a non-existent file /
> with a non-existent component?).
> 
> Would it work with just `readlink <file>`?

This would fail if $file is a symlink to a symlink.  This is what
ldconfig usually does (libfoo.so -> libfoo.so.x -> libfoo.so.x.y).

I guess the most robust approach will be something like:

${GDB} --batch -nx -iex 'set auto-load no' \
       -iex "file '$file'" -iex 'maint info bfds' | \
    awk -F/ '{ fname=$NF } END { print fname }'

This rely on how 'maint info bfds' formats its output and I am not
sure how stable shit will be in time, but hopefully the new test case
will catch any breackage.

This will require few more tweaks along the way but nothing too fanncy.
I’ll do that unless there is a simpler and as portable approach
available.

Lancelot.

> 
> Simon
> 

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

end of thread, other threads:[~2021-03-26 19:44 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-26 17:11 [PATCH v2] [PR gdb/27614] gdb-add-index fails on symlinks Lancelot SIX
2021-03-26 18:53 ` Simon Marchi
2021-03-26 19:44   ` Lancelot SIX

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