aboutsummaryrefslogtreecommitdiff
path: root/gdbserver
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 /gdbserver
parent0696ccbdb57b0a0d1ed34ed83b963a875ceffd1e (diff)
downloadbinutils-a8651ef51822f91ec86d0d5caffbf2e50b174c23.zip
binutils-a8651ef51822f91ec86d0d5caffbf2e50b174c23.tar.gz
binutils-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.
Diffstat (limited to 'gdbserver')
-rw-r--r--gdbserver/linux-aarch64-tdesc.cc22
1 files changed, 14 insertions, 8 deletions
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");