[PATCH 1/3] Fix inferior memory reading in GDBServer for arm/aarch32.
Thu Dec 1 15:28:00 GMT 2016
Yao Qi writes:
> On Mon, Nov 28, 2016 at 07:27:56AM -0500, Antoine Tremblay wrote:
>> Before this patch, some functions would read the inferior memory with
>> (*the_target)->read_memory, which returns the raw memory, rather than the
>> shadowed memory.
>> This is wrong since these functions do not expect to read a breakpoint
>> instruction and can lead to invalid behavior.
>> Use of raw memory in get_next_pcs_read_memory_unsigned_integer for example
>> could lead to get_next_pc returning an invalid pc.
> Can you elaborate under what circumstance breakpoints are still in memory
> when these functions are called? Can we have a test case?
Here is an example:
In non-stop mode multiple threads are stepping, like in the
thread 1 is at instruction A
installs single step breakpoint on instruction B
thread 2 is at instruction B
GDBServer needs to install a single step breakpoint at the next
instruction from B.
To do so get_next_pc is called, but since the single step
breakpoint for thread 1 at instruction B is there. get_next_pc
reads the current instruction as a breakpoint instruction and fails.
Note that I used a user driven example here to make it more clear but
this is also true while range-stepping in a loop for example:
- thread 1 hits its single-step breakpoint deletes it
- it's not out of a range-step so
- tries to install a single-step breakpoint at the next
- but thread 2 has a breakpoint at thread 1's current
instruction and get_next_pc fails.
This is already tested by non-stop-fair-events.exp, the test will fail
without this patch.
Note that this test is testing both range-stepping and the user
>> @@ -769,15 +770,15 @@ arm_sigreturn_next_pc (struct regcache *regcache, int svc_number,
>> gdb_assert (svc_number == __NR_sigreturn || svc_number == __NR_rt_sigreturn);
>> collect_register_by_name (regcache, "sp", &sp);
>> - (*the_target->read_memory) (sp, (unsigned char *) &sp_data, 4);
>> + target_read_memory (sp, (unsigned char *) &sp_data, 4);
>> pc_offset = arm_linux_sigreturn_next_pc_offset
>> (sp, sp_data, svc_number, __NR_sigreturn == svc_number ? 1 : 0);
>> - (*the_target->read_memory) (sp + pc_offset, (unsigned char *) &next_pc, 4);
>> + target_read_memory (sp + pc_offset, (unsigned char *) &next_pc, 4);
>> /* Set IS_THUMB according the CPSR saved on the stack. */
>> - (*the_target->read_memory) (sp + pc_offset + 4, (unsigned char *) &cpsr, 4);
>> + target_read_memory (sp + pc_offset + 4, (unsigned char *) &cpsr, 4);
>> *is_thumb = ((cpsr & CPSR_T) != 0);
> We are reading from stack, so we don't need to check weather there is
> a breakpoint or not.
Ho right, is it worth it to make the distinction however ?
I mean, would it be better general practice to use target_read_memory
unless we absolutely need to use the_target->read_memory like with
breakpoint_at funcs.. ? The counterpart looks more error prone for the
More information about the Gdb-patches