[PATCH v3] [PR gdb/27614] gdb-add-index fails on symlinks.

[PATCH v3] [PR gdb/27614] gdb-add-index fails on symlinks.

[Lancelot SIX]

Since V2:
        - Use GDB to follow symlink instead of readlink. Unlike
          readlink, GDB is guaranteed to be available.

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

The common option to solve such issue would be to use readlink to follow
the symlink.  Unfortunately, this command is not available in the POSIX
standard.  This commit therefore proposes to use GDB itself to identify
where the symlink points to.  This requires some parsing of GDB output.
The added test should be enough to detect regression if GDB where to
change the way it formats its output.

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                  | 18 ++++++-
 .../gdb.dwarf2/gdb-add-index-symlink.exp      | 47 +++++++++++++++++++
 2 files changed, 64 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 2ac3fddbf26..3f53937e1bb 100755
--- a/gdb/contrib/gdb-add-index.sh
+++ b/gdb/contrib/gdb-add-index.sh
@@ -35,7 +35,23 @@ if test $# != 1; then
     exit 1
 fi
 
-file="$1"
+if test -L "$1"; then
+    # Since readlink is not available in POSIX, use GDB to follow the
+    # link.
+    file=$(${GDB} --batch -nx \
+	-iex 'set auto-load no' \
+	-iex "file '$1'" \
+	-iex 'maint info bfds' |
+	awk -F/ 'BEGIN { OFS=FS } { $1=""; fname=$0 } END { print fname }'|
+	sed 's/ *$//')
+
+    if ! test -f "$file"; then
+	echo "$myname: Failed to follow symlink $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

Re: [PATCH v3] [PR gdb/27614] gdb-add-index fails on symlinks.

[Simon Marchi]

On 2021-03-27 1:27 p.m., Lancelot SIX via Gdb-patches wrote:
> Since V2:
>         - Use GDB to follow symlink instead of readlink. Unlike
>           readlink, GDB is guaranteed to be available.
> 
> 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).
> 
> The common option to solve such issue would be to use readlink to follow
> the symlink.  Unfortunately, this command is not available in the POSIX
> standard.  This commit therefore proposes to use GDB itself to identify
> where the symlink points to.  This requires some parsing of GDB output.
> The added test should be enough to detect regression if GDB where to
> change the way it formats its output.

I preferred your previous approach, compared to relying on a maintenance
command.  Relying on a maintenance command is fine in tests, for example,
but here somebody could use gdb-add-index from a given GDB version with
a GDB of a different version.

  GDB=/my/newer/gdb gdb-add-index a.out

In the previous review, you said:

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

Can't you just call it in a loop then?

  while file is a symlink:
    file=readlink $file

Simon

Re: [PATCH v3] [PR gdb/27614] gdb-add-index fails on symlinks.

[Lancelot SIX]

Le Sun, Mar 28, 2021 at 12:11:22PM -0400, Simon Marchi a écrit :
> 
> 
> On 2021-03-27 1:27 p.m., Lancelot SIX via Gdb-patches wrote:
> > Since V2:
> >         - Use GDB to follow symlink instead of readlink. Unlike
> >           readlink, GDB is guaranteed to be available.
> > 
> > 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).
> > 
> > The common option to solve such issue would be to use readlink to follow
> > the symlink.  Unfortunately, this command is not available in the POSIX
> > standard.  This commit therefore proposes to use GDB itself to identify
> > where the symlink points to.  This requires some parsing of GDB output.
> > The added test should be enough to detect regression if GDB where to
> > change the way it formats its output.
> 
> I preferred your previous approach, compared to relying on a maintenance
> command.  Relying on a maintenance command is fine in tests, for example,
> but here somebody could use gdb-add-index from a given GDB version with
> a GDB of a different version.
> 
>   GDB=/my/newer/gdb gdb-add-index a.out

Yes, agreed. I was reluctant to rely on maint command in the first
place, I should have kept it that way!

> 
> In the previous review, you said:
> 
> >> 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).
> 
> Can't you just call it in a loop then?
> 
>   while file is a symlink:
>     file=readlink $file
> 

Yes. It would look somethink like:


	if test -L "$1"; then
		if command -v readlink >/dev/null 2>&1; then
			file="$1"
			while test -L "$file"; do
				target=$(readlink "$file")
				case "$target" in
					/*)
						file="$target"
						;;
					*)
						file="$(dirname "$file")/$target"
						;;
				esac
			done
		else
			echo "$myname: 'readlink' missing.  Failed to follow symlink $1." 1>&2
			exit 1
		fi
	else
		file="$1"
	fi

I’ll test this and prepare a V4 shortly.

Thanks for the comments.
Lancelot.

> Simon

Re: [PATCH v3] [PR gdb/27614] gdb-add-index fails on symlinks.

[Simon Marchi]

On 2021-03-28 12:56 p.m., Lancelot SIX wrote:
> Le Sun, Mar 28, 2021 at 12:11:22PM -0400, Simon Marchi a écrit :
>>
>>
>> On 2021-03-27 1:27 p.m., Lancelot SIX via Gdb-patches wrote:
>>> Since V2:
>>>         - Use GDB to follow symlink instead of readlink. Unlike
>>>           readlink, GDB is guaranteed to be available.
>>>
>>> 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).
>>>
>>> The common option to solve such issue would be to use readlink to follow
>>> the symlink.  Unfortunately, this command is not available in the POSIX
>>> standard.  This commit therefore proposes to use GDB itself to identify
>>> where the symlink points to.  This requires some parsing of GDB output.
>>> The added test should be enough to detect regression if GDB where to
>>> change the way it formats its output.
>>
>> I preferred your previous approach, compared to relying on a maintenance
>> command.  Relying on a maintenance command is fine in tests, for example,
>> but here somebody could use gdb-add-index from a given GDB version with
>> a GDB of a different version.
>>
>>   GDB=/my/newer/gdb gdb-add-index a.out
> 
> Yes, agreed. I was reluctant to rely on maint command in the first
> place, I should have kept it that way!
> 
>>
>> In the previous review, you said:
>>
>>>> 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).
>>
>> Can't you just call it in a loop then?
>>
>>   while file is a symlink:
>>     file=readlink $file
>>
> 
> Yes. It would look somethink like:
> 
> 
> 	if test -L "$1"; then
> 		if command -v readlink >/dev/null 2>&1; then
> 			file="$1"
> 			while test -L "$file"; do
> 				target=$(readlink "$file")
> 				case "$target" in
> 					/*)
> 						file="$target"
> 						;;
> 					*)
> 						file="$(dirname "$file")/$target"
> 						;;
> 				esac
> 			done
> 		else
> 			echo "$myname: 'readlink' missing.  Failed to follow symlink $1." 1>&2
> 			exit 1
> 		fi
> 	else
> 		file="$1"
> 	fi

Looks good, one minor tweak I suggest is having:

	file=$1

above that and using $file after that, so we refer to $1 only once (it's
less obvious what $1 is than $file).

And maybe exit early if readlink isn't available to avoid one indent:

	if ! command -v readlink >/dev/null 2>&1; then
		echo ...
		exit 1
	fi

	... continue with using readlink ...

Simon