Re: [patch] gdbarch_syscall_pc_increment

[Pedro Alves <palves at redhat dot com>]

On 12/14/2012 02:23 PM, Aleksandar Ristovski wrote:
> On 12-12-13 04:47 PM, Pedro Alves wrote:
>> On 12/13/2012 02:07 PM, Aleksandar Ristovski wrote:
>>
>>> In this patch, we are talking about two things:
>>>
>>> a) the concept of syscall behaving as a conditional branch with uncomputable condition (i.e. gdb can not really compute whether the branch will be taken or not). This is a concept specific to a given kernel implementation of system calls and as such belongs to gdbarch.
>>>
>>> b) The amount by which pc may be incremented if the branch is taken. For an os that has a), this is cpu specific. It may also be dependent on the instruction set mode on e.g. arm (this is why frame is being passed).
>>>
>>> Where does this belong: in the case here, the line is a bit blurry since a) is generic concept applicable for all cpus for a given os thus definitely belonging to gdbarch. b) is cpu concept and strictly speaking belongs to gdbarch_tdep. However, since a) can also answer b) I see no point in further breaking it down.
>>
>> The interface is not pretty, but you can just install an additional
>> breakpoint from your OS's tdep->syscall_next_pc hook or
>> gdbarch_software_single_step wrapper hook.  See e.g.,:
>>
>> /* Find the next PC after the current instruction executes.  In some
>>     cases we can not statically determine the answer (see the IT state
>>     handling in this function); in that case, a breakpoint may be
>>     inserted in addition to the returned PC, which will be used to set
>>     another breakpoint by our caller.  */
>>
>> static CORE_ADDR
>> thumb_get_next_pc_raw (struct frame_info *frame, CORE_ADDR pc)
>> {
>> ...
>>           /* Set a breakpoint on the following instruction.  */
>>           gdb_assert ((itstate & 0x0f) != 0);
>>           arm_insert_single_step_breakpoint (gdbarch, aspace,
>>                          MAKE_THUMB_ADDR (pc));
>> ...
>>
>> I believe other ports do similar things in similar cases.
> 
> 
> This seems just wrong to me, to install a breakpoint when answering a question.

I did say the interface wasn't pretty.  :-) "get_next_pc" returning a single address
is what IMO is broken.  These hooks should be returning a list of locations, so that
the callers could install the multiple breakpoints themselves.

-- 
Pedro Alves