Re: [patch] gdbarch_syscall_pc_increment

[Aleksandar Ristovski <aristovski@qnx.com>]

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.


>
>>> b.t.w, I don't see how 'set_gdbarch_syscall_pc_increment' is called in
>>> your patch.
>>>
>>
>> No, as we have not yet contributed arm-nto-tdep.c to FSF.
>
> When do you plan to do so?  I'm afraid we shouldn't be accepting
> unused hooks in the tree.
>

Well, I plan to, but can't provide any ETA.

No problem - there is no urgency in this, at least we have exchanged 
thoughts on where would this fit (it doesn't fit anywhere ATM :-) ). 
I'll stick with providing my own software_single_step.


Consider the patch abandoned.


Thanks,

Aleksandar