Hi everyone,
I've been trying to port strace to Nios-II, but came across some oddities
in the ptrace ABI. The pt_regs structure is exposed to userspace through
the ptrace.h UAPI header: arch/nios2/include/uapi/asm/ptrace.h
Such pt_regs has the following layout:
struct pt_regs {
unsigned long r8;
unsigned long r9;
unsigned long r10;
[and so on]
}
But the regset implementation in arch/nios2/kernel/ptrace.c
pushes a different layout to userspace, as it uses the PTR_
macros, also in the UAPI header:
#define PTR_R0 0
#define PTR_R1 1
#define PTR_R2 2
#define PTR_R3 3
#define PTR_R4 4
#define PTR_R5 5
#define PTR_R6 6
#define PTR_R7 7
#define PTR_R8 8
[..]
In other words, the PTRACE_GETREGSET will pass to userspace register
r8 at offset 8*4, but the pt_regs structure says it's at offset 0.
This difference appears because ptrace returns one layout to userspace,
and pushes the registers to the stack with another layout.
I've fixed this and managed to port strace by changing the genregs_get
implementation, so it returns a layout that matches pt_regs, and therefore
matches the stack.
Having this one-to-one correspondence, has a nice benefit. The implementation
is trivial:
static int genregs_get(struct task_struct *target,
const struct user_regset *regset,
unsigned int pos, unsigned int count,
void *kbuf, void __user *ubuf)
{
const struct pt_regs *regs = task_pt_regs(target);
const struct switch_stack *sw = (struct switch_stack *)regs - 1;
int ret;
ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
regs, 0, PT_REGS_SIZE);
if (!ret)
ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
sw, 0, SWITCH_STACK_SIZE);
return ret;
}
However, there's a problem. The ABI is already different, and current gdb
seems to rely in the current layout. However, it does it by *not* using the
pt_regs structure.
Give the ABI is already used, I'm reluctant to change it as I don't want to
break our beloved users.
So, tried a different approach and removed pt_regs from the UAPI ptrace.h,
replacing it with a new user_regs that describes how registers are passed
to user. This however is also problematic, as pt_regs is already used
by glibc (not really sure what for).
In conclusion, I'm lost and have no clue how a proper fix
would look like. (Or perhaps, I'm really lost and there's no fix needed.)
Any ideas?
--
Ezequiel Garcia, VanguardiaSur
http://www.vanguardiasur.com.ar
On Tuesday 24 February 2015 00:04:21 Ezequiel Garcia wrote:
> So, tried a different approach and removed pt_regs from the UAPI ptrace.h,
> replacing it with a new user_regs that describes how registers are passed
> to user. This however is also problematic, as pt_regs is already used
> by glibc (not really sure what for).
>
I've looked at glibc and could not find a use for pt_regs there. Where
did you find it? It's quite possible that it's incorrect as well
if the structures don't match.
Arnd
Hi Arnd,
On 02/24/2015 05:54 AM, Arnd Bergmann wrote:
> On Tuesday 24 February 2015 00:04:21 Ezequiel Garcia wrote:
>> So, tried a different approach and removed pt_regs from the UAPI ptrace.h,
>> replacing it with a new user_regs that describes how registers are passed
>> to user. This however is also problematic, as pt_regs is already used
>> by glibc (not really sure what for).
>>
>
> I've looked at glibc and could not find a use for pt_regs there. Where
> did you find it? It's quite possible that it's incorrect as well
> if the structures don't match.
>
Gah, no, you are right. I got confused.
So it would be OK to avoid remove pt_regs from the uapi headers?
How does this affect the signal handling nios2 implementation?
--
Ezequiel Garcia, VanguardiaSur
http://www.vanguardiasur.com.ar
On Tuesday 24 February 2015 12:28:41 Ezequiel Garcia wrote:
>
> Gah, no, you are right. I got confused.
>
> So it would be OK to avoid remove pt_regs from the uapi headers?
> How does this affect the signal handling nios2 implementation?
>
We have a number of architectures that don't provide this structure:
$ git grep -L pt_regs arch/*/include/uapi/asm/ptrace.h
arch/frv/include/uapi/asm/ptrace.h
arch/metag/include/uapi/asm/ptrace.h
arch/openrisc/include/uapi/asm/ptrace.h
arch/s390/include/uapi/asm/ptrace.h
so I'd assume it's ok in general not to have it. However, on
nios2, struct pt_regs is embedded inside of struct sigcontext.
If I read the code in arch/nios2/kernel/signal.c correctly,
this is actually a bug and you should use a different structure
there too, because pt_regs does not match the layout of the
stack either. This means that the (rare) user programs that
would know about the architecture to modify signal stacks
are currently broken.
Arnd
On 02/24/2015 04:25 PM, Arnd Bergmann wrote:
> On Tuesday 24 February 2015 12:28:41 Ezequiel Garcia wrote:
>>
>> Gah, no, you are right. I got confused.
>>
>> So it would be OK to avoid remove pt_regs from the uapi headers?
>> How does this affect the signal handling nios2 implementation?
>>
>
> We have a number of architectures that don't provide this structure:
>
> $ git grep -L pt_regs arch/*/include/uapi/asm/ptrace.h
> arch/frv/include/uapi/asm/ptrace.h
> arch/metag/include/uapi/asm/ptrace.h
> arch/openrisc/include/uapi/asm/ptrace.h
> arch/s390/include/uapi/asm/ptrace.h
>
> so I'd assume it's ok in general not to have it. However, on
> nios2, struct pt_regs is embedded inside of struct sigcontext.
> If I read the code in arch/nios2/kernel/signal.c correctly,
> this is actually a bug and you should use a different structure
> there too, because pt_regs does not match the layout of the
> stack either. This means that the (rare) user programs that
> would know about the architecture to modify signal stacks
> are currently broken.
>
/me is more confused now
In arch/nios2/include/asm/ucontext.h
struct ucontext {
unsigned long uc_flags;
struct ucontext *uc_link;
stack_t uc_stack;
struct mcontext uc_mcontext;
sigset_t uc_sigmask;
};
And in include/uapi/asm-generic/ucontext.h:
struct ucontext {
unsigned long uc_flags;
struct ucontext *uc_link;
stack_t uc_stack;
struct sigcontext uc_mcontext;
sigset_t uc_sigmask;
};
Which one is the one that userspace sees? And why does the kernel has
two different structures?
Given this oddities, I'm wondering how troublesome would be to just
re-do this and break the ptrace and signal ABI. For instance, just
pushing pt_regs in PTRACE_GETREGSET would make things much clearer.
I guess Linus would burn me for even suggesting to breaking users... but
do we have any users at all? This arch has just been mainlined. Altera's
out-of-tree is already ABI-incompatible with mainline so that's not an
issue.
The only one using this ABI is gdb, which is easily fixed.
--
Ezequiel Garcia, VanguardiaSur
http://www.vanguardiasur.com.ar
On Wednesday 25 February 2015 08:33:16 Ezequiel Garcia wrote:
>
> /me is more confused now
>
> In arch/nios2/include/asm/ucontext.h
>
> struct ucontext {
> unsigned long uc_flags;
> struct ucontext *uc_link;
> stack_t uc_stack;
> struct mcontext uc_mcontext;
> sigset_t uc_sigmask;
> };
>
> And in include/uapi/asm-generic/ucontext.h:
>
> struct ucontext {
> unsigned long uc_flags;
> struct ucontext *uc_link;
> stack_t uc_stack;
> struct sigcontext uc_mcontext;
> sigset_t uc_sigmask;
> };
>
> Which one is the one that userspace sees? And why does the kernel has
> two different structures?
Userspace sees the asm-generic header, which I assume is a bug
in this case.
> Given this oddities, I'm wondering how troublesome would be to just
> re-do this and break the ptrace and signal ABI. For instance, just
> pushing pt_regs in PTRACE_GETREGSET would make things much clearer.
Could you change pt_regs to match the layout you have for PTRACE_GETREGSET
instead? It seems much more intuitive.
> I guess Linus would burn me for even suggesting to breaking users... but
> do we have any users at all? This arch has just been mainlined. Altera's
> out-of-tree is already ABI-incompatible with mainline so that's not an
> issue.
>
> The only one using this ABI is gdb, which is easily fixed.
You can change anything you like as long as nobody complains about
regressions.
Arnd
On 15/2/25 10:07 PM, Arnd Bergmann wrote:
> On Wednesday 25 February 2015 08:33:16 Ezequiel Garcia wrote:
>>
>> /me is more confused now
>>
>> In arch/nios2/include/asm/ucontext.h
>>
>> struct ucontext {
>> unsigned long uc_flags;
>> struct ucontext *uc_link;
>> stack_t uc_stack;
>> struct mcontext uc_mcontext;
>> sigset_t uc_sigmask;
>> };
>>
>> And in include/uapi/asm-generic/ucontext.h:
>>
>> struct ucontext {
>> unsigned long uc_flags;
>> struct ucontext *uc_link;
>> stack_t uc_stack;
>> struct sigcontext uc_mcontext;
>> sigset_t uc_sigmask;
>> };
>>
>> Which one is the one that userspace sees? And why does the kernel has
>> two different structures?
>
> Userspace sees the asm-generic header, which I assume is a bug
> in this case.
Yes, I believe nios2 doesn't not need this asm-generic/ucontext.h
header; OTOH it just isn't used; no real harm done, so easily fixed.
>> Given this oddities, I'm wondering how troublesome would be to just
>> re-do this and break the ptrace and signal ABI. For instance, just
>> pushing pt_regs in PTRACE_GETREGSET would make things much clearer.
>
> Could you change pt_regs to match the layout you have for PTRACE_GETREGSET
> instead? It seems much more intuitive.
There is a reason for this pt_regs arrangement: the nios2 syscall
interface uses r4-r9 for parameters, while the usual C conventions use
only r4-r7, placing r8-r9 at the start of pt_regs creates a natural
stack layout for entering C code after the asm shims in entry.S
>> I guess Linus would burn me for even suggesting to breaking users... but
>> do we have any users at all? This arch has just been mainlined. Altera's
>> out-of-tree is already ABI-incompatible with mainline so that's not an
>> issue.
>>
>> The only one using this ABI is gdb, which is easily fixed.
>
> You can change anything you like as long as nobody complains about
> regressions.
PTRACE_GET/SETREGSET is a new feature in nios2-linux that we're still
about to support in upstream GDB, so things could be fixed if needed,
but why can't you just use the [0...] ordering in userspace?
BTW, it's even that way in signal stacks as well; nios2 does not
use/export sigcontext inside struct ucontext. We just use a int[32]
array there.
Thanks,
Chung-Lin
Hi Chung-Lin Tang,
On 02/27/2015 05:57 AM, Chung-Lin Tang wrote:
> On 15/2/25 10:07 PM, Arnd Bergmann wrote:
>> On Wednesday 25 February 2015 08:33:16 Ezequiel Garcia wrote:
>>>
>>> /me is more confused now
>>>
>>> In arch/nios2/include/asm/ucontext.h
>>>
>>> struct ucontext {
>>> unsigned long uc_flags;
>>> struct ucontext *uc_link;
>>> stack_t uc_stack;
>>> struct mcontext uc_mcontext;
>>> sigset_t uc_sigmask;
>>> };
>>>
>>> And in include/uapi/asm-generic/ucontext.h:
>>>
>>> struct ucontext {
>>> unsigned long uc_flags;
>>> struct ucontext *uc_link;
>>> stack_t uc_stack;
>>> struct sigcontext uc_mcontext;
>>> sigset_t uc_sigmask;
>>> };
>>>
>>> Which one is the one that userspace sees? And why does the kernel has
>>> two different structures?
>>
>> Userspace sees the asm-generic header, which I assume is a bug
>> in this case.
>
> Yes, I believe nios2 doesn't not need this asm-generic/ucontext.h
> header; OTOH it just isn't used; no real harm done, so easily fixed.
>
>>> Given this oddities, I'm wondering how troublesome would be to just
>>> re-do this and break the ptrace and signal ABI. For instance, just
>>> pushing pt_regs in PTRACE_GETREGSET would make things much clearer.
>>
>> Could you change pt_regs to match the layout you have for PTRACE_GETREGSET
>> instead? It seems much more intuitive.
>
> There is a reason for this pt_regs arrangement: the nios2 syscall
> interface uses r4-r9 for parameters, while the usual C conventions use
> only r4-r7, placing r8-r9 at the start of pt_regs creates a natural
> stack layout for entering C code after the asm shims in entry.S
>
>>> I guess Linus would burn me for even suggesting to breaking users... but
>>> do we have any users at all? This arch has just been mainlined. Altera's
>>> out-of-tree is already ABI-incompatible with mainline so that's not an
>>> issue.
>>>
>>> The only one using this ABI is gdb, which is easily fixed.
>>
>> You can change anything you like as long as nobody complains about
>> regressions.
>
> PTRACE_GET/SETREGSET is a new feature in nios2-linux that we're still
> about to support in upstream GDB, so things could be fixed if needed,
> but why can't you just use the [0...] ordering in userspace?
>
Sure, that's doable. However, I believe the problem is the pt_regs
struct is exported in ptrace.h so we seem to be telling userspace to use it.
> BTW, it's even that way in signal stacks as well; nios2 does not
> use/export sigcontext inside struct ucontext. We just use a int[32]
> array there.
>
I think we are more or less on the same page.
Right now, my biggest problem is how to remove the pt_regs from being
exported to userspace, without breaking things. The struct is defined
and used in a few UAPI headers:
arch/nios2/include/uapi/asm/ptrace.h
arch/nios2/include/uapi/asm/sigcontext.h
arch/nios2/include/uapi/asm/elf.h
For ucontext, we would need to remove
arch/nios2/include/uapi/asm/sigcontext.h and avoid using
asm-generic/ucontext.h.
For ptrace, we could move pt_regs from the UAPI header to some internal
header. That way userspace is not exposed the wrong struct.
For the elf coredump, I have no idea yet :)
--
Ezequiel Garcia, VanguardiaSur
http://www.vanguardiasur.com.ar