[PATCH] Fix quoting in gdb-add-index.sh

[PATCH] Fix quoting in gdb-add-index.sh

[Tom Tromey]

When the filename quoting change was merged into the AdaCore tree, we
saw a regression in a test setup that uses the DWARF 5 index (that is
running gdb-add-index), and a filename with a space in it.

Initially I thought this was a change in the 'file' command -- but
looking again, I found out that 'file' has worked this way for a
while, and our immediate error was caused by the (documented) change
to "save gdb-index".

While I'm not sure why this test was working previously, it seems to
me that gdb-add-index.sh requires a change to quote the arguments to
"file" and "save gdb-index".

While working on this, though, it seemed to me that multiple other
spots needed quoting for the script to work correctly.  And, I was
unable to get quoting working correctly in the objcopy calls, so I
split it into multiple different invocations.
---
 gdb/contrib/gdb-add-index.sh | 39 +++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/gdb/contrib/gdb-add-index.sh b/gdb/contrib/gdb-add-index.sh
index 80bc4956358..bd5a8f75973 100755
--- a/gdb/contrib/gdb-add-index.sh
+++ b/gdb/contrib/gdb-add-index.sh
@@ -113,7 +113,7 @@ trap "rm -f $tmp_files" 0
 
 $GDB --batch -nx -iex 'set auto-load no' \
     -iex 'set debuginfod enabled off' \
-    -ex "file $file" -ex "save gdb-index $dwarf5 $dir" || {
+    -ex "file '$file'" -ex "save gdb-index $dwarf5 '$dir'" || {
     # Just in case.
     status=$?
     echo "$myname: gdb error generating index for $file" 1>&2
@@ -143,35 +143,32 @@ handle_file ()
 	    index="$index5"
 	    section=".debug_names"
 	fi
-	debugstradd=false
-	debugstrupdate=false
 	if test -s "$debugstr"; then
 	    if ! $OBJCOPY --dump-section .debug_str="$debugstrmerge" "$fpath" \
-		 /dev/null 2>$debugstrerr; then
-		cat >&2 $debugstrerr
+		 /dev/null 2> "$debugstrerr"; then
+		cat >&2 "$debugstrerr"
 		exit 1
 	    fi
+	    cat "$debugstr" >>"$debugstrmerge"
 	    if grep -q "can't dump section '.debug_str' - it does not exist" \
-		    $debugstrerr; then
-		debugstradd=true
+		    "$debugstrerr"; then
+		$OBJCOPY --add-section $section="$index" \
+			 --set-section-flags $section=readonly \
+			 --add-section .debug_str="$debugstrmerge" \
+		         --set-section-flags .debug_str=readonly \
+			 "$fpath" "$fpath"
 	    else
-		debugstrupdate=true
-		cat >&2 $debugstrerr
+		$OBJCOPY --add-section $section="$index" \
+			 --set-section-flags $section=readonly \
+			 --update-section .debug_str="$debugstrmerge" \
+			 "$fpath" "$fpath"
 	    fi
-	    cat "$debugstr" >>"$debugstrmerge"
+	else
+	    $OBJCOPY --add-section $section="$index" \
+		     --set-section-flags $section=readonly \
+		     "$fpath" "$fpath"
 	fi
 
-	$OBJCOPY --add-section $section="$index" \
-		 --set-section-flags $section=readonly \
-		 $(if $debugstradd; then \
-		       echo --add-section .debug_str="$debugstrmerge"; \
-		       echo --set-section-flags .debug_str=readonly; \
-		   fi; \
-		   if $debugstrupdate; then \
-		       echo --update-section .debug_str="$debugstrmerge"; \
-		   fi) \
-		 "$fpath" "$fpath"
-
 	status=$?
     else
 	echo "$myname: No index was created for $fpath" 1>&2
-- 
2.45.0

Re: [PATCH] Fix quoting in gdb-add-index.sh

[Kevin Buettner]

On Wed, 11 Sep 2024 08:34:26 -0600
Tom Tromey <tromey@adacore.com> wrote:

> When the filename quoting change was merged into the AdaCore tree, we
> saw a regression in a test setup that uses the DWARF 5 index (that is
> running gdb-add-index), and a filename with a space in it.
> 
> Initially I thought this was a change in the 'file' command -- but
> looking again, I found out that 'file' has worked this way for a
> while, and our immediate error was caused by the (documented) change
> to "save gdb-index".
> 
> While I'm not sure why this test was working previously, it seems to
> me that gdb-add-index.sh requires a change to quote the arguments to
> "file" and "save gdb-index".
> 
> While working on this, though, it seemed to me that multiple other
> spots needed quoting for the script to work correctly.  And, I was
> unable to get quoting working correctly in the objcopy calls, so I
> split it into multiple different invocations.

FWIW, I think it's now easier to read with separate invocations.

Approved-by: Kevin Buettner <kevinb@redhat.com>