disas: Always initialize read_memory_inner_func properly

PATCH Applied Thomas Huth 3 participants 4 messages Sep 27, 2017 – Oct 10, 2017

lore

Share & tools

lore

Thomas Huth

author

2017-09-27 14:58 UTC #0 lore raw

PATCH 1/1 disas: Always initialize read_memory_inner_func properly

I've recently seen this with valgrind while running the HMP tester:

==22373== Conditional jump or move depends on uninitialised value(s)

==22373==    at 0x4A41FD: arm_disas_set_info (cpu.c:504)
==22373==    by 0x3867A7: monitor_disas (disas.c:390)
==22373==    by 0x38E80E: memory_dump (monitor.c:1339)
==22373==    by 0x38FA43: handle_hmp_command (monitor.c:3123)
==22373==    by 0x38FB9E: qmp_human_monitor_command (monitor.c:613)
==22373==    by 0x4E3124: qmp_marshal_human_monitor_command (qmp-marshal.c:1736)
==22373==    by 0x769678: do_qmp_dispatch (qmp-dispatch.c:104)
==22373==    by 0x769678: qmp_dispatch (qmp-dispatch.c:131)
==22373==    by 0x38B734: handle_qmp_command (monitor.c:3853)
==22373==    by 0x76ED07: json_message_process_token (json-streamer.c:105)
==22373==    by 0x78D40A: json_lexer_feed_char (json-lexer.c:323)
==22373==    by 0x78D4CD: json_lexer_feed (json-lexer.c:373)
==22373==    by 0x38A08D: monitor_qmp_read (monitor.c:3895)

And indeed, in monitor_disas, the read_memory_inner_func variable was not initialized, but arm_disas_set_info() expects this to be NULL or a valid pointer. Let's properly set this to NULL in the INIT_DISASSEMBLE_INFO to fix it in all functions that use the disassemble_info struct.

Fixes f7478a92dd9ee2276bfaa5b7317140d3f9d6a53b ("Fix Thumb-1 BE32 execution")

Signed-off-by Thomas Huth <[email protected]>

---
 disas.c             | 1 -
 include/disas/bfd.h | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

disas.c +0 -1

index d6a1eb9..54eea3f 100644
@@ -190,7 +190,6 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
 
     s.cpu = cpu;
     s.info.read_memory_func = target_read_memory;
-    s.info.read_memory_inner_func = NULL;
     s.info.buffer_vma = code;
     s.info.buffer_length = size;
     s.info.print_address_func = generic_print_address;

include/disas/bfd.h +1 -0

index b01e002..d99da68 100644
@@ -479,6 +479,7 @@ int generic_symbol_at_address(bfd_vma, struct disassemble_info *);
   (INFO).buffer_vma = 0, \
   (INFO).buffer_length = 0, \
   (INFO).read_memory_func = buffer_read_memory, \
+  (INFO).read_memory_inner_func = NULL, \
   (INFO).memory_error_func = perror_memory, \
   (INFO).print_address_func = generic_print_address, \
   (INFO).print_insn = NULL, \

Signature · 1 line

1.8.3.1

Richard Henderson

Reviewed-by on v1 1/1

2017-09-28 17:19 UTC #1 lore raw

Quoted 23 lines · Thomas Huth

I've recently seen this with valgrind while running the HMP tester:
==22373== Conditional jump or move depends on uninitialised value(s)
==22373==    at 0x4A41FD: arm_disas_set_info (cpu.c:504)
==22373==    by 0x3867A7: monitor_disas (disas.c:390)
==22373==    by 0x38E80E: memory_dump (monitor.c:1339)
==22373==    by 0x38FA43: handle_hmp_command (monitor.c:3123)
==22373==    by 0x38FB9E: qmp_human_monitor_command (monitor.c:613)
==22373==    by 0x4E3124: qmp_marshal_human_monitor_command (qmp-marshal.c:1736)
==22373==    by 0x769678: do_qmp_dispatch (qmp-dispatch.c:104)
==22373==    by 0x769678: qmp_dispatch (qmp-dispatch.c:131)
==22373==    by 0x38B734: handle_qmp_command (monitor.c:3853)
==22373==    by 0x76ED07: json_message_process_token (json-streamer.c:105)
==22373==    by 0x78D40A: json_lexer_feed_char (json-lexer.c:323)
==22373==    by 0x78D4CD: json_lexer_feed (json-lexer.c:373)
==22373==    by 0x38A08D: monitor_qmp_read (monitor.c:3895)
And indeed, in monitor_disas, the read_memory_inner_func variable was not initialized, but arm_disas_set_info() expects this to be NULL or a valid pointer. Let's properly set this to NULL in the INIT_DISASSEMBLE_INFO to fix it in all functions that use the disassemble_info struct.

Quoted 6 lines

Fixes f7478a92dd9ee2276bfaa5b7317140d3f9d6a53b ("Fix Thumb-1 BE32 execution")
Signed-off-by Thomas Huth <[email protected]>
---
 disas.c             | 1 -
 include/disas/bfd.h | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

Reviewed-by Richard Henderson <[email protected]>

Thomas Huth

author

2017-10-10 08:30 UTC #2 lore raw

In reply to Thomas Huth

Quoted 48 lines · Thomas Huth

I've recently seen this with valgrind while running the HMP tester:

