Re: [patch] gdbarch_syscall_pc_increment

[Aleksandar Ristovski <aristovski@qnx.com>]

On 12-12-12 08:32 PM, Yao Qi wrote:
> On 12/12/2012 11:42 PM, Aleksandar Ristovski wrote:
>>
>> This is generic for a given OS that happens to increment instruction
>> pointer to allow user code to e.g. set errno.
>>
>> I provided only arm implementation, but other target cpus would need the
>> same if they implement software single stepping.
>>
>> Increment is cpu specific for a given architecture.
>>
>
> 'software single step' is implemented differently in the backend of each
> port and your 'syscall_pc_increment' depends on the arch as well,  so a
> gdbarch hook is not needed here.
>
> 'gdbarch' stands for a certain general architecture, such as arm, mips,
> and etc.  'gdbarch_tdep' contains the details of the cpus under this
> architecture.

Yes, I see your point, though I would say gdbarch stands for a certain 
architecture such as arm-nto, arm-linux, etc... therefore, cpu/os 
combination, not cpu only.

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.

Of course, the alternative is to not introduce concept from a) at all 
and do everything in my cpu-os-tdep.c which is how I have it set up now 
in our port.

>
> 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.

---
Aleksandar