Re: [PATCH] gdb/riscv: Don't error when decoding a 6 or 8 byte instruction

[Palmer Dabbelt <palmer@sifive.com>]

On Tue, 04 Jun 2019 04:50:54 PDT (-0700), andrew.burgess@embecosm.com wrote:
> 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.
> ---
>  gdb/ChangeLog                                     |  5 ++
>  gdb/riscv-tdep.c                                  | 10 ++--
>  gdb/testsuite/ChangeLog                           |  7 +++
>  gdb/testsuite/gdb.arch/riscv-unwind-long-insn-6.s | 45 +++++++++++++++++
>  gdb/testsuite/gdb.arch/riscv-unwind-long-insn-8.s | 45 +++++++++++++++++
>  gdb/testsuite/gdb.arch/riscv-unwind-long-insn.c   | 25 ++++++++++
>  gdb/testsuite/gdb.arch/riscv-unwind-long-insn.exp | 60 +++++++++++++++++++++++
>  7 files changed, 193 insertions(+), 4 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.arch/riscv-unwind-long-insn-6.s
>  create mode 100644 gdb/testsuite/gdb.arch/riscv-unwind-long-insn-8.s
>  create mode 100644 gdb/testsuite/gdb.arch/riscv-unwind-long-insn.c
>  create mode 100644 gdb/testsuite/gdb.arch/riscv-unwind-long-insn.exp
>
> 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 <http://www.gnu.org/licenses/>.  */
> +
> +	.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 <http://www.gnu.org/licenses/>.  */
> +
> +	.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 <http://www.gnu.org/licenses/>.  */
> +
> +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..e4bc489720
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/riscv-unwind-long-insn.exp
> @@ -0,0 +1,60 @@
> +# 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 <http://www.gnu.org/licenses/>.
> +
> +# 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
> +
> +    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:.*"
> +}

Thanks!