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 13:09:56 -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> <wwoky3zzwtwx.fsf@ericsson.com> <wwokwpfjwsop.fsf@ericsson.com> <wwoktwanwrlo.fsf@ericsson.com>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
Antoine Tremblay writes:
> Antoine Tremblay writes:
>
>> Antoine Tremblay writes:
>>
>>> 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.
>>>
>>
>> Sorry I got confused with the code patched with the latest 2 patches I
>> sent refactoring the single stepping code.
>>
>> Considering the current code this is handled by the step-over process,
>> and should not be an issue as it will always step-over before installing
>> any single-step breakpoints.
>>
>> And step-over removes all breakpoints when stepping over thus
>> get_next_pc is ok.
>>
>> This becomes an issue like I said before with
>> https://sourceware.org/ml/gdb-patches/2016-11/msg00939.html
>>
>> Since with this it's possible to install single-step breakpoints without
>> a step-over check.
>>
>> We could consider this patch a preparation for
>> https://sourceware.org/ml/gdb-patches/2016-11/msg00939.html
>>
>> or just a good pratice to use target_read_memory.
>>
>> Thanks,
>> Antoine
>
> Just to supplement about:
> https://sourceware.org/ml/gdb-patches/2016-11/msg00939.html
>
> If we consider this patch the is 2 reasons we can't install step over
> breakpoints.
>
> One is to be able to delay a step-over.
>
> The other is since GDBServer inserts single-step breakpoints when it
> processes the resume requests and threads are about to be resumed. If
> threads still have pending status, single-step breakpoints are not
> installed, so it needs to install them in proceed_all_lwp. And in this
> case the single-step breakpoints are inserted outside of a step-over
> process.
After some more thought, it can happen even with current code too that
single step breakpoints are installed without a step-over.
Consider this situation:
In non-stop:
the user issues:
thread 1
step&
thread 2
step&
thread 3
step&
In a similar way as non-stop-fair-events.exp (threads are looping).
GDBServer:
linux_resume is called
GDBServer has pending events,
threads are not resumed and single-step breakpoint for thread 1 not installed.
linux_wait_1 is called with a pending event on thread 2 at pc A
GDBServer handles the event and calls proceed_all_lwps
This calls proceed_one_lwp and installs single-step breakpoints on all
the threads that need one.
Now since thread 1 needs to install a single-step breakpoint and is at pc B
(different than thread 2), a step-over is not initiated and get_next_pc
is called to figure out the next instruction from pc B.
However it may just be that thread 3 as a single step breakpoint at pc
B. And thus get_next_pc fails.
This situation is tested with non-stop-fair-events.exp.
Sorry for the confusion, you can consider only the two last replies as
valid.