From patchwork Wed Nov 21 18:17:04 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 30249 Received: (qmail 75908 invoked by alias); 21 Nov 2018 18:17:19 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 75893 invoked by uid 89); 21 Nov 2018 18:17:18 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.2 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=burn, acknowledge, 2777, encountered X-HELO: mail-wr1-f65.google.com Received: from mail-wr1-f65.google.com (HELO mail-wr1-f65.google.com) (209.85.221.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 21 Nov 2018 18:17:16 +0000 Received: by mail-wr1-f65.google.com with SMTP id p4so6700181wrt.7 for ; Wed, 21 Nov 2018 10:17:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=from:to:cc:subject:date:message-id; bh=yMPs6D0ffqzPSLQClh9MdcXTsXx17mdewS0FQsVmd/U=; b=UdaHoZl2bR2SmwsZhDngaRj41tzeLJjE0pp9Bi+un2WuCY94itVOg0xFdWisfeKHT4 1gCb45gNYs3fLg/OPqGHvsGuCA0JlZKTBXe8oc70XzWViylMlglhuX8s/A95Q3zW6QqD OC2XPSRHIRdAd7w8lUw9zaH0UbWFBql14xyas1/5BbuS/+8/Woi1LyYZWHGznVANu9DR kBq+7BztEjkh9z6djVMKyw/WA4AIvA6bKX3uhB4fly/uYO/lYThmelPUhc6lc3UulQDr +bjrVjHjqi4DhTWXBD3QG09E3AcG33tr9/ond0POaaeHbhXrL6ZtFU0FCS1GytRmuzMc Ddyg== Return-Path: Received: from localhost (host81-156-111-139.range81-156.btcentralplus.com. [81.156.111.139]) by smtp.gmail.com with ESMTPSA id v62-v6sm2151563wme.3.2018.11.21.10.17.12 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 21 Nov 2018 10:17:13 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH] gdb/regcache: When saving, ignore registers that can't be read Date: Wed, 21 Nov 2018 18:17:04 +0000 Message-Id: <20181121181704.15929-1-andrew.burgess@embecosm.com> X-IsSubscribed: yes If during a call to reg_buffer::save GDB encounters an error trying to read a register then this should not cause GDB to crash, nor should it force the save to quit. Instead, GDB should just treat the register as unavailable and push on. The specific example I encountered was a RISC-V remote target that claimed in its target description to have floating point register support. However, this was not true, when GDB tried to read a floating point register the remote sent back an error. Mostly this was fine, the program I was testing were integer only, however, when trying to make an inferior call, GDB would try to preserve the current values of the floating point registers, this result in a read of a register that threw an error, and GDB would crash like this: (gdb) call some_inferior_function () ../../src/gdb/regcache.c:310: internal-error: void regcache::restore(readonly_detached_regcache*): Assertion `src != NULL' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. Quit this debugging session? (y or n) I acknowledge that the target description sent back in this case is wrong, and the target should be fixed. However, I think that GDB should, at a minimum, not crash and burn in this case, and better, I think GDB can probably just push on, ignoring the registers that can't be read. The solution I propose in this patch is to catch errors in reg_buffer::save while calling cooked_read, and register that throws an error should be considered unavailable. GDB will not try to restore these registers after the inferior call. What I haven't done in this commit is provide any user feedback that GDB would like to backup a particular register, but can't. Right now I figure that if the user cares about this they would probably try 'p $reg_name' themselves, at which point it becomes obvious that the register can't be read. That said, I'm open to adding a warning that the regiter failed to save if that is thought important. I've tested this using on X86-64/Linux native, and for native-gdbserver with no regressions. Against my miss-behaving target I can now make inferior calls without any problems. gdb/ChangeLog: * regcache.c (reg_buffer::save): When saving the current register state, ignore registers that can't be read. --- gdb/ChangeLog | 5 +++++ gdb/regcache.c | 12 +++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/gdb/regcache.c b/gdb/regcache.c index 946035ae67a..b89be24ccb6 100644 --- a/gdb/regcache.c +++ b/gdb/regcache.c @@ -277,7 +277,17 @@ reg_buffer::save (register_read_ftype cooked_read) if (gdbarch_register_reggroup_p (gdbarch, regnum, save_reggroup)) { gdb_byte *dst_buf = register_buffer (regnum); - enum register_status status = cooked_read (regnum, dst_buf); + enum register_status status; + + TRY + { + status = cooked_read (regnum, dst_buf); + } + CATCH (ex, RETURN_MASK_ERROR) + { + status = REG_UNAVAILABLE; + } + END_CATCH gdb_assert (status != REG_UNKNOWN);