[PATCH 1/3] Fix inferior memory reading in GDBServer for arm/aarch32.
Thu Dec 1 18:10:00 GMT 2016
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.
>>> thread 1
>>> thread 1 is at instruction A
>>> installs single step breakpoint on instruction B
>>> thread 2
>>> 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
>> 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
>> Since with this it's possible to install single-step breakpoints without
>> a step-over check.
>> We could consider this patch a preparation for
>> or just a good pratice to use target_read_memory.
> Just to supplement about:
> If we consider this patch the is 2 reasons we can't install step over
> 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
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:
the user issues:
In a similar way as non-stop-fair-events.exp (threads are looping).
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
More information about the Gdb-patches