==22373== Conditional jump or move depends on uninitialised value(s)

==22373==    at 0x4A41FD: arm_disas_set_info (cpu.c:504)
==22373==    by 0x3867A7: monitor_disas (disas.c:390)
==22373==    by 0x38E80E: memory_dump (monitor.c:1339)
==22373==    by 0x38FA43: handle_hmp_command (monitor.c:3123)
==22373==    by 0x38FB9E: qmp_human_monitor_command (monitor.c:613)
==22373==    by 0x4E3124: qmp_marshal_human_monitor_command (qmp-marshal.c:1736)
==22373==    by 0x769678: do_qmp_dispatch (qmp-dispatch.c:104)
==22373==    by 0x769678: qmp_dispatch (qmp-dispatch.c:131)
==22373==    by 0x38B734: handle_qmp_command (monitor.c:3853)
==22373==    by 0x76ED07: json_message_process_token (json-streamer.c:105)
==22373==    by 0x78D40A: json_lexer_feed_char (json-lexer.c:323)
==22373==    by 0x78D4CD: json_lexer_feed (json-lexer.c:373)
==22373==    by 0x38A08D: monitor_qmp_read (monitor.c:3895)

Fixes f7478a92dd9ee2276bfaa5b7317140d3f9d6a53b ("Fix Thumb-1 BE32 execution")

Signed-off-by Thomas Huth <[email protected]>

---
 disas.c             | 1 -
 include/disas/bfd.h | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

disas.c +0 -1

index d6a1eb9..54eea3f 100644
@@ -190,7 +190,6 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
 
     s.cpu = cpu;
     s.info.read_memory_func = target_read_memory;
-    s.info.read_memory_inner_func = NULL;
     s.info.buffer_vma = code;
     s.info.buffer_length = size;
     s.info.print_address_func = generic_print_address;

include/disas/bfd.h +0 -0

index b01e002..d99da68 100644

include/disas/bfd.h int generic_symbol_at_address(bfd_vma, struct disassemble_info *); · line 485

Quoted 7 lines

include/disas/bfd.h +1 -0

@@ -479,6 +479,7 @@ int generic_symbol_at_address(bfd_vma, struct disassemble_info *);
   (INFO).buffer_vma = 0, \
   (INFO).buffer_length = 0, \
   (INFO).read_memory_func = buffer_read_memory, \
+  (INFO).read_memory_inner_func = NULL, \
   (INFO).memory_error_func = perror_memory, \
   (INFO).print_address_func = generic_print_address, \
   (INFO).print_insn = NULL, \

Ping?

Thomas

Paolo Bonzini

Applied

2017-10-10 13:24 UTC #3 lore raw

In reply to Thomas Huth

Quoted 48 lines · Thomas Huth

I've recently seen this with valgrind while running the HMP tester:

==22373== Conditional jump or move depends on uninitialised value(s)

==22373==    at 0x4A41FD: arm_disas_set_info (cpu.c:504)
==22373==    by 0x3867A7: monitor_disas (disas.c:390)
==22373==    by 0x38E80E: memory_dump (monitor.c:1339)
==22373==    by 0x38FA43: handle_hmp_command (monitor.c:3123)
==22373==    by 0x38FB9E: qmp_human_monitor_command (monitor.c:613)
==22373==    by 0x4E3124: qmp_marshal_human_monitor_command (qmp-marshal.c:1736)
==22373==    by 0x769678: do_qmp_dispatch (qmp-dispatch.c:104)
==22373==    by 0x769678: qmp_dispatch (qmp-dispatch.c:131)
==22373==    by 0x38B734: handle_qmp_command (monitor.c:3853)
==22373==    by 0x76ED07: json_message_process_token (json-streamer.c:105)
==22373==    by 0x78D40A: json_lexer_feed_char (json-lexer.c:323)
==22373==    by 0x78D4CD: json_lexer_feed (json-lexer.c:373)
==22373==    by 0x38A08D: monitor_qmp_read (monitor.c:3895)

Fixes f7478a92dd9ee2276bfaa5b7317140d3f9d6a53b ("Fix Thumb-1 BE32 execution")

Signed-off-by Thomas Huth <[email protected]>

---
 disas.c             | 1 -
 include/disas/bfd.h | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

disas.c +0 -1

index d6a1eb9..54eea3f 100644
@@ -190,7 +190,6 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
 
     s.cpu = cpu;
     s.info.read_memory_func = target_read_memory;
-    s.info.read_memory_inner_func = NULL;
     s.info.buffer_vma = code;
     s.info.buffer_length = size;
     s.info.print_address_func = generic_print_address;

include/disas/bfd.h +0 -0

index b01e002..d99da68 100644

include/disas/bfd.h int generic_symbol_at_address(bfd_vma, struct disassemble_info *); · line 485

Quoted 7 lines

include/disas/bfd.h +1 -0

@@ -479,6 +479,7 @@ int generic_symbol_at_address(bfd_vma, struct disassemble_info *);
   (INFO).buffer_vma = 0, \
   (INFO).buffer_length = 0, \
   (INFO).read_memory_func = buffer_read_memory, \
+  (INFO).read_memory_inner_func = NULL, \
   (INFO).memory_error_func = perror_memory, \
   (INFO).print_address_func = generic_print_address, \
   (INFO).print_insn = NULL, \

Queued, thanks.

Paolo