Re: [PATCH 1/3] Fix inferior memory reading in GDBServer for arm/aarch32.

[Antoine Tremblay <antoine.tremblay@ericsson.com>]

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.