From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id BD5533856279 for ; Thu, 9 Jun 2022 12:32:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BD5533856279 Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-607-ns5SZo54PL-ngClFF7AU3Q-1; Thu, 09 Jun 2022 08:32:41 -0400 X-MC-Unique: ns5SZo54PL-ngClFF7AU3Q-1 Received: by mail-wm1-f70.google.com with SMTP id ay28-20020a05600c1e1c00b0039c5cbe76c1so3526702wmb.1 for ; Thu, 09 Jun 2022 05:32:41 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=PzqXly3wikS1ExKyVtboGxx12Zm3GzudtyhV3NSLsYA=; b=vdKsCpfXv8EkllZp45iTdgK9qkPBO5SnorvmHHEXuZIJcWjHU1cY/cJJ3eW53Ps1D/ axvxt2Eia1NGJHnMo8U92nvcfBNuGT5UAoZz252U8BUdkl8g5uC1oN0fzt3HvAiSuqnz NiBYoxS/ohuAwtQb6S3UtH/56s86910hTOAnAWOtYL8mpazfLPKidIKGt+onwTLbRLgl 0Mld/1XKUZ85l6MHbgIxlBP5YfRWZUcru/Scvn00Z2NS9MXFVRum0QLCbn/2d1BVRJmd l8dP8kuxs8FPOFX1a2lUD2fKdTbly+y4Xcj4ddrZOXBngSh8QunENjBdIvmbTSuUXQcP /oGw== X-Gm-Message-State: AOAM530O1SCEy2XO4clHaxdUkWBcPs182zVfHVDaPWjIfOW5ZsDvC2/+ nC7+D3luoADHF7bZDyYqsdJsDYkOVFYTvwKHdRarN6SNnb8WmcYpJp7bjYIgcwFtMonK8pByQbs 6YIWeoLjUAm4JeY4MwdNf82teXgPTqsOC6xtfU0FIiBc5s1Hvmb1rumAWNNIQo1DoRqDreF0G2g == X-Received: by 2002:adf:ef01:0:b0:20a:8068:ca5e with SMTP id e1-20020adfef01000000b0020a8068ca5emr37612038wro.661.1654777959928; Thu, 09 Jun 2022 05:32:39 -0700 (PDT) X-Google-Smtp-Source: ABdhPJw+VdiG1Nup3HqiETVK/La3QqCbKfGtU7bND+YoX8qQAvoR7BD+sxJAiEiXMVVJqX8GCTN2eg== X-Received: by 2002:adf:ef01:0:b0:20a:8068:ca5e with SMTP id e1-20020adfef01000000b0020a8068ca5emr37612007wro.661.1654777959592; Thu, 09 Jun 2022 05:32:39 -0700 (PDT) Received: from localhost (host109-152-215-36.range109-152.btcentralplus.com. [109.152.215.36]) by smtp.gmail.com with ESMTPSA id h12-20020a5d6e0c000000b0020ff877cfbdsm24081609wrz.87.2022.06.09.05.32.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Jun 2022 05:32:39 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH] gdb/aarch64: fix 32-bit arm compatibility Date: Thu, 9 Jun 2022 13:32:36 +0100 Message-Id: <20220609123236.336949-1-aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Jun 2022 12:32:46 -0000 GDB's ability to run 32-bit ARM processes on an AArch64 native target is currently broken. The test gdb.multi/multi-arch.exp currently fails with a timeout. The cause of these problems is the following three functions: aarch64_linux_nat_target::thread_architecture aarch64_linux_nat_target::fetch_registers aarch64_linux_nat_target::store_registers What has happened, over time, is that these functions have been modified, forgetting that any particular thread (running on the native target) might be an ARM thread, or might be an AArch64 thread. The problems always start with a line similar to this: aarch64_gdbarch_tdep *tdep = (aarch64_gdbarch_tdep *) gdbarch_tdep (inf->gdbarch); The problem with this line is that if 'inf->gdbarch' is an ARM architecture, then gdbarch_tdep will return a pointer to an arm_gdbarch_tdep object, not an aarch64_gdbarch_tdep object. The result of the above cast will, as a consequence, be undefined. In aarch64_linux_nat_target::thread_architecture, after the undefined cast we then proceed to make use of TDEP, like this: if (vq == tdep->vq) return inf->gdbarch; Obviously at this point the result is undefined, but, if this check returns false we then proceed with this code: struct gdbarch_info info; info.bfd_arch_info = bfd_lookup_arch (bfd_arch_aarch64, bfd_mach_aarch64); info.id = (int *) (vq == 0 ? -1 : vq); return gdbarch_find_by_info (info); As a consequence we will return an AArch64 gdbarch object for our ARM thread! Things go downhill from there on. There are similar problems, with similar undefined behaviour, in the fetch_registers and store_registers functions. The solution is to make use of a check like this: if (gdbarch_bfd_arch_info (inf->gdbarch)->bits_per_word == 32) If the word size is 32 then we know we have an ARM architecture. We just need to make sure that we perform this check before trying to read the tdep field. In aarch64_linux_nat_target::thread_architecture a little reordering, and the addition of the above check allows us to easily avoid the undefined behaviour. For fetch_registers and store_registers I made the decision to split each of the functions into two new helper functions, and so aarch64_linux_nat_target::fetch_registers now calls to either aarch64_fetch_registers or aarch32_fetch_registers, and there's a similar change for store_registers. One thing I had to decide was whether to place the new aarch32_* functions into the aarch32-linux-nat.c file. In the end I decided to NOT place the functions there, but instead leave them in aarch64-linux-nat.c, my reasoning was this: The existing functions in that file are shared from arm-linux-nat.c and aarch64-linux-nat.c, this generic code to support 32-bit ARM debugging from either native target. In contrast, the two new aarch32 functions I have added _only_ make sense when debugging on an AArch64 native target. These function shouldn't be called from arm-linux-nat.c at all, and so, if we places the functions into aarch32-linux-nat.c, the functions would be built into a 32-bit ARM GDB, but never used. With that said, there's no technical reason why they couldn't go in aarch32-linux-nat.c, so if that is preferred I'm happy to move them. After this commit the gdb.multi/multi-arch.exp passes. --- gdb/aarch64-linux-nat.c | 114 +++++++++++++++++++++++++++++++++++----- 1 file changed, 100 insertions(+), 14 deletions(-) diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c index ff5c4c0a212..d58ad0143a2 100644 --- a/gdb/aarch64-linux-nat.c +++ b/gdb/aarch64-linux-nat.c @@ -46,6 +46,7 @@ #include "gregset.h" #include "linux-tdep.h" +#include "arm-tdep.h" /* Defines ps_err_e, struct ps_prochandle. */ #include "gdb_proc_service.h" @@ -485,11 +486,11 @@ store_tlsregs_to_thread (struct regcache *regcache) perror_with_name (_("unable to store TLS register")); } -/* Implement the "fetch_registers" target_ops method. */ +/* The AArch64 version of the "fetch_registers" target_ops method. Fetch + REGNO from the target and place the result into REGCACHE. */ -void -aarch64_linux_nat_target::fetch_registers (struct regcache *regcache, - int regno) +static void +aarch64_fetch_registers (struct regcache *regcache, int regno) { aarch64_gdbarch_tdep *tdep = (aarch64_gdbarch_tdep *) gdbarch_tdep (regcache->arch ()); @@ -534,11 +535,48 @@ aarch64_linux_nat_target::fetch_registers (struct regcache *regcache, fetch_tlsregs_from_thread (regcache); } -/* Implement the "store_registers" target_ops method. */ +/* A version of the "fetch_registers" target_ops method used when running + 32-bit ARM code on an AArch64 target. Fetch REGNO from the target and + place the result into REGCACHE. */ + +static void +aarch32_fetch_registers (struct regcache *regcache, int regno) +{ + arm_gdbarch_tdep *tdep + = (arm_gdbarch_tdep *) gdbarch_tdep (regcache->arch ()); + + if (regno == -1) + { + fetch_gregs_from_thread (regcache); + if (tdep->vfp_register_count > 0) + fetch_fpregs_from_thread (regcache); + } + else if (regno < ARM_F0_REGNUM || regno == ARM_PS_REGNUM) + fetch_gregs_from_thread (regcache); + else if (tdep->vfp_register_count > 0 + && regno >= ARM_D0_REGNUM + && (regno < ARM_D0_REGNUM + tdep->vfp_register_count + || regno == ARM_FPSCR_REGNUM)) + fetch_fpregs_from_thread (regcache); +} + +/* Implement the "fetch_registers" target_ops method. */ void -aarch64_linux_nat_target::store_registers (struct regcache *regcache, +aarch64_linux_nat_target::fetch_registers (struct regcache *regcache, int regno) +{ + if (gdbarch_bfd_arch_info (regcache->arch ())->bits_per_word == 32) + aarch32_fetch_registers (regcache, regno); + else + aarch64_fetch_registers (regcache, regno); +} + +/* The AArch64 version of the "store_registers" target_ops method. Copy + the value of register REGNO from REGCACHE into the the target. */ + +static void +aarch64_store_registers (struct regcache *regcache, int regno) { aarch64_gdbarch_tdep *tdep = (aarch64_gdbarch_tdep *) gdbarch_tdep (regcache->arch ()); @@ -573,6 +611,43 @@ aarch64_linux_nat_target::store_registers (struct regcache *regcache, store_tlsregs_to_thread (regcache); } +/* A version of the "store_registers" target_ops method used when running + 32-bit ARM code on an AArch64 target. Copy the value of register REGNO + from REGCACHE into the the target. */ + +static void +aarch32_store_registers (struct regcache *regcache, int regno) +{ + arm_gdbarch_tdep *tdep + = (arm_gdbarch_tdep *) gdbarch_tdep (regcache->arch ()); + + if (regno == -1) + { + store_gregs_to_thread (regcache); + if (tdep->vfp_register_count > 0) + store_fpregs_to_thread (regcache); + } + else if (regno < ARM_F0_REGNUM || regno == ARM_PS_REGNUM) + store_gregs_to_thread (regcache); + else if (tdep->vfp_register_count > 0 + && regno >= ARM_D0_REGNUM + && (regno < ARM_D0_REGNUM + tdep->vfp_register_count + || regno == ARM_FPSCR_REGNUM)) + store_fpregs_to_thread (regcache); +} + +/* Implement the "store_registers" target_ops method. */ + +void +aarch64_linux_nat_target::store_registers (struct regcache *regcache, + int regno) +{ + if (gdbarch_bfd_arch_info (regcache->arch ())->bits_per_word == 32) + aarch32_store_registers (regcache, regno); + else + aarch64_store_registers (regcache, regno); +} + /* Fill register REGNO (if it is a general-purpose register) in *GREGSETPS with the value in GDB's register array. If REGNO is -1, do this for all registers. */ @@ -793,22 +868,33 @@ aarch64_linux_nat_target::can_do_single_step () return 1; } -/* Implement the "thread_architecture" target_ops method. */ +/* Implement the "thread_architecture" target_ops method. + + Returns the gdbarch for the thread identified by PTID. If the thread in + question is a 32-bit ARM thread, then the architecture returned will be + that of the process itself. + + If the thread is an AArch64 thread then we need to check the current + vector length; if the vector length has changed then we need to lookup a + new gdbarch that matches the new vector length. */ struct gdbarch * aarch64_linux_nat_target::thread_architecture (ptid_t ptid) { - /* Return the gdbarch for the current thread. If the vector length has - changed since the last time this was called, then do a further lookup. */ - - uint64_t vq = aarch64_sve_get_vq (ptid.lwp ()); - - /* Find the current gdbarch the same way as process_stratum_target. Only - return it if the current vector length matches the one in the tdep. */ + /* Find the current gdbarch the same way as process_stratum_target. */ inferior *inf = find_inferior_ptid (this, ptid); gdb_assert (inf != NULL); + + /* If this is a 32-bit architecture, then this is ARM, not AArch64. + There's no SVE vectors here, so just return the inferior + architecture. */ + if (gdbarch_bfd_arch_info (inf->gdbarch)->bits_per_word == 32) + return inf->gdbarch; + + /* Only return it if the current vector length matches the one in the tdep. */ aarch64_gdbarch_tdep *tdep = (aarch64_gdbarch_tdep *) gdbarch_tdep (inf->gdbarch); + uint64_t vq = aarch64_sve_get_vq (ptid.lwp ()); if (vq == tdep->vq) return inf->gdbarch; -- 2.25.4