This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/3] Fix inferior memory reading in GDBServer for arm/aarch32.
- From: Antoine Tremblay <antoine dot tremblay at ericsson dot com>
- To: Yao Qi <qiyaoltc at gmail dot com>
- Cc: Antoine Tremblay <antoine dot tremblay at ericsson dot com>, <gdb-patches at sourceware dot org>
- Date: Thu, 1 Dec 2016 10:28:14 -0500
- Subject: Re: [PATCH 1/3] Fix inferior memory reading in GDBServer for arm/aarch32.
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=none (sender IP is ) smtp.mailfrom=antoine dot tremblay at ericsson dot com;
- References: <20161128122758.7762-1-antoine.tremblay@ericsson.com> <20161201144401.GA19289@E107787-LIN>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
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
non-stop-fair-events.exp test.
GDB:
thread 1
step&
GDBServer:
thread 1 is at instruction A
installs single step breakpoint on instruction B
GDB:
thread 2
step&
GDBServer:
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
instruction
- 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
stepping.
>> @@ -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
developer...