[6/6] Don't throw exception in dis_asm_memory_error
Commit Message
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-10 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-10 Yao Qi <yao.qi@linaro.org>
* gdb.base/all-architectures.exp.in (do_arch_tests): Test
disassemble on address 0.
---
gdb/disasm.c | 13 +++++++++++--
gdb/disasm.h | 1 +
gdb/testsuite/gdb.base/all-architectures.exp.in | 3 +++
3 files changed, 15 insertions(+), 2 deletions(-)
Comments
On 01/16/2017 04:02 AM, Yao Qi wrote:
> 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-10 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-10 Yao Qi <yao.qi@linaro.org>
>
> * gdb.base/all-architectures.exp.in (do_arch_tests): Test
> disassemble on address 0.
> ---
> gdb/disasm.c | 13 +++++++++++--
> gdb/disasm.h | 1 +
> gdb/testsuite/gdb.base/all-architectures.exp.in | 3 +++
> 3 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/disasm.c b/gdb/disasm.c
> index f31d8d3..8c8c42e 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;
> }
>
> /* Like print_address with slightly different parameters. */
> @@ -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 5122fa3..8e0b9f9 100644
> --- a/gdb/disasm.h
> +++ b/gdb/disasm.h
> @@ -63,6 +63,7 @@ protected:
> private:
> struct gdbarch *m_gdbarch;
> 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/gdb.base/all-architectures.exp.in b/gdb/testsuite/gdb.base/all-architectures.exp.in
> index c7615ac..50a615c 100644
> --- a/gdb/testsuite/gdb.base/all-architectures.exp.in
> +++ b/gdb/testsuite/gdb.base/all-architectures.exp.in
> @@ -152,6 +152,9 @@ proc print_floats {} {
>
> proc do_arch_tests {} {
> print_floats
> +
> + gdb_test_internal "disassemble 0x0,+4" \
> + "Cannot access memory at address 0x0"
I think you missed the comment you had proposed?
# GDB can't access any memory because there is no live inferior.
I take it we have no loaded executable as well? Otherwise we could
eventually read data from the executable file, which wouldn't yield an
error.
> }
>
> # Given we can't change arch, osabi, endianness, etc. atomically, we
>
On 17-01-17 08:42:24, Luis Machado wrote:
> >+ gdb_test_internal "disassemble 0x0,+4" \
> >+ "Cannot access memory at address 0x0"
>
> I think you missed the comment you had proposed?
>
Sorry.
> # GDB can't access any memory because there is no live inferior.
>
> I take it we have no loaded executable as well? Otherwise we could
> eventually read data from the executable file, which wouldn't yield
> an error.
How about this?
# GDB can't access memory because there is no loaded executable
# nor live inferior.
On 01/18/2017 08:54 AM, Yao Qi wrote:
> On 17-01-17 08:42:24, Luis Machado wrote:
>>> + gdb_test_internal "disassemble 0x0,+4" \
>>> + "Cannot access memory at address 0x0"
>>
>> I think you missed the comment you had proposed?
>>
>
> Sorry.
>
>> # GDB can't access any memory because there is no live inferior.
>>
>> I take it we have no loaded executable as well? Otherwise we could
>> eventually read data from the executable file, which wouldn't yield
>> an error.
>
> How about this?
>
> # GDB can't access memory because there is no loaded executable
> # nor live inferior.
>
Looks good to me.
@@ -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;
}
/* Like print_address with slightly different parameters. */
@@ -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)
@@ -63,6 +63,7 @@ protected:
private:
struct gdbarch *m_gdbarch;
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,
@@ -152,6 +152,9 @@ proc print_floats {} {
proc do_arch_tests {} {
print_floats
+
+ gdb_test_internal "disassemble 0x0,+4" \
+ "Cannot access memory at address 0x0"
}
# Given we can't change arch, osabi, endianness, etc. atomically, we