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 7B0C63829BF9 for ; Fri, 10 Jun 2022 13:08:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7B0C63829BF9 Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-77-cf8ZKp3UPWu93urj1fIQ1g-1; Fri, 10 Jun 2022 09:08:26 -0400 X-MC-Unique: cf8ZKp3UPWu93urj1fIQ1g-1 Received: by mail-wr1-f69.google.com with SMTP id r13-20020adff10d000000b002160e9d64f8so4701298wro.0 for ; Fri, 10 Jun 2022 06:08:25 -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:in-reply-to :references:mime-version:content-transfer-encoding; bh=ZJpWSM7Lw3/O/ohcCqKGM0qSplZH0350xjn/vSOM9Sg=; b=TgArQ7+vmp8c9t5dWrO8HwuP9/7HW5hhKqZ9FqyCDhNGnyhIInkHDoGf0Wy2lL3A/F Kzr6NiEQRtVkdYNUn1oic6rgDv45qsB9gu/8FxI38Hl1culAJYkRufbSNoSuNNkmYE9Q TEHqg9mWiIDpQUm41JHwiyZ/mHnXfpk6WwZ0cnLYlPh+6b8x2zz20wR2WUnzeMq9u0Sv MFA5iWnTAmG5hxqiEUPucGpwnbGebwq1feZbZjk+HnAth5KTkKi7bO1aPkIMvKxFBEPk AfCe3gqfCv5CLmdYgDwqohfWbJxFoO81TwbTXxqaRPc9YlDY0ar8IWsunS1Mk7ry1WC5 TTCA== X-Gm-Message-State: AOAM531sIQheKuB76+spzi3WTjsrUxrMg3HcfEpJoZuZ6EiFq1lfsR5w k04TwbySJ8DL1hc+lycztfHMEOZIrOHMXgDUxUrGJnFbo+DV6LzU9h6P+oC0//guyPVGfOR5BY+ D3lETmmZbvPqMsdOcZHxdnfH8ztUNt6EBXXzu1I51a/i3CnGt4St5wr7KluvBDOd0l5PFhz/1EA == X-Received: by 2002:a5d:40c5:0:b0:216:ddde:9c2e with SMTP id b5-20020a5d40c5000000b00216ddde9c2emr31573574wrq.241.1654866504429; Fri, 10 Jun 2022 06:08:24 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzx7Z3kH8BGZvbkm2Bd7y8Ocs2NJLrDKKWYGdd44BeY/ZT6RUCPUScWuB20gvFi3XV1XRTPgA== X-Received: by 2002:a5d:40c5:0:b0:216:ddde:9c2e with SMTP id b5-20020a5d40c5000000b00216ddde9c2emr31573541wrq.241.1654866504057; Fri, 10 Jun 2022 06:08:24 -0700 (PDT) Received: from localhost (host109-152-215-36.range109-152.btcentralplus.com. [109.152.215.36]) by smtp.gmail.com with ESMTPSA id w12-20020a7bc74c000000b003973d425a7fsm3601058wmk.41.2022.06.10.06.08.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Jun 2022 06:08:23 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCHv2 3/6] gdb/arm: avoid undefined behaviour in arm_frame_is_thumb Date: Fri, 10 Jun 2022 14:08:11 +0100 Message-Id: <0d968da223ab233af5ce95520f5472a4d849d269.1654866187.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: References: 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.7 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: Fri, 10 Jun 2022 13:08:28 -0000 This commit fixes real undefined behaviour in GDB which I spotted when working on a later patch in this series. The later patch in this series detects when the result of gdbarch_tdep() is cast to the wrong type. The issue is revealed by the gdb.multi/multi-arch.exp test. In this test we setup two inferiors, an AArch64 process, and an ARM process, then at one point we have inferior 1 selected (the AArch64 inferior), and we place a breakpoint on a symbol present in the other inferior (the ARM inferior). During the process of creating the breakpoint we call arm_pc_is_thumb, the GDBARCH passed into this function is correct, that is, represents the ARM process. For whatever reason we are unable to figure out if the address in question is thumb or not throughout most of arm_pc_is_thumb, and so we get to this code at the end of the function: /* If we couldn't find any symbol, but we're talking to a running target, then trust the current value of $cpsr. This lets "display/i $pc" always show the correct mode (though if there is a symbol table we will not reach here, so it still may not be displayed in the mode it will be executed). */ if (target_has_registers ()) return arm_frame_is_thumb (get_current_frame ()); Which I guess is a last attempt to figure out the thumb status of an address. However, remember, we the AArch64 inferior is current at this time, so the current frame is an AArch64 frame. In arm_frame_is_thumb we call arm_psr_thumb_bit with the architecture of the current frame (which will be an AArch64 architecture). And in arm_psr_thumb_bit we do this: arm_gdbarch_tdep *tdep = (arm_gdbarch_tdep *) gdbarch_tdep (gdbarch); which assumes that the current architecture is an ARM architecture. In our case this is not true, its AArch64, and so we cast the tdep object to arm_gdbarch_tdep when really it is of type aarch64_gdbarch_tdep. After this we access the fields of the tdep object, but this is undefined behaviour. To fix this I propose adding new code to arm_frame_is_thumb that checks the bfd architecture of the current frame. And frame that is not bfd_arch_arm can't be a thumb frame. I could add a gdb_assert to arm_psr_thumb_bit, however, I have not done this. A later commit in this series will add an assert to the gdbarch_tdep() call to ensure that the cast is of the correct type. This will make any assert (of bfd architecture) added to arm_psr_thumb_bit redundant, so I've not added any such assert here. With this commit in place the gdb.multi/multi-arch.exp test continues to pass after I have applied the later patches that detect incorrect casting of the gdbarch_tdep objects. --- gdb/arm-tdep.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index 7f27d4bd6e8..46027afade4 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -546,15 +546,19 @@ arm_is_thumb (struct regcache *regcache) int arm_frame_is_thumb (struct frame_info *frame) { - CORE_ADDR cpsr; - ULONGEST t_bit = arm_psr_thumb_bit (get_frame_arch (frame)); + /* If the current frame is not ARM then it can't be thumb. */ + struct gdbarch *gdbarch = get_frame_arch (frame); + if (gdbarch_bfd_arch_info (target_gdbarch ())->arch != bfd_arch_arm) + return 0; /* Every ARM frame unwinder can unwind the T bit of the CPSR, either directly (from a signal frame or dummy frame) or by interpreting the saved LR (from a prologue or DWARF frame). So consult it and trust the unwinders. */ - cpsr = get_frame_register_unsigned (frame, ARM_PS_REGNUM); + CORE_ADDR cpsr = get_frame_register_unsigned (frame, ARM_PS_REGNUM); + /* Find and extract the thumb bit. */ + ULONGEST t_bit = arm_psr_thumb_bit (gdbarch); return (cpsr & t_bit) != 0; } -- 2.25.4