From dd4bbfd655bbe0324288d5e922dcae7c5e7f5904 Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Thu, 4 May 2023 14:11:19 +0000 Subject: [PATCH 1/3] 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 13eb704..a514977 100644 --- a/_gdb.spec.Patch.include +++ b/_gdb.spec.Patch.include @@ -237,14 +237,19 @@ Patch056: gdb-container-rh-pkg.patch #=fedora Patch057: 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 -Patch058: gdb-libexec-add-index.patch +Patch058: gdb-add-index.patch # [s390x] Backport arch12 instructions decoding (RH BZ 1553104). # =fedoratest diff --git a/_patch_order b/_patch_order index 60e004b..9aa6157 100644 --- a/_patch_order +++ b/_patch_order @@ -55,7 +55,7 @@ gdb-opcodes-clflushopt-test.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 3fb8247..2d4e04c 100644 --- a/gdb.spec +++ b/gdb.spec @@ -1192,6 +1192,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-4 - Backport "Pass const frame_info_ptr reference for skip_[language_]trampoline". (Mark Wielaard, RHBZ 2192105, build/30413) From b00a04230b31c9fb4e824ec3418432e14afb5efb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandra=20H=C3=A1jkov=C3=A1?= Date: Thu, 8 Jun 2023 22:52:37 +0200 Subject: [PATCH 2/3] Rebase to FSF GDB 13.2. Update gdb-6.3-rh-testversion-20041202.patch. Remove gdb-rhbz2177655-aarch64-pauth-valid-regcache.patch. Remove gdb-rhbz2183595-rustc-inside_main.patch. --- .gitignore | 1 + _gdb.spec.Patch.include | 11 +- _gdb.spec.patch.include | 2 - _git_upstream_commit | 2 +- _patch_order | 2 - gdb-6.3-rh-testversion-20041202.patch | 2 +- ...2177655-aarch64-pauth-valid-regcache.patch | 279 ------------------ gdb-rhbz2183595-rustc-inside_main.patch | 136 --------- gdb.spec | 10 +- sources | 2 +- 10 files changed, 14 insertions(+), 433 deletions(-) delete mode 100644 gdb-rhbz2177655-aarch64-pauth-valid-regcache.patch delete mode 100644 gdb-rhbz2183595-rustc-inside_main.patch diff --git a/.gitignore b/.gitignore index 59fdeb1..b4b0840 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ /gdb-libstdc++-v3-python-8.1.1-20180626.tar.xz /v2.0.5.tar.gz /gdb-13.1.tar.xz +/gdb-13.2.tar.xz diff --git a/_gdb.spec.Patch.include b/_gdb.spec.Patch.include index a514977..819df04 100644 --- a/_gdb.spec.Patch.include +++ b/_gdb.spec.Patch.include @@ -255,18 +255,11 @@ Patch058: gdb-add-index.patch # =fedoratest Patch059: gdb-rhbz1553104-s390x-arch12-test.patch -# [aarch64] Backport fix from Luis Machado for RH BZ 2177655. -Patch060: gdb-rhbz2177655-aarch64-pauth-valid-regcache.patch - -# Backport "Fix crash in inside_main_func" -# (Tom Tromey, RHBZ 2183595) -Patch061: gdb-rhbz2183595-rustc-inside_main.patch - # Backport "Fix a potential illegal memory access in the BFD library..." # (Nick Clifton, binutils/29988) -Patch062: gdb-binutils29988-read_indexed_address.patch +Patch060: gdb-binutils29988-read_indexed_address.patch # Backport upstream patch fixing a "dangling pointer" build problem # first seen when building with GCC 13.1.1 20230426 (Red Hat ;; 13.1.1-1). -Patch063: gdb-rhbz2192105-ftbs-dangling-pointer +Patch061: gdb-rhbz2192105-ftbs-dangling-pointer diff --git a/_gdb.spec.patch.include b/_gdb.spec.patch.include index 7d2cc95..12bf93a 100644 --- a/_gdb.spec.patch.include +++ b/_gdb.spec.patch.include @@ -59,5 +59,3 @@ %patch059 -p1 %patch060 -p1 %patch061 -p1 -%patch062 -p1 -%patch063 -p1 diff --git a/_git_upstream_commit b/_git_upstream_commit index 550f43d..26ed85a 100644 --- a/_git_upstream_commit +++ b/_git_upstream_commit @@ -1 +1 @@ -4f3e26ac6ee31f7bc4b04abd8bdb944e7f1fc5d2 +662243de0e14a4945555a480dca33c0e677976eb diff --git a/_patch_order b/_patch_order index 9aa6157..263476f 100644 --- a/_patch_order +++ b/_patch_order @@ -57,7 +57,5 @@ gdb-container-rh-pkg.patch gdb-linux_perf-bundle.patch gdb-add-index.patch gdb-rhbz1553104-s390x-arch12-test.patch -gdb-rhbz2177655-aarch64-pauth-valid-regcache.patch -gdb-rhbz2183595-rustc-inside_main.patch gdb-binutils29988-read_indexed_address.patch gdb-rhbz2192105-ftbs-dangling-pointer diff --git a/gdb-6.3-rh-testversion-20041202.patch b/gdb-6.3-rh-testversion-20041202.patch index 718e53a..b6b8e29 100644 --- a/gdb-6.3-rh-testversion-20041202.patch +++ b/gdb-6.3-rh-testversion-20041202.patch @@ -27,7 +27,7 @@ diff --git a/gdb/testsuite/gdb.gdb/selftest.exp b/gdb/testsuite/gdb.gdb/selftest diff --git a/gdb/top.c b/gdb/top.c --- a/gdb/top.c +++ b/gdb/top.c -@@ -2382,7 +2382,7 @@ The second argument is the terminal the UI runs on."), &cmdlist); +@@ -2384,7 +2384,7 @@ The second argument is the terminal the UI runs on."), &cmdlist); struct internalvar *major_version_var = create_internalvar ("_gdb_major"); struct internalvar *minor_version_var = create_internalvar ("_gdb_minor"); int vmajor = 0, vminor = 0, vrevision = 0; diff --git a/gdb-rhbz2177655-aarch64-pauth-valid-regcache.patch b/gdb-rhbz2177655-aarch64-pauth-valid-regcache.patch deleted file mode 100644 index 4f72647..0000000 --- a/gdb-rhbz2177655-aarch64-pauth-valid-regcache.patch +++ /dev/null @@ -1,279 +0,0 @@ -From FEDORA_PATCHES Mon Sep 17 00:00:00 2001 -From: Kevin Buettner -Date: Fri, 24 Mar 2023 15:26:57 -0700 -Subject: gdb-rhbz2177655-aarch64-pauth-valid-regcache.patch - -;; [aarch64] Backport fix from Luis Machado for RH BZ 2177655. - -aarch64: Check for valid inferior thread/regcache before reading pauth registers - -There were reports of gdb throwing internal errors when calling -inferior_thread ()/get_current_regcache () on a system with -Pointer Authentication enabled. - -In such cases, gdb produces the following backtrace, or a variation -of it (for gdb's with the non-address removal implemented only in -the aarch64-linux-tdep.c file). - -../../../repos/binutils-gdb/gdb/thread.c:86: internal-error: inferior_thread: Assertion `current_thread_ != nullptr' failed. -A problem internal to GDB has been detected, -further debugging may prove unreliable. ------ Backtrace ----- -0xaaaae04a571f gdb_internal_backtrace_1 - ../../../repos/binutils-gdb/gdb/bt-utils.c:122 -0xaaaae04a57f3 _Z22gdb_internal_backtracev - ../../../repos/binutils-gdb/gdb/bt-utils.c:168 -0xaaaae0b52ccf internal_vproblem - ../../../repos/binutils-gdb/gdb/utils.c:401 -0xaaaae0b5310b _Z15internal_verrorPKciS0_St9__va_list - ../../../repos/binutils-gdb/gdb/utils.c:481 -0xaaaae0e24b8f _Z18internal_error_locPKciS0_z - ../../../repos/binutils-gdb/gdbsupport/errors.cc:58 -0xaaaae0a88983 _Z15inferior_threadv - ../../../repos/binutils-gdb/gdb/thread.c:86 -0xaaaae0956c87 _Z20get_current_regcachev - ../../../repos/binutils-gdb/gdb/regcache.c:428 -0xaaaae035223f aarch64_remove_non_address_bits - ../../../repos/binutils-gdb/gdb/aarch64-tdep.c:3572 -0xaaaae03e8abb _Z31gdbarch_remove_non_address_bitsP7gdbarchm - ../../../repos/binutils-gdb/gdb/gdbarch.c:3109 -0xaaaae0a692d7 memory_xfer_partial - ../../../repos/binutils-gdb/gdb/target.c:1620 -0xaaaae0a695e3 _Z19target_xfer_partialP10target_ops13target_objectPKcPhPKhmmPm - ../../../repos/binutils-gdb/gdb/target.c:1684 -0xaaaae0a69e9f target_read_partial - ../../../repos/binutils-gdb/gdb/target.c:1937 -0xaaaae0a69fdf _Z11target_readP10target_ops13target_objectPKcPhml - ../../../repos/binutils-gdb/gdb/target.c:1977 -0xaaaae0a69937 _Z18target_read_memorymPhl - ../../../repos/binutils-gdb/gdb/target.c:1773 -0xaaaae08be523 ps_xfer_memory - ../../../repos/binutils-gdb/gdb/proc-service.c:90 -0xaaaae08be6db ps_pdread - ../../../repos/binutils-gdb/gdb/proc-service.c:124 -0x40001ed7c3b3 _td_fetch_value - /build/glibc-RIFKjK/glibc-2.31/nptl_db/fetch-value.c:115 -0x40001ed791ef td_ta_map_lwp2thr - /build/glibc-RIFKjK/glibc-2.31/nptl_db/td_ta_map_lwp2thr.c:194 -0xaaaae07f4473 thread_from_lwp - ../../../repos/binutils-gdb/gdb/linux-thread-db.c:413 -0xaaaae07f6d6f _ZN16thread_db_target4waitE6ptid_tP17target_waitstatus10enum_flagsI16target_wait_flagE - ../../../repos/binutils-gdb/gdb/linux-thread-db.c:1420 -0xaaaae0a6b33b _Z11target_wait6ptid_tP17target_waitstatus10enum_flagsI16target_wait_flagE - ../../../repos/binutils-gdb/gdb/target.c:2586 -0xaaaae0789cf7 do_target_wait_1 - ../../../repos/binutils-gdb/gdb/infrun.c:3825 -0xaaaae0789e6f operator() - ../../../repos/binutils-gdb/gdb/infrun.c:3884 -0xaaaae078a167 do_target_wait - ../../../repos/binutils-gdb/gdb/infrun.c:3903 -0xaaaae078b0af _Z20fetch_inferior_eventv - ../../../repos/binutils-gdb/gdb/infrun.c:4314 -0xaaaae076652f _Z22inferior_event_handler19inferior_event_type - ../../../repos/binutils-gdb/gdb/inf-loop.c:41 -0xaaaae07dc68b handle_target_event - ../../../repos/binutils-gdb/gdb/linux-nat.c:4206 -0xaaaae0e25fbb handle_file_event - ../../../repos/binutils-gdb/gdbsupport/event-loop.cc:573 -0xaaaae0e264f3 gdb_wait_for_event - ../../../repos/binutils-gdb/gdbsupport/event-loop.cc:694 -0xaaaae0e24f9b _Z16gdb_do_one_eventi - ../../../repos/binutils-gdb/gdbsupport/event-loop.cc:217 -0xaaaae080f033 start_event_loop - ../../../repos/binutils-gdb/gdb/main.c:411 -0xaaaae080f1b7 captured_command_loop - ../../../repos/binutils-gdb/gdb/main.c:475 -0xaaaae0810b97 captured_main - ../../../repos/binutils-gdb/gdb/main.c:1318 -0xaaaae0810c1b _Z8gdb_mainP18captured_main_args - ../../../repos/binutils-gdb/gdb/main.c:1337 -0xaaaae0338453 main - ../../../repos/binutils-gdb/gdb/gdb.c:32 ---------------------- -../../../repos/binutils-gdb/gdb/thread.c:86: internal-error: inferior_thread: Assertion `current_thread_ != nullptr' failed. -A problem internal to GDB has been detected, -further debugging may prove unreliable. -Quit this debugging session? (y or n) - -We also see failures across the testsuite if the tests get executed on a target -that has native support for the pointer authentication feature. But -gdb.base/break.exp and gdb.base/access-mem-running.exp are two examples of -tests that run into errors and internal errors. - -This issue started after commit d88cb738e6a7a7179dfaff8af78d69250c852af1, which -enabled more broad use of pointer authentication masks to remove non-address -bits of pointers, but wasn't immediately detected because systems with native -support for pointer authentication are not that common yet. - -The above crash happens because gdb is in the middle of handling an event, -and do_target_wait_1 calls switch_to_inferior_no_thread, nullifying the -current thread. This means a call to inferior_thread () will assert, and -attempting to call get_current_regcache () will also call inferior_thread (), -resulting in an assertion as well. - -target_has_registers was one function that seemed useful for detecting these -types of situation where we don't have a register cache. The problem with that -is the inconsistent state of inferior_ptid, which is used by -target_has_registers. - -Despite the call to switch_to_no_thread in switch_to_inferior_no_thread from -do_target_wait_1 in the backtrace above clearing inferior_ptid, the call to -ps_xfer_memory sets inferior_ptid momentarily before reading memory: - -static ps_err_e -ps_xfer_memory (const struct ps_prochandle *ph, psaddr_t addr, - gdb_byte *buf, size_t len, int write) -{ - scoped_restore_current_inferior restore_inferior; - set_current_inferior (ph->thread->inf); - - scoped_restore_current_program_space restore_current_progspace; - set_current_program_space (ph->thread->inf->pspace); - - scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid); - inferior_ptid = ph->thread->ptid; - - CORE_ADDR core_addr = ps_addr_to_core_addr (addr); - - int ret; - if (write) - ret = target_write_memory (core_addr, buf, len); - else - ret = target_read_memory (core_addr, buf, len); - return (ret == 0 ? PS_OK : PS_ERR); -} - -Maybe this shouldn't happen, or maybe it is just an unfortunate state to be -in. But this prevents the use of target_has_registers to guard against the -lack of registers, since, although current_thread_ is still nullptr, -inferior_ptid is valid and is not null_ptid. - -There is another crash scenario after we kill a previously active inferior, in -which case the gdbarch will still say we support pointer authentication but we -will also have no current thread (inferior_thread () will assert etc). - -If the target has support for pointer authentication, gdb needs to use -a couple (or 4, for bare-metal) mask registers to mask off some bits of -pointers, and for that it needs to access the registers. - -At some points, like the one from the backtrace above, there is no active -thread/current regcache because gdb is in the middle of doing event handling -and switching between threads. - -Simon suggested the use of inferior_ptid to fetch the register cache, as -opposed to relying on the current register cache. Though we need to make sure -inferior_ptid is valid (not null_ptid), I think this works nicely. - -With inferior_ptid, we can do safety checks along the way, making sure we have -a thread to fetch a register cache from and checking if the thread is actually -stopped or running. - -The following patch implements this idea with safety checks to make sure we -don't run into assertions or errors. If any of the checks fail, we fallback to -using a default mask to remove non-address bits of a pointer. - -I discussed with Pedro the possibility of caching the mask register values -(which are per-process and can change mid-execution), but there isn't a good -spot to cache those values. Besides, the mask registers can change constantly -for bare-metal debugging when switching between exception levels. - -In some cases, it is just not possible to get access to these mask registers, -like the case where threads are running. In those cases, using a default mask -to remove the non-address bits should be enough. - -This can happen when we let threads run in the background and then we attempt -to access a memory address (now that gdb is capable of reading memory even -with threads running). Thus gdb will attempt to remove non-address bits -of that memory access, will attempt to access registers, running into errors. - -Regression-tested on aarch64-linux Ubuntu 20.04. - -diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c ---- a/gdb/aarch64-linux-tdep.c -+++ b/gdb/aarch64-linux-tdep.c -@@ -57,6 +57,9 @@ - #include "elf/common.h" - #include "elf/aarch64.h" - -+/* For inferior_ptid and current_inferior (). */ -+#include "inferior.h" -+ - /* Signal frame handling. - - +------------+ ^ -@@ -1986,29 +1989,60 @@ aarch64_linux_decode_memtag_section (struct gdbarch *gdbarch, - static CORE_ADDR - aarch64_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer) - { -- aarch64_gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); -- - /* By default, we assume TBI and discard the top 8 bits plus the VA range -- select bit (55). */ -+ select bit (55). Below we try to fetch information about pointer -+ authentication masks in order to make non-address removal more -+ precise. */ - CORE_ADDR mask = AARCH64_TOP_BITS_MASK; - -- if (tdep->has_pauth ()) -+ /* Check if we have an inferior first. If not, just use the default -+ mask. -+ -+ We use the inferior_ptid here because the pointer authentication masks -+ should be the same across threads of a process. Since we may not have -+ access to the current thread (gdb may have switched to no inferiors -+ momentarily), we use the inferior ptid. */ -+ if (inferior_ptid != null_ptid) - { -- /* Fetch the PAC masks. These masks are per-process, so we can just -- fetch data from whatever thread we have at the moment. -+ /* If we do have an inferior, attempt to fetch its thread's thread_info -+ struct. */ -+ thread_info *thread -+ = find_thread_ptid (current_inferior ()->process_target (), -+ inferior_ptid); - -- Also, we have both a code mask and a data mask. For now they are the -- same, but this may change in the future. */ -- struct regcache *regs = get_current_regcache (); -- CORE_ADDR cmask, dmask; -+ /* If the thread is running, we will not be able to fetch the mask -+ registers. */ -+ if (thread != nullptr && thread->state != THREAD_RUNNING) -+ { -+ /* Otherwise, fetch the register cache and the masks. */ -+ struct regcache *regs -+ = get_thread_regcache (current_inferior ()->process_target (), -+ inferior_ptid); -+ -+ /* Use the gdbarch from the register cache to check for pointer -+ authentication support, as it matches the features found in -+ that particular thread. */ -+ aarch64_gdbarch_tdep *tdep -+ = gdbarch_tdep (regs->arch ()); -+ -+ /* Is there pointer authentication support? */ -+ if (tdep->has_pauth ()) -+ { -+ /* We have both a code mask and a data mask. For now they are -+ the same, but this may change in the future. */ -+ CORE_ADDR cmask, dmask; - -- if (regs->cooked_read (tdep->pauth_reg_base, &dmask) != REG_VALID) -- dmask = mask; -+ if (regs->cooked_read (tdep->pauth_reg_base, &dmask) -+ != REG_VALID) -+ dmask = mask; - -- if (regs->cooked_read (tdep->pauth_reg_base + 1, &cmask) != REG_VALID) -- cmask = mask; -+ if (regs->cooked_read (tdep->pauth_reg_base + 1, &cmask) -+ != REG_VALID) -+ cmask = mask; - -- mask |= aarch64_mask_from_pac_registers (cmask, dmask); -+ mask |= aarch64_mask_from_pac_registers (cmask, dmask); -+ } -+ } - } - - return aarch64_remove_top_bits (pointer, mask); diff --git a/gdb-rhbz2183595-rustc-inside_main.patch b/gdb-rhbz2183595-rustc-inside_main.patch deleted file mode 100644 index 16a77b6..0000000 --- a/gdb-rhbz2183595-rustc-inside_main.patch +++ /dev/null @@ -1,136 +0,0 @@ -From FEDORA_PATCHES Mon Sep 17 00:00:00 2001 -From: Tom Tromey -Date: Fri, 24 Feb 2023 10:40:16 -0700 -Subject: gdb-rhbz2183595-rustc-inside_main.patch - -;; Backport "Fix crash in inside_main_func" -;; (Tom Tromey, RHBZ 2183595) - -gdb 13.1 crashes while running the rust compiler's debugger tests. -The crash has a number of causes. - -First, the rust compiler still uses the C++-like _Z mangling, but with -its own twist -- some hex digits added to the end of a symbol. So, -while gdb finds the correct name of "main": - -(top-gdb) p name -$13 = 0x292e0c0 "rustc_gdb_1031745::main" - -It isn't found in the minsyms, because C++ demangling yields: - -[99] t 0x90c0 _ZN17rustc_gdb_10317454main17h5b5be7fe16a97225E section .text rustc_gdb_1031745::main::h5b5be7fe16a97225 zko06yobckx336v - -This could perhaps be fixed. I also filed a new PR to suggest -preferring the linkage name of the main program. - -Next, the rust compiler emits both a DW_TAG_subprogram and a -DW_TAG_namespace for "main". This happens because the file is named -"main.rs" -- i.e., the bug is specific to the source file name. The -crash also seems to require the nested function inside of 'main', at -least for me. The namespace always is generated, but perhaps this -changes the ordering in the DWARF. - -When inside_main_func looks up the main symbol, it finds the namespace -symbol rather than the function. (I filed a bug about fixing gdb's -symbol tables -- long overdue.) - -Meanwhile, as I think it's important to fix this crash sooner rather -than later, this patch changes inside_main_func to check that the -symbol that is found is LOC_BLOCK. This perhaps should have been done -in the first place, anyway. - -Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30158 - -diff --git a/gdb/frame.c b/gdb/frame.c ---- a/gdb/frame.c -+++ b/gdb/frame.c -@@ -2453,6 +2453,14 @@ inside_main_func (frame_info_ptr this_frame) - if (bs.symbol == nullptr) - return false; - -+ /* We might have found some unrelated symbol. For example, the -+ Rust compiler can emit both a subprogram and a namespace with -+ the same name in the same scope; and due to how gdb's symbol -+ tables currently work, we can't request the one we'd -+ prefer. */ -+ if (bs.symbol->aclass () != LOC_BLOCK) -+ return false; -+ - const struct block *block = bs.symbol->value_block (); - gdb_assert (block != nullptr); - sym_addr = block->start (); -diff --git a/gdb/testsuite/gdb.rust/main-crash.exp b/gdb/testsuite/gdb.rust/main-crash.exp -new file mode 100644 ---- /dev/null -+++ b/gdb/testsuite/gdb.rust/main-crash.exp -@@ -0,0 +1,35 @@ -+# Copyright (C) 2023 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 . -+ -+# Regression test for a crash in inside_main_func. -+ -+load_lib rust-support.exp -+require allow_rust_tests -+ -+standard_testfile main.rs -+if {[prepare_for_testing "failed to prepare" $testfile $srcfile \ -+ {debug rust}]} { -+ return -1 -+} -+ -+set line [gdb_get_line_number "BREAK"] -+# The bug was that this would crash. -+if {![runto ${srcfile}:$line]} { -+ untested "could not run to breakpoint" -+ return -1 -+} -+ -+# Test that gdb is alive. -+gdb_test "print 23" " = 23" -diff --git a/gdb/testsuite/gdb.rust/main.rs b/gdb/testsuite/gdb.rust/main.rs -new file mode 100644 ---- /dev/null -+++ b/gdb/testsuite/gdb.rust/main.rs -@@ -0,0 +1,30 @@ -+// Copyright (C) 2016-2023 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 . -+ -+#![allow(dead_code)] -+#![allow(unused_variables)] -+#![allow(unused_assignments)] -+ -+fn global_fn(x: u8) { -+ // BREAK -+} -+ -+fn main() { -+ fn nested(y: u8) { -+ global_fn(y) -+ } -+ -+ nested(23); -+} diff --git a/gdb.spec b/gdb.spec index 2d4e04c..ca0af3b 100644 --- a/gdb.spec +++ b/gdb.spec @@ -53,11 +53,11 @@ Name: %{?scl_prefix}gdb # See timestamp of source gnulib installed into gnulib/ . %global snapgnulib 20220501 %global tarname gdb-%{version} -Version: 13.1 +Version: 13.2 # The release always contains a leading reserved number, start it at 1. # `upstream' is not a part of `name' to stay fully rpm dependencies compatible for the testing. -Release: 4%{?dist} +Release: 1%{?dist} License: GPLv3+ and GPLv3+ with exceptions and GPLv2+ and GPLv2+ with exceptions and GPL+ and LGPLv2+ and LGPLv3+ and BSD and Public Domain and GFDL # Do not provide URL for snapshots as the file lasts there only for 2 days. @@ -1192,6 +1192,12 @@ fi %endif %changelog +* Thu Jun 22 2023 Alexandra Hájková - 13.1-2 +- Rebase to FSF GDB 13.2. +- Update gdb-6.3-rh-testversion-20041202.patch. +- Remove gdb-rhbz2177655-aarch64-pauth-valid-regcache.patch. +- Remove gdb-rhbz2183595-rustc-inside_main.patch. + * 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 diff --git a/sources b/sources index 7f7f1ed..bac9830 100644 --- a/sources +++ b/sources @@ -1,3 +1,3 @@ SHA512 (gdb-libstdc++-v3-python-8.1.1-20180626.tar.xz) = a8b1c54dd348cfeb37da73f968742896be3dd13a4215f8d8519870c2abea915f5176c3fa6989ddd10f20020a16f0fab20cbae68ee8d58a82234d8778023520f8 SHA512 (v2.0.5.tar.gz) = 2e7ac2aede84671b15597d9c56dbe077a81357bbf44b6684802592246fb7729b4a5743238ddf02f6ea143b4d29872f581408135f9c1ea1ccc99dab905916d98d -SHA512 (gdb-13.1.tar.xz) = e65054ffbc0357eeed4b17e1edc5ef45aa73c9ddf3b1210651e3d859576e27c1d27b266800fe26328eda58857455ccd8632f4000cfc5f63f90854096290187ca +SHA512 (gdb-13.2.tar.xz) = 8185d3e11ab60dafff5860a5016577bfe7dd7547ef01ebc867bc247603d82b74ff74c4f29492c7d2aee57076f52be33e289f4c6b414a4b870d4b3004909f4c34 From ef8cdc2db87c2ce23dd7fe53df208d531e596838 Mon Sep 17 00:00:00 2001 From: Kevin Buettner Date: Fri, 30 Jun 2023 17:58:51 -0700 Subject: [PATCH 3/3] Suppress repeated warnings when loading a core file Backport upstream changes which prevent repeated warnings from being printed when loading a core file (RHBZ 2160211, Lancelot SIX). --- _gdb.spec.Patch.include | 5 + _gdb.spec.patch.include | 1 + _patch_order | 1 + ...2160211-excessive-core-file-warnings.patch | 108 ++++++++++++++++++ gdb.spec | 8 +- 5 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 gdb-rhbz2160211-excessive-core-file-warnings.patch diff --git a/_gdb.spec.Patch.include b/_gdb.spec.Patch.include index 819df04..43f5e1d 100644 --- a/_gdb.spec.Patch.include +++ b/_gdb.spec.Patch.include @@ -263,3 +263,8 @@ Patch060: gdb-binutils29988-read_indexed_address.patch # first seen when building with GCC 13.1.1 20230426 (Red Hat ;; 13.1.1-1). Patch061: gdb-rhbz2192105-ftbs-dangling-pointer +# Backport two commits, 0ad504dd464 and ea70f941f9b, from Lancelot SIX +# which prevent repeated warnings from being printed while loading a +# core file. (RH BZ 2160211) +Patch062: gdb-rhbz2160211-excessive-core-file-warnings.patch + diff --git a/_gdb.spec.patch.include b/_gdb.spec.patch.include index 12bf93a..76f988a 100644 --- a/_gdb.spec.patch.include +++ b/_gdb.spec.patch.include @@ -59,3 +59,4 @@ %patch059 -p1 %patch060 -p1 %patch061 -p1 +%patch062 -p1 diff --git a/_patch_order b/_patch_order index 263476f..ef42abe 100644 --- a/_patch_order +++ b/_patch_order @@ -59,3 +59,4 @@ gdb-add-index.patch gdb-rhbz1553104-s390x-arch12-test.patch gdb-binutils29988-read_indexed_address.patch gdb-rhbz2192105-ftbs-dangling-pointer +gdb-rhbz2160211-excessive-core-file-warnings.patch diff --git a/gdb-rhbz2160211-excessive-core-file-warnings.patch b/gdb-rhbz2160211-excessive-core-file-warnings.patch new file mode 100644 index 0000000..a790054 --- /dev/null +++ b/gdb-rhbz2160211-excessive-core-file-warnings.patch @@ -0,0 +1,108 @@ +From FEDORA_PATCHES Mon Sep 17 00:00:00 2001 +From: Kevin Buettner +Date: Thu, 29 Jun 2023 18:20:30 -0700 +Subject: gdb-rhbz2160211-excessive-core-file-warnings.patch + +;; Backport two commits, 0ad504dd464 and ea70f941f9b, from Lancelot SIX +;; which prevent repeated warnings from being printed while loading a +;; core file. (RH BZ 2160211) + +gdb/corelow.c: avoid repeated warnings in build_file_mappings + +When GDB opens a coredump it tries to locate and then open all files +which were mapped in the process. + +If a file is found but cannot be opened with BFD (bfd_open / +bfd_check_format fails), then a warning is printed to the user. If the +same file was mapped multiple times in the process's address space, the +warning is printed once for each time the file was mapped. I find this +un-necessarily noisy. + +This patch makes it so the warning message is printed only once per +file. + +There was a comment in the code assuming that if the file was found on +the system, opening it (bfd_open + bfd_check_format) should always +succeed. A recent change in BFD (014a602b86f "Don't optimise bfd_seek +to same position") showed that this assumption is not valid. For +example, it is possible to have a core dump of a process which had +mmaped an IO page from a DRI render node (/dev/dri/runderD$NUM). In +such case the core dump does contain the information that portions of +this special file were mapped in the host process, but trying to seek to +position 0 will fail, making bfd_check_format fail. This patch removes +this comment. + +Reviewed-By: John Baldwin +Approved-By: Andrew Burgess + +gdb/corelow.c: do not try to reopen a file if open failed once + +In the current implementation, core_target::build_file_mappings will try +to locate and open files which were mapped in the process for which the +core dump was produced. If the file cannot be found or cannot be +opened, GDB will re-try to open it once for each time it was mapped in +the process's address space. + +This patch makes it so GDB recognizes that it has already failed to open +a given file once and does not re-try the process for each mapping. + +Reviewed-By: John Baldwin +Approved-By: Andrew Burgess + +diff --git a/gdb/corelow.c b/gdb/corelow.c +--- a/gdb/corelow.c ++++ b/gdb/corelow.c +@@ -237,6 +237,16 @@ core_target::build_file_mappings () + weed out non-file-backed mappings. */ + gdb_assert (filename != nullptr); + ++ if (unavailable_paths.find (filename) != unavailable_paths.end ()) ++ { ++ /* We have already seen some mapping for FILENAME but failed to ++ find/open the file. There is no point in trying the same ++ thing again so just record that the range [start, end) is ++ unavailable. */ ++ m_core_unavailable_mappings.emplace_back (start, end - start); ++ return; ++ } ++ + struct bfd *bfd = bfd_map[filename]; + if (bfd == nullptr) + { +@@ -254,11 +264,10 @@ core_target::build_file_mappings () + if (expanded_fname == nullptr) + { + m_core_unavailable_mappings.emplace_back (start, end - start); +- /* Print just one warning per path. */ +- if (unavailable_paths.insert (filename).second) +- warning (_("Can't open file %s during file-backed mapping " +- "note processing"), +- filename); ++ unavailable_paths.insert (filename); ++ warning (_("Can't open file %s during file-backed mapping " ++ "note processing"), ++ filename); + return; + } + +@@ -268,18 +277,11 @@ core_target::build_file_mappings () + if (bfd == nullptr || !bfd_check_format (bfd, bfd_object)) + { + m_core_unavailable_mappings.emplace_back (start, end - start); +- /* If we get here, there's a good chance that it's due to +- an internal error. We issue a warning instead of an +- internal error because of the possibility that the +- file was removed in between checking for its +- existence during the expansion in exec_file_find() +- and the calls to bfd_openr() / bfd_check_format(). +- Output both the path from the core file note along +- with its expansion to make debugging this problem +- easier. */ ++ unavailable_paths.insert (filename); + warning (_("Can't open file %s which was expanded to %s " + "during file-backed mapping note processing"), + filename, expanded_fname.get ()); ++ + if (bfd != nullptr) + bfd_close (bfd); + return; diff --git a/gdb.spec b/gdb.spec index ca0af3b..df8d4f9 100644 --- a/gdb.spec +++ b/gdb.spec @@ -57,7 +57,7 @@ Version: 13.2 # The release always contains a leading reserved number, start it at 1. # `upstream' is not a part of `name' to stay fully rpm dependencies compatible for the testing. -Release: 1%{?dist} +Release: 2%{?dist} License: GPLv3+ and GPLv3+ with exceptions and GPLv2+ and GPLv2+ with exceptions and GPL+ and LGPLv2+ and LGPLv3+ and BSD and Public Domain and GFDL # Do not provide URL for snapshots as the file lasts there only for 2 days. @@ -1192,7 +1192,11 @@ fi %endif %changelog -* Thu Jun 22 2023 Alexandra Hájková - 13.1-2 +* Fri Jun 30 2023 Kevin Buettner - 13.2-2 +- Backport upstream changes which prevent repeated warnings from being + printed when loading a core file (RHBZ 2160211, Lancelot SIX). + +* Thu Jun 22 2023 Alexandra Hájková - 13.2-1 - Rebase to FSF GDB 13.2. - Update gdb-6.3-rh-testversion-20041202.patch. - Remove gdb-rhbz2177655-aarch64-pauth-valid-regcache.patch.