[1/2] gdb/riscv: Stop prologue scan if instruction fetch/decode fails

  If an error is thrown from the instruction fetch/decode during a
prologue scan then we should stop the prologue scan at that point
rather than propagating the error.

Propagating the error out of the prologue scan was causing unwanted
behaviour when connecting to a remote target.  When connecting to a
remote target GDB will read the $pc value from the target and try to
establish a frame-id, this can involve a prologue scan.

If the target has not yet had a program loaded into it, and the $pc
value is pointing an unreadable memory, then the prologue scan would
throw an error, this would then cause GDB to abandon its attempt to
connect to the target.  It was in fact impossible to connect to the
target at all.

With this patch in place GDB simply stops the prologue scan at the
point of the error, and GDB can now successfully connect.

I did consider placing the error catch within riscv_insn::decode
however, in the end I felt that catching and ignoring errors should be
done on a case by case basis, the other users of riscv_insn::decode
are currently all related to finding the next pc as part of software
single step.  If the user asks for a step and the contents of $pc
can't be read then if feels like terminating that command with an
error is the right thing to do.

gdb/ChangeLog:

	* riscv-tdep.c (riscv_insn::decode): Update header comment.
	(riscv_scan_prologue): Catch errors thrown from
	riscv_insn::decode and stop prologue scan.
---
 gdb/ChangeLog    |  6 ++++++
 gdb/riscv-tdep.c | 27 ++++++++++++++++++++++++---
 2 files changed, 30 insertions(+), 3 deletions(-)
  

On Mon, Nov 5, 2018 at 3:10 PM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
> If the target has not yet had a program loaded into it, and the $pc
> value is pointing an unreadable memory, then the prologue scan would
> throw an error, this would then cause GDB to abandon its attempt to
> connect to the target.  It was in fact impossible to connect to the
> target at all.

In my case, with openocd/spike, the pc value is actually correct and
there is a valid instruction there.  The problem rather happens in
riscv_frame_cache which calls get_frame_func, and this returns 0
because there is no program loaded yet.  This then causes a scan for
the prologue to start at address zero, which is wrong, and leads to
the null deref error that kills the connection.  I have a simpler fix
based on code I found in mips-tdep.c, which just returns from
riscv_frame_cache if start_addr is zero, and also in
riscv_frame_this_id we don't set this_id if the frame_base is zero.
With your fix, riscv_scan_prologue will be run, the frame cache will
be filled with incorrect values, and we will try to compute a frame id
based on bad info.  That doesn't look like the right solution to me.
My patch is a slightly cleaned up version of the workarounds I sent to
you last week, which I am testing now.

Jim

PS Did you see my code_elim testcase fix? Simon Marchi suggested that
you should review it.