aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndrew Burgess <aburgess@redhat.com>2024-06-12 13:10:53 -0400
committerAndrew Burgess <aburgess@redhat.com>2024-06-14 14:47:38 +0100
commita8651ef51822f91ec86d0d5caffbf2e50b174c23 (patch)
treece94d7b8c5fb0635c1ae7fd758c5e8d2113b3c16
parent0696ccbdb57b0a0d1ed34ed83b963a875ceffd1e (diff)
downloadgdb-a8651ef51822f91ec86d0d5caffbf2e50b174c23.zip
gdb-a8651ef51822f91ec86d0d5caffbf2e50b174c23.tar.gz
gdb-a8651ef51822f91ec86d0d5caffbf2e50b174c23.tar.bz2
gdb/aarch64: prevent crash from in process agent
Since this commit: commit 0ee6b1c511c0e2a6793568692d2e5418cd6bc10d Date: Wed May 18 13:32:04 2022 -0700 Use aarch64_features to describe register features in target descriptions. There has been an issue with how aarch64 target descriptions are cached within gdbserver, and specifically, how this caching impacts the in process agent (IPA). The function initialize_tracepoint_ftlib (gdbserver/tracepoint.cc) is part of the IPA, this function is a constructor function, i.e. is called as part of the global initialisation process. We can't guarantee the ordering of when this function is called vs when other global state is initialised. Now initialize_tracepoint_ftlib calls initialize_tracepoint, which calls initialize_low_tracepoint, which for aarch64 calls aarch64_linux_read_description. The aarch64_linux_read_description function lives in linux-aarch64-tdesc.cc and after the above commit, depends on a std::unordered_map having been initialized. Prior to the above commit aarch64_linux_read_description used a global C style array, which obviously requires no runtime initialization. The consequence of the above is that any inferior linked with the IPA (for aarch64) will experience undefined behaviour (access to an uninitialized std::unordered_map) during startup, which for me manifests as a segfault. I propose fixing this by moving the std::unordered_map into the function body, but leaving it static. The map will now be initialized the first time the function is called, which removes the undefiend behaviour. The same problem exists for the expedited_registers global, however this global can just be made into a function local instead. The expedited_registers variable is used to build a pointer list which is then passed to init_target_desc, however init_target_desc copies the values it is given so expedited_registers does not need to live longer than its containing function. On most of the AArch64 machines I have access too tracing is not supported, and so the gdb.trace/*.exp tests that use the IPA just exit early reporting unsupported. I've added a test which links an inferior with the IPA and just starts the inferior. No tracing is performed. This exposes the current issue even on hosts that don't support tracing. After this patch the test passes.
-rw-r--r--gdb/testsuite/gdb.trace/basic-libipa.c22
-rw-r--r--gdb/testsuite/gdb.trace/basic-libipa.exp46
-rw-r--r--gdbserver/linux-aarch64-tdesc.cc22
3 files changed, 82 insertions, 8 deletions
diff --git a/gdb/testsuite/gdb.trace/basic-libipa.c b/gdb/testsuite/gdb.trace/basic-libipa.c
new file mode 100644
index 0000000..bbcfb01
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/basic-libipa.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2024 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/>. */
+
+int
+main ()
+{
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.trace/basic-libipa.exp b/gdb/testsuite/gdb.trace/basic-libipa.exp
new file mode 100644
index 0000000..e28f61e
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/basic-libipa.exp
@@ -0,0 +1,46 @@
+# Copyright 2024 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/>.
+
+# Very simple test that links with libinproctrace.so and then starts
+# the resulting inferior.
+#
+# This test should run on targets that don't support tracing, but for
+# which libinproctrace.so is built and helps catch some issues where
+# libinproctrace.so is so broken inferiors wont even start.
+
+load_lib "trace-support.exp"
+
+require allow_shlib_tests
+
+standard_testfile
+
+set libipa [get_in_proc_agent]
+
+if { ![file exists $libipa] } {
+ unsupported "missing libinproctrace.so"
+ return -1
+}
+
+gdb_download_shlib $libipa
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
+ [list debug shlib=$libipa]] } {
+ return -1
+}
+
+if {![runto_main]} {
+ return -1
+}
+
+pass "inferior with libinproctrace.so started"
diff --git a/gdbserver/linux-aarch64-tdesc.cc b/gdbserver/linux-aarch64-tdesc.cc
index 0ed9a42..5d3b6dd 100644
--- a/gdbserver/linux-aarch64-tdesc.cc
+++ b/gdbserver/linux-aarch64-tdesc.cc
@@ -26,16 +26,17 @@
#include <inttypes.h>
#include <unordered_map>
-/* All possible aarch64 target descriptors. */
-static std::unordered_map<aarch64_features, target_desc *> tdesc_aarch64_map;
-
-static std::vector<const char *> expedited_registers;
-
/* Create the aarch64 target description. */
const target_desc *
aarch64_linux_read_description (const aarch64_features &features)
{
+ /* All possible aarch64 target descriptors. This map must live within
+ this function as the in-process-agent calls this function from a
+ constructor function, when globals might not yet have been
+ initialised. */
+ static std::unordered_map<aarch64_features, target_desc *> tdesc_aarch64_map;
+
if (features.vq > AARCH64_MAX_SVE_VQ)
error (_("VQ is %" PRIu64 ", maximum supported value is %d"), features.vq,
AARCH64_MAX_SVE_VQ);
@@ -50,10 +51,15 @@ aarch64_linux_read_description (const aarch64_features &features)
if (tdesc == NULL)
{
tdesc = aarch64_create_target_description (features);
- expedited_registers.clear ();
- /* Configure the expedited registers. By default we include x29, sp and
- pc. */
+ /* Configure the expedited registers. By default we include x29, sp
+ and pc, but we allow for up to 6 pointers as this is (currently)
+ the most that we push.
+
+ Calling init_target_desc takes a copy of all the strings pointed
+ to by expedited_registers so this vector only needs to live for
+ the scope of this function. */
+ std::vector<const char *> expedited_registers (6);
expedited_registers.push_back ("x29");
expedited_registers.push_back ("sp");
expedited_registers.push_back ("pc");