From e19c0f8fb786f149f224f49de41cf02c117e5f93 Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Thu, 4 May 2023 14:48:16 +0100 Subject: [PATCH] Rewrite (and rename) gdb-libexec-add-index.patch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It has been observed that the changes added by gdb-libexec-add-index.patch will result in GDB testing hanging when the tests are being run using an in-tree GDB; that is when using 'make check'. One test that is known to fail is gdb.base/with-mf.exp, though any test that calls the gdb-add-index.sh script will also hang. The problem is that when the gdb-add-index.sh script is run, the GDB testsuite passes the GDB command to use within the GDB environment variable. For in-tree testing this will be something like: GDB="/path/to/gdb -data-directory /path/to/data-directory" Notice that the environment variable contains both an executable and an argument. Our changes to gdb-add-index.sh add this: GDB2=/usr/libexec/gdb if test -x $GDB2 && ! which $GDB &>/dev/null; then GDB=$GDB2 fi The problem then is that '-data-directory' is treated as a set of options to 'which'. Many of these options are not known to 'which', but the '-i' option is known. The documentation of '-i' says: --read-alias, -i Read aliases from stdin, reporting matching ones on stdout. This is useful in combination with using an alias for which itself. For example alias which=´alias | which -i´. And here's the problem; this option causes 'which' to read from stdin. As the GDB testsuite doesn't send any additional input on stdin then the which command will never complete, and the test will hang. The solution I think is to avoid calling 'which' like this on a user supplied GDB environment variable. The changes in the gdb-libexec-add-index.patch were really about what the _default_ GDB executable should be. The upstream version of this script does this: GDB=${GDB:=gdb} That is, the default is just 'gdb'. However, for RH this is not good enough. We want to handle two additional cases, first, when only the gdb-minimal package is installed, in which case the default should be /usr/bin/gdb.minimal. Then we also want to handle the case where the user doesn't have 'gdb' itself in their $PATH, but does have the 'gdb' executable installed in /usr/libexec/gdb. The code as it currently stands also has a problem where, if gdb.minimal is installed on the machine this will _always_ be used in preference to the user supplied GDB value (assuming the code worked at all) this means that when doing in-tree testing we wouldn't actually be using the in-tree GDB to build the index, which isn't ideal. So in this commit I propose that we rework our gdb-add-index.sh changes. Now, we only use the RH special values in the case that there is no GDB environment variable set. I believe this handles all the required use cases: 1. When doing in-tree testing GDB environment variable will be set, and this will always be used as is, with no special processing, 2. When gdb-add-index.sh is used and GDB environment variable is not set then we will use the first of the following as the default: (a) /usr/bin/gdb.minimal if this file exists and is executable, (b) The first gdb executable that can be found in the $PATH, (c) /usr/libexec/gdb if this file exists and is executable. While I was changing this patch anyway I've removed the libexec part of the patch name -- this no longer seemed relevant, I suspect this related to an older version of this patch. --- _gdb.spec.Patch.include | 13 +++++-- _patch_order | 2 +- gdb-add-index.patch | 77 +++++++++++++++++++++++++++++++++++++ gdb-libexec-add-index.patch | 37 ------------------ gdb.spec | 5 +++ 5 files changed, 92 insertions(+), 42 deletions(-) create mode 100644 gdb-add-index.patch delete mode 100644 gdb-libexec-add-index.patch diff --git a/_gdb.spec.Patch.include b/_gdb.spec.Patch.include index 46a73bf..2017068 100644 --- a/_gdb.spec.Patch.include +++ b/_gdb.spec.Patch.include @@ -196,14 +196,19 @@ Patch046: gdb-container-rh-pkg.patch #=fedora Patch047: gdb-linux_perf-bundle.patch -# Fix gdb-headless /usr/bin/ executables (BZ 1390251). +# Update gdb-add-index.sh such that, when the GDB environment +# variable is not set, the script is smarter than just looking for +# 'gdb' in the $PATH. # -# Also, make /usr/bin/gdb.minimal be the default GDB used, if it's -# present. For rationale, see: +# The actual search order is now: /usr/bin/gdb.minimal, gdb (in the +# $PATH), then /usr/libexec/gdb. +# +# For the rationale of looking for gdb.minimal see: # # https://fedoraproject.org/wiki/Changes/Minimal_GDB_in_buildroot +# #=fedora -Patch048: gdb-libexec-add-index.patch +Patch048: gdb-add-index.patch # [s390x] Backport arch12 instructions decoding (RH BZ 1553104). # =fedoratest diff --git a/_patch_order b/_patch_order index fb645b7..e3bb2aa 100644 --- a/_patch_order +++ b/_patch_order @@ -45,7 +45,7 @@ gdb-fedora-libncursesw.patch gdb-rhbz1261564-aarch64-hw-watchpoint-test.patch gdb-container-rh-pkg.patch gdb-linux_perf-bundle.patch -gdb-libexec-add-index.patch +gdb-add-index.patch gdb-rhbz1553104-s390x-arch12-test.patch gdb-rhbz2177655-aarch64-pauth-valid-regcache.patch gdb-rhbz2183595-rustc-inside_main.patch diff --git a/gdb-add-index.patch b/gdb-add-index.patch new file mode 100644 index 0000000..b95e06f --- /dev/null +++ b/gdb-add-index.patch @@ -0,0 +1,77 @@ +From FEDORA_PATCHES Mon Sep 17 00:00:00 2001 +From: Fedora GDB patches +Date: Fri, 27 Oct 2017 21:07:50 +0200 +Subject: gdb-add-index.patch + +;; Update gdb-add-index.sh such that, when the GDB environment +;; variable is not set, the script is smarter than just looking for +;; 'gdb' in the $PATH. +;; +;; The actual search order is now: /usr/bin/gdb.minimal, gdb (in the +;; $PATH), then /usr/libexec/gdb. +;; +;; For the rationale of looking for gdb.minimal see: +;; +;; https://fedoraproject.org/wiki/Changes/Minimal_GDB_in_buildroot +;; +;;=fedora + +diff --git a/gdb/contrib/gdb-add-index.sh b/gdb/contrib/gdb-add-index.sh +--- a/gdb/contrib/gdb-add-index.sh ++++ b/gdb/contrib/gdb-add-index.sh +@@ -16,14 +16,52 @@ + # You should have received a copy of the GNU General Public License + # along with this program. If not, see . + +-# This program assumes gdb and objcopy are in $PATH. +-# If not, or you want others, pass the following in the environment +-GDB=${GDB:=gdb} ++# This program assumes objcopy and readelf are in $PATH. If not, or ++# you want others, pass the following in the environment + OBJCOPY=${OBJCOPY:=objcopy} + READELF=${READELF:=readelf} + + myname="${0##*/}" + ++# For GDB itself we need to be a little smarter. If GDB is set in the ++# environment then we will use that. But if GDB is not set in the ++# environment then we have a couple of options that we need to check ++# through. ++# ++# Our default choice is for /usr/bin/gdb.minimal. For an explanation ++# of why this is chosen, check out: ++# https://bugzilla.redhat.com/show_bug.cgi?id=1695015 ++# https://fedoraproject.org/wiki/Changes/Minimal_GDB_in_buildroot ++# ++# If gdb.minimal is not found then we look for a 'gdb' executable on ++# the path. ++# ++# And finally, we check for /usr/libexec/gdb. ++# ++# If none of those result in a useable GDB then we give an error and ++# exit. ++if test -z "$GDB"; then ++ for possible_gdb in /usr/bin/gdb.minimal gdb /usr/libexec/gdb; do ++ if ! which "$possible_gdb" 2>/dev/null; then ++ continue ++ fi ++ ++ possible_gdb=$(which "$possible_gdb") ++ ++ if ! test -x "$possible_gdb"; then ++ continue ++ fi ++ ++ GDB="$possible_gdb" ++ break ++ done ++ ++ if test -z "$GDB"; then ++ echo "$myname: Failed to find a useable GDB binary" 1>&2 ++ exit 1 ++ fi ++fi ++ + dwarf5="" + if [ "$1" = "-dwarf-5" ]; then + dwarf5="$1" diff --git a/gdb-libexec-add-index.patch b/gdb-libexec-add-index.patch deleted file mode 100644 index 19f6b54..0000000 --- a/gdb-libexec-add-index.patch +++ /dev/null @@ -1,37 +0,0 @@ -From FEDORA_PATCHES Mon Sep 17 00:00:00 2001 -From: Fedora GDB patches -Date: Fri, 27 Oct 2017 21:07:50 +0200 -Subject: gdb-libexec-add-index.patch - -;; Fix gdb-headless /usr/bin/ executables (BZ 1390251). -;; -;; Also, make /usr/bin/gdb.minimal be the default GDB used, if it's -;; present. For rationale, see: -;; -;; https://fedoraproject.org/wiki/Changes/Minimal_GDB_in_buildroot -;;=fedora - -diff --git a/gdb/contrib/gdb-add-index.sh b/gdb/contrib/gdb-add-index.sh ---- a/gdb/contrib/gdb-add-index.sh -+++ b/gdb/contrib/gdb-add-index.sh -@@ -22,6 +22,20 @@ GDB=${GDB:=gdb} - OBJCOPY=${OBJCOPY:=objcopy} - READELF=${READELF:=readelf} - -+GDB2=/usr/libexec/gdb -+if test -x $GDB2 && ! which $GDB &>/dev/null; then -+ GDB=$GDB2 -+fi -+ -+# We default to using /usr/bin/gdb.minimal if it's present. See -+# https://bugzilla.redhat.com/show_bug.cgi?id=1695015 and -+# https://fedoraproject.org/wiki/Changes/Minimal_GDB_in_buildroot for -+# explanations. -+GDB3=/usr/bin/gdb.minimal -+if test -x $GDB3; then -+ GDB=$GDB3 -+fi -+ - myname="${0##*/}" - - dwarf5="" diff --git a/gdb.spec b/gdb.spec index 458e3e4..482b542 100644 --- a/gdb.spec +++ b/gdb.spec @@ -1249,6 +1249,11 @@ fi %endif %changelog +* Thu May 4 2023 Andrew Burgess +- Rewrite the changes to gdb-add-index.sh. If the user has set the + GDB environment variable then use that value, otherwise find a + suitable GDB executable by looking in various places. + * Wed May 3 2023 Kevin Buettner 13.1-5 - Backport "Pass const frame_info_ptr reference for skip_[language_]trampoline". (Mark Wielaard, RHBZ 2192105, build/30413)