[binutils-gdb] Don't throw exception in dis_asm_memory_error

[Yao Qi <qiyao at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d8b49cf0c891d09dd58de05ad5cfe396b612cf3b

commit d8b49cf0c891d09dd58de05ad5cfe396b612cf3b
Author: Yao Qi <yao.qi@linaro.org>
Date:   Thu Jan 26 14:29:20 2017 +0000

    Don't throw exception in dis_asm_memory_error
    
    Hi,
    GDB calls some APIs from opcodes to do disassembly and provide some
    call backs.  This model makes troubles on C++ exception unwinding,
    because GDB is a C++ program, and opcodes is still compiled as C.
    As we can see, frame #10 and #12 are C++, while #frame 11 is C,
    
     #10 0x0000000000544228 in memory_error (err=TARGET_XFER_E_IO, memaddr=<optimized out>) at ../../binutils-gdb/gdb/corefile.c:237
     #11 0x00000000006b0a54 in print_insn_aarch64 (pc=0, info=0xffffffffeeb0) at ../../binutils-gdb/opcodes/aarch64-dis.c:3185
     #12 0x0000000000553590 in gdb_pretty_print_insn (gdbarch=gdbarch@entry=0xbbceb0, uiout=uiout@entry=0xbc73d0, di=di@entry=0xffffffffeeb0,
        insn=0xffffffffed40, insn@entry=0xffffffffed90, flags=flags@entry=0,
    
    C++ exception unwinder can't go across frame #11 unless it has
    unwind table.  However, C program on many architectures doesn't
    have it in default.  As a result, GDB aborts, which is described
    in PR 20939.
    
    This is not the first time we see this kind of problem.  We've
    had a commit 89525768cd086a0798a504c81fdf7ebcd4c904e1
    "Propagate GDB/C++ exceptions across readline using sj/lj-based TRY/CATCH".
    We can fix the disassembly bug in a similar way, this is the option one.
    
    Since opcodes is built with gdb, we fix this problem in a different
    way as we did for the same issue with readline.  Instead of throwing
    exception in dis_asm_memory_error, we record the failed memory
    address, and throw exception when GDB returns from opcodes disassemblers.
    
    gdb:
    
    2017-01-26  Yao Qi  <yao.qi@linaro.org>
    	    Pedro Alves  <palves@redhat.com>
    
    	PR gdb/20939
    	* disasm.c (gdb_disassembler::dis_asm_memory_error): Don't
    	call memory_error, save memaddr instead.
    	(gdb_disassembler::print_insn): If gdbarch_print_insn returns
    	negative, cal memory_error.
    	* disasm.h (gdb_disassembler) <m_err_memaddr>: New field.
    
    gdb/testsuite:
    
    2017-01-26  Yao Qi  <yao.qi@linaro.org>
    
    	* gdb.base/all-architectures.exp.in (do_arch_tests): Test
    	disassemble on address 0.

Diff:
---
 gdb/ChangeLog                                   | 10 ++++++++++
 gdb/disasm.c                                    | 13 +++++++++++--
 gdb/disasm.h                                    |  1 +
 gdb/testsuite/ChangeLog                         |  5 +++++
 gdb/testsuite/gdb.base/all-architectures.exp.in |  5 +++++
 5 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2e8a96f..4e85fcd 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,4 +1,14 @@
 2017-01-26  Yao Qi  <yao.qi@linaro.org>
+	    Pedro Alves  <palves@redhat.com>
+
+	PR gdb/20939
+	* disasm.c (gdb_disassembler::dis_asm_memory_error): Don't
+	call memory_error, save memaddr instead.
+	(gdb_disassembler::print_insn): If gdbarch_print_insn returns
+	negative, cal memory_error.
+	* disasm.h (gdb_disassembler) <m_err_memaddr>: New field.
+
+2017-01-26  Yao Qi  <yao.qi@linaro.org>
 
 	* disasm-selftests.c (memory_error_test): New function.
 	(_initialize_disasm_selftests): Register memory_error_test.
diff --git a/gdb/disasm.c b/gdb/disasm.c
index b58da0f..897f2f1 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -135,7 +135,10 @@ void
 gdb_disassembler::dis_asm_memory_error (int err, bfd_vma memaddr,
 					struct disassemble_info *info)
 {
-  memory_error (TARGET_XFER_E_IO, memaddr);
+  gdb_disassembler *self
+    = static_cast<gdb_disassembler *>(info->application_data);
+
+  self->m_err_memaddr = memaddr;
 }
 
 /* Wrapper of print_address.  */
@@ -765,7 +768,8 @@ fprintf_disasm (void *stream, const char *format, ...)
 gdb_disassembler::gdb_disassembler (struct gdbarch *gdbarch,
 				    struct ui_file *file,
 				    di_read_memory_ftype read_memory_func)
-  : m_gdbarch (gdbarch)
+  : m_gdbarch (gdbarch),
+    m_err_memaddr (0)
 {
   init_disassemble_info (&m_di, file, fprintf_disasm);
   m_di.flavour = bfd_target_unknown_flavour;
@@ -792,8 +796,13 @@ int
 gdb_disassembler::print_insn (CORE_ADDR memaddr,
 			      int *branch_delay_insns)
 {
+  m_err_memaddr = 0;
+
   int length = gdbarch_print_insn (arch (), memaddr, &m_di);
 
+  if (length < 0)
+    memory_error (TARGET_XFER_E_IO, m_err_memaddr);
+
   if (branch_delay_insns != NULL)
     {
       if (m_di.insn_info_valid)
diff --git a/gdb/disasm.h b/gdb/disasm.h
index c6cb13b..7bbfa31 100644
--- a/gdb/disasm.h
+++ b/gdb/disasm.h
@@ -66,6 +66,7 @@ private:
   /* Stores data required for disassembling instructions in
      opcodes.  */
   struct disassemble_info m_di;
+  CORE_ADDR m_err_memaddr;
 
   static int dis_asm_read_memory (bfd_vma memaddr, gdb_byte *myaddr,
 				  unsigned int len,
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 28e337a..404b408 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2017-01-26  Yao Qi  <yao.qi@linaro.org>
+
+	* gdb.base/all-architectures.exp.in (do_arch_tests): Test
+	disassemble on address 0.
+
 2017-01-25  Andreas Arnez  <arnez@linux.vnet.ibm.com>
 	    Yao Qi  <yao.qi@linaro.org>
 
diff --git a/gdb/testsuite/gdb.base/all-architectures.exp.in b/gdb/testsuite/gdb.base/all-architectures.exp.in
index c7615ac..b07caf5 100644
--- a/gdb/testsuite/gdb.base/all-architectures.exp.in
+++ b/gdb/testsuite/gdb.base/all-architectures.exp.in
@@ -152,6 +152,11 @@ proc print_floats {} {
 
 proc do_arch_tests {} {
     print_floats
+
+    # GDB can't access memory because there is no loaded executable
+    # nor live inferior.
+    gdb_test_internal "disassemble 0x0,+4" \
+	"Cannot access memory at address 0x0"
 }
 
 # Given we can't change arch, osabi, endianness, etc. atomically, we