From patchwork Wed Jun 5 09:08:16 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 33013 Received: (qmail 19649 invoked by alias); 5 Jun 2019 09:08:23 -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 18666 invoked by uid 89); 5 Jun 2019 09:08:23 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.5 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mail-wr1-f54.google.com Received: from mail-wr1-f54.google.com (HELO mail-wr1-f54.google.com) (209.85.221.54) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 05 Jun 2019 09:08:20 +0000 Received: by mail-wr1-f54.google.com with SMTP id p11so13801392wre.7 for ; Wed, 05 Jun 2019 02:08:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=q21CxSvpQ6LDJH8rKgaJHurMCIkTH6x07sfQjA/qdZU=; b=WEmiGpgzDQ7E/LOyar8dOwHNYbauT15GIZ8xDJOlx2jZXrJ50TIa1iqjZj3ZveuPkf 2rB4pOuACdtPyp+P4jqctOQ/lrGjxIyM04fFWOjdStHwDDQRlWjVfZBl8WB51V3wFTE/ fISNyHcJ9vf0rsApLxhst/4L3q1MlasZoNO53Adc5cNFVKrlxwfyVNDBaB+59E0BEcjh 7euY+w91V3GB9wrToy6on1SwYGmJ4Emz80XswDNSlVguEQsb/J6WeqwuNSE3KAtowspp Ilfq8vrVpCA2vif5Z62Tt79006J6Py8P2Z6oTF9qRsWeLZVpb0NYjRDNnQITqgmokM+P fpzA== Return-Path: Received: from localhost (host86-180-62-212.range86-180.btcentralplus.com. [86.180.62.212]) by smtp.gmail.com with ESMTPSA id p16sm36656009wrg.49.2019.06.05.02.08.17 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 05 Jun 2019 02:08:17 -0700 (PDT) Date: Wed, 5 Jun 2019 10:08:16 +0100 From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Jim Wilson , Palmer Dabbelt Subject: Re: [PATCH] gdb/riscv: Don't error when decoding a 6 or 8 byte instruction Message-ID: <20190605090816.GD23204@embecosm.com> References: <20190604115054.25306-1-andrew.burgess@embecosm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190604115054.25306-1-andrew.burgess@embecosm.com> X-Fortune: The biggest problem with communication is the illusion that it has occurred. X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes * Andrew Burgess [2019-06-04 12:50:54 +0100]: > If the RISC-V prologue scanner finds a 6 or 8 byte instruction we > currently throw an internal error, which is not great for the user. > > A mechanism already exists in the prologue scanner to leave > instructions marked as unknown so that we can stop the prologue scan > without raising an error, this is used for all 2 and 4 byte > instructions that are not part of the small set the prologue scanner > actually understands. > > This commit changes GDB so that all 6 and 8 byte instructions are > marked as unknown, rather than causing an error. > > gdb/ChangeLog: > > * riscv-tdep.c (riscv_insn::decode): Gracefully ignore > instructions of lengths 6 or 8 bytes. > > gdb/testsuite/ChangeLog: > > * gdb.arch/riscv-unwind-long-insn-6.s: New file. > * gdb.arch/riscv-unwind-long-insn-8.s: New file. > * gdb.arch/riscv-unwind-long-insn.c: New file. > * gdb.arch/riscv-unwind-long-insn.exp: New file. I have now pushed the patch version below. The only change from the previous version is in riscv-unwind-long-insn.exp where I make the test binary name unique over the two iterations of the test. Thanks, Andrew --- commit 312617a3d06b8df67b9f4f63f92ebfaa6b591921 Author: Andrew Burgess Date: Tue Jun 4 10:30:41 2019 +0100 gdb/riscv: Don't error when decoding a 6 or 8 byte instruction If the RISC-V prologue scanner finds a 6 or 8 byte instruction we currently throw an internal error, which is not great for the user. A mechanism already exists in the prologue scanner to leave instructions marked as unknown so that we can stop the prologue scan without raising an error, this is used for all 2 and 4 byte instructions that are not part of the small set the prologue scanner actually understands. This commit changes GDB so that all 6 and 8 byte instructions are marked as unknown, rather than causing an error. gdb/ChangeLog: * riscv-tdep.c (riscv_insn::decode): Gracefully ignore instructions of lengths 6 or 8 bytes. gdb/testsuite/ChangeLog: * gdb.arch/riscv-unwind-long-insn-6.s: New file. * gdb.arch/riscv-unwind-long-insn-8.s: New file. * gdb.arch/riscv-unwind-long-insn.c: New file. * gdb.arch/riscv-unwind-long-insn.exp: New file. diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c index 3fc86ab825..bae987cf66 100644 --- a/gdb/riscv-tdep.c +++ b/gdb/riscv-tdep.c @@ -1385,10 +1385,12 @@ riscv_insn::decode (struct gdbarch *gdbarch, CORE_ADDR pc) m_opcode = OTHER; } else - internal_error (__FILE__, __LINE__, - _("unable to decode %d byte instructions in " - "prologue at %s"), m_length, - core_addr_to_string (pc)); + { + /* This must be a 6 or 8 byte instruction, we don't currently decode + any of these, so just ignore it. */ + gdb_assert (m_length == 6 || m_length == 8); + m_opcode = OTHER; + } } /* The prologue scanner. This is currently only used for skipping the diff --git a/gdb/testsuite/gdb.arch/riscv-unwind-long-insn-6.s b/gdb/testsuite/gdb.arch/riscv-unwind-long-insn-6.s new file mode 100644 index 0000000000..b21b1e10f3 --- /dev/null +++ b/gdb/testsuite/gdb.arch/riscv-unwind-long-insn-6.s @@ -0,0 +1,45 @@ +/* Copyright 2019 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 . */ + + .option nopic + .text + + .align 1 + .globl bar + .type bar, @function +bar: + tail 1f + .size bar, .-func + + .align 1 + .globl func + .type func, @function +func: + /* A fake 6 byte instruction. This is never executed, but the + prologue scanner will try to decode it. These long + instructions are ISA extensions, I use .byte rather than an + actual instruction mnemonic so that the test can be compiled + with a toolchain that doesn't include any long instruction + extensions. */ + .byte 0x1f, 0x00, 0x00, 0x00, 0x00, 0x00 +1: + addi sp,sp,-16 + sw s0,12(sp) + addi s0,sp,16 + nop + lw s0,12(sp) + addi sp,sp,16 + jr ra + .size func, .-func diff --git a/gdb/testsuite/gdb.arch/riscv-unwind-long-insn-8.s b/gdb/testsuite/gdb.arch/riscv-unwind-long-insn-8.s new file mode 100644 index 0000000000..3fad07b59d --- /dev/null +++ b/gdb/testsuite/gdb.arch/riscv-unwind-long-insn-8.s @@ -0,0 +1,45 @@ +/* Copyright 2019 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 . */ + + .option nopic + .text + + .align 1 + .globl bar + .type bar, @function +bar: + tail 1f + .size bar, .-func + + .align 1 + .globl func + .type func, @function +func: + /* A fake 8 byte instruction. This is never executed, but the + prologue scanner will try to decode it. These long + instructions are ISA extensions, I use .byte rather than an + actual instruction mnemonic so that the test can be compiled + with a toolchain that doesn't include any long instruction + extensions. */ + .byte 0x3f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 +1: + addi sp,sp,-16 + sw s0,12(sp) + addi s0,sp,16 + nop + lw s0,12(sp) + addi sp,sp,16 + jr ra + .size func, .-func diff --git a/gdb/testsuite/gdb.arch/riscv-unwind-long-insn.c b/gdb/testsuite/gdb.arch/riscv-unwind-long-insn.c new file mode 100644 index 0000000000..d601e2d3d8 --- /dev/null +++ b/gdb/testsuite/gdb.arch/riscv-unwind-long-insn.c @@ -0,0 +1,25 @@ +/* Copyright 2019 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 . */ + +extern void func (void); +extern void bar (void); + +int +main () +{ + bar (); + func (); + return 0; +} diff --git a/gdb/testsuite/gdb.arch/riscv-unwind-long-insn.exp b/gdb/testsuite/gdb.arch/riscv-unwind-long-insn.exp new file mode 100644 index 0000000000..ddb914295c --- /dev/null +++ b/gdb/testsuite/gdb.arch/riscv-unwind-long-insn.exp @@ -0,0 +1,61 @@ +# Copyright 2019 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 . + +# This tests GDB's ability to handle 6 and 8 byte instructions in the +# RISC-V prologue scanner. These instruction should be ignored, but +# should not result in an error that interrupts the debug session. +# +# Each of the files riscv-unwind-long-insn-*.s include a function +# (func) that contains a fake long instruction (6 or 8 bytes) in the +# prologue. We trick GDB into parsing the fake instruction by tail +# calling from a different function, 'bar' to the middle of 'func'. + +if {![istarget "riscv*-*-*"]} { + verbose "Skipping ${gdb_test_file_name}." + return +} + +foreach_with_prefix {insn_size} {6 8} { + standard_testfile riscv-unwind-long-insn.c \ + riscv-unwind-long-insn-${insn_size}.s + + set testfile "${testfile}-${insn_size}" + if {[prepare_for_testing "failed to prepare" $testfile \ + "$srcfile $srcfile2" debug]} { + return -1 + } + + if ![runto_main] then { + fail "can't run to main" + return 0 + } + + gdb_breakpoint "bar" + gdb_continue_to_breakpoint "bar" + + # This next single instruction step takes us through a tail-call + # from 'bar' into 'func'. + gdb_test "si" "func \(\).*" + + # Now check that we have a sane backtrace. + gdb_test "bt" \ + [multi_line \ + "#0\[ \t\]*func \\\(\\\) at .*$srcfile2:\[0-9\]+" \ + "#1\[ \t\]*$hex in main \\\(\\\) at .*$srcfile:\[0-9\]+"] \ + "Backtrace to the main frame" + + # Finally finish, and we should end up back in main. + gdb_test "finish" "main \\\(\\\) at .*$srcfile:.*" +}