Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-11-21 Thread Jason Thorpe
If strip removes it, then you’re doomed anyway and trampoline assist via a 
function won’t help you, because you won’t be able to get to the trampoline or 
past it anyway.

-- thorpej
Sent from my iPhone.

> On Nov 21, 2021, at 7:54 AM, John Marino (NetBSD)  wrote:
> 
> 
> I'm not very familiar with CFA information.  I've been worried that strip(1) 
> removes those symbols.  Is that worry meritless?
> 
>> On Sun, Nov 21, 2021 at 9:32 AM Jason Thorpe  wrote:
>> 
>> > On Nov 21, 2021, at 6:35 AM, John Marino (NetBSD)  wrote:
>> > 
>> > Sorry, I meant to answer and got overwhelmed.
>> > Actually, I had a typo in the previous response. I meant to say "GCC" 
>> > unwinder, not gdb unwinder.
>> > I don't see how this helps support the GCC unwinder.  The context 
>> > information to support the unwind is already provided, we just need the 
>> > boolean (is this a sigtramp frame?) to decide whether or not to use it.  I 
>> > thought Uwe's suggestion to return the context was either an expansion for 
>> > more general use or a second function.  All we have is the pc pointer. 
>> 
>> Well, getting back to a previous part of the conversation, there can be 
>> multiple kinds of contexts, either a “sigcontext” or a “ucontext_t”, and so 
>> telling you “it’s in a trampoline” isn’t necessarily useful — you need to 
>> know what kind it is.  The idea behind __sigtramp_unwind_np() is that you 
>> would need less architecture-dependent logic (and that the API would be 
>> future-proof, in case the way the handlers work changes for some reason).
>> 
>> However, Joerg correctly pointed out that the real correct solution already 
>> exists, which is to say “make sure DWARF CFA information exists for signal 
>> trampolines”, which is what I’ve been focusing on over the last couple of 
>> weeks.  Only if that proves to be insufficient for some reason should we add 
>> a new non-portable API to assist unwind.  DWARF unwind information doesn’t 
>> really work for FreeBSD trampolines, because the handler is supplied by the 
>> kernel, and so of course there’s not really a good way to look up the CIE / 
>> FDE for it.
>> 
>> -- thorpej
>> 


Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-11-21 Thread Jason Thorpe



> On Nov 21, 2021, at 8:28 AM, Jason Thorpe  wrote:
> 
> If strip removes it, then you’re doomed anyway and trampoline assist via a 
> function won’t help you, because you won’t be able to get to the trampoline 
> or past it anyway.

Here’s a before/after of a “strip -s” of a binary on the DWARF unwind 
information:

 12 .eh_frame_hdr 01a4  0001200077f0  0001200077f0  77f0  2**2
  CONTENTS, ALLOC, LOAD, READONLY, DATA
 13 .eh_frame 07a4  000120007998  000120007998  7998  2**3
  CONTENTS, ALLOC, LOAD, READONLY, DATA
 
 
 12 .eh_frame_hdr 01a4  0001200077f0  0001200077f0  77f0  2**2
  CONTENTS, ALLOC, LOAD, READONLY, DATA
 13 .eh_frame 07a4  000120007998  000120007998  7998  2**3
  CONTENTS, ALLOC, LOAD, READONLY, DATA

I.e. strip does not effect the unwind information, because unwind information 
is not debugging information nor is it part of the symbol information; it is, 
in fact, required for correct operation of the program in the face of 
exceptions.  And my test program still works:

alpha-vm:thorpej 22$ ./test1  
^Cx 2
Backtrace 4 stack frames.
0x1200014e4 <_init+0x304> at ./test1
0x3fffdddbfd4 <__sigtramp_siginfo_2> at /usr/lib/libc.so.12
0x1200015e4 <_init+0x404> at ./test1
0x120001590 <_init+0x3b0> at ./test1

As you can see, because I stripped the symbols out of the program binary, the 
symbol names are wrong, but the unwind works and the program counter values are 
the same as an un-stripped copy of the program:

alpha-vm:thorpej 23$ ./sigbttest   
^Cx 2
Backtrace 4 stack frames.
0x1200014e4  at ./sigbttest
0x3fffdddbfd4 <__sigtramp_siginfo_2> at /usr/lib/libc.so.12
0x1200015e4  at ./sigbttest
0x120001590  at ./sigbttest

So, I think your worry about it is unwarranted.

>> On Nov 21, 2021, at 7:54 AM, John Marino (NetBSD)  wrote:
>> 
>> 
>> I'm not very familiar with CFA information.  I've been worried that strip(1) 
>> removes those symbols.  Is that worry meritless?

-- thorpej



Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-11-21 Thread John Marino (NetBSD)
I'm not very familiar with CFA information.  I've been worried that
strip(1) removes those symbols.  Is that worry meritless?

On Sun, Nov 21, 2021 at 9:32 AM Jason Thorpe  wrote:

>
> > On Nov 21, 2021, at 6:35 AM, John Marino (NetBSD) 
> wrote:
> >
> > Sorry, I meant to answer and got overwhelmed.
> > Actually, I had a typo in the previous response. I meant to say "GCC"
> unwinder, not gdb unwinder.
> > I don't see how this helps support the GCC unwinder.  The context
> information to support the unwind is already provided, we just need the
> boolean (is this a sigtramp frame?) to decide whether or not to use it.  I
> thought Uwe's suggestion to return the context was either an expansion for
> more general use or a second function.  All we have is the pc pointer.
>
> Well, getting back to a previous part of the conversation, there can be
> multiple kinds of contexts, either a “sigcontext” or a “ucontext_t”, and so
> telling you “it’s in a trampoline” isn’t necessarily useful — you need to
> know what kind it is.  The idea behind __sigtramp_unwind_np() is that you
> would need less architecture-dependent logic (and that the API would be
> future-proof, in case the way the handlers work changes for some reason).
>
> However, Joerg correctly pointed out that the real correct solution
> already exists, which is to say “make sure DWARF CFA information exists for
> signal trampolines”, which is what I’ve been focusing on over the last
> couple of weeks.  Only if that proves to be insufficient for some reason
> should we add a new non-portable API to assist unwind.  DWARF unwind
> information doesn’t really work for FreeBSD trampolines, because the
> handler is supplied by the kernel, and so of course there’s not really a
> good way to look up the CIE / FDE for it.
>
> -- thorpej
>
>


Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-11-21 Thread Jason Thorpe


> On Nov 21, 2021, at 6:35 AM, John Marino (NetBSD)  wrote:
> 
> Sorry, I meant to answer and got overwhelmed.
> Actually, I had a typo in the previous response. I meant to say "GCC" 
> unwinder, not gdb unwinder.
> I don't see how this helps support the GCC unwinder.  The context information 
> to support the unwind is already provided, we just need the boolean (is this 
> a sigtramp frame?) to decide whether or not to use it.  I thought Uwe's 
> suggestion to return the context was either an expansion for more general use 
> or a second function.  All we have is the pc pointer. 

Well, getting back to a previous part of the conversation, there can be 
multiple kinds of contexts, either a “sigcontext” or a “ucontext_t”, and so 
telling you “it’s in a trampoline” isn’t necessarily useful — you need to know 
what kind it is.  The idea behind __sigtramp_unwind_np() is that you would need 
less architecture-dependent logic (and that the API would be future-proof, in 
case the way the handlers work changes for some reason).

However, Joerg correctly pointed out that the real correct solution already 
exists, which is to say “make sure DWARF CFA information exists for signal 
trampolines”, which is what I’ve been focusing on over the last couple of 
weeks.  Only if that proves to be insufficient for some reason should we add a 
new non-portable API to assist unwind.  DWARF unwind information doesn’t really 
work for FreeBSD trampolines, because the handler is supplied by the kernel, 
and so of course there’s not really a good way to look up the CIE / FDE for it.

-- thorpej



Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-11-21 Thread John Marino (NetBSD)
Sorry, I meant to answer and got overwhelmed.
Actually, I had a typo in the previous response. I meant to say "GCC"
unwinder, not gdb unwinder.
I don't see how this helps support the GCC unwinder.  The context
information to support the unwind is already provided, we just need the
boolean (is this a sigtramp frame?) to decide whether or not to use it.  I
thought Uwe's suggestion to return the context was either an expansion for
more general use or a second function.  All we have is the pc pointer.

In the original post, I provided two implementations (current freebsd,
former netbsd) of the gcc unwinding.  Maybe I'm missing how
__sigtramp_unwind_np is supposed to be used there.  All I was asking for
was to pass pc (context->return_address) and get a boolean answer if it
were inside a signal trampoline.  The rest is already done.
Thanks,
John


On Sun, Nov 14, 2021 at 6:04 PM Jason Thorpe  wrote:

>
>
> > On Nov 14, 2021, at 3:12 PM, John Marino (NetBSD) 
> wrote:
> >
> > So for the purpose of the gdb unwinder, I would pass NULL for the sp
> argument?
> > The unwinder would only be checking the result to see
> __sigtramp_unwind_np returns null or not.
>
> This would not work for the gdb unwinder — it only works for unwinding
> from within the same process, so for exception handling, etc.  I.e. if the
> unwinder in the compiler runtime wasn’t able to process the DWARF call
> frame info for the signal trampoline, for example.  The gdb unwinder may
> have to operate on a process that’s no longer running (and, at the very
> least, is not the current process), so it needs to rely only on DWARF call
> frame info and/or heuristics based on the symbol name (which I added to gdb
> quite some years ago now).
>
> The requirements for using this are:
>
> ==> In the frame that represents the handler itself, you are able to get
> the return address the handler will return to.
>
> ==> In that same frame, you can compute what the stack pointer should be
> when the handler returns (by either inspecting the instructions in the
> function prologue that setup the handler’s stack frame, or by parsing the
> DWARF call frame info).
>
> It’s that return address and stack pointer that you would pass to
> __sigtramp_unwind_np().
>
> Then the context returned by __sigtramp_unwind_np() would allow you to
> then get the stack pointer, PC, etc. for the context that would be resumed
> after the handler returns (because remember, signals can run on their own
> stack).
>
> So, this is a little more than just “is this PC in the signal
> trampoline?”.  This is “If this PC is in the signal trampoline, then return
> a pointer to the context that will be restored when the signal returns,
> given this SP.”  This was uwe’s suggestion.
>
> I can still expose just __sigtramp_check_np(), but the assumption was that
> you would use a “YES!” result from that to to off an find the
> context-to-restore… so we decided to encapsulate some of that work for you
> as well.  Does that make sense?
>
> If that assumption about how you would use __sigtramp_check_np() is
> invalid, please let me know so I can adjust the proposal.
>
> -- thorpej
>
>


Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-11-14 Thread Jason Thorpe



> On Nov 14, 2021, at 3:12 PM, John Marino (NetBSD)  wrote:
> 
> So for the purpose of the gdb unwinder, I would pass NULL for the sp argument?
> The unwinder would only be checking the result to see __sigtramp_unwind_np 
> returns null or not.

This would not work for the gdb unwinder — it only works for unwinding from 
within the same process, so for exception handling, etc.  I.e. if the unwinder 
in the compiler runtime wasn’t able to process the DWARF call frame info for 
the signal trampoline, for example.  The gdb unwinder may have to operate on a 
process that’s no longer running (and, at the very least, is not the current 
process), so it needs to rely only on DWARF call frame info and/or heuristics 
based on the symbol name (which I added to gdb quite some years ago now).

The requirements for using this are:

==> In the frame that represents the handler itself, you are able to get the 
return address the handler will return to.

==> In that same frame, you can compute what the stack pointer should be when 
the handler returns (by either inspecting the instructions in the function 
prologue that setup the handler’s stack frame, or by parsing the DWARF call 
frame info).

It’s that return address and stack pointer that you would pass to 
__sigtramp_unwind_np().

Then the context returned by __sigtramp_unwind_np() would allow you to then get 
the stack pointer, PC, etc. for the context that would be resumed after the 
handler returns (because remember, signals can run on their own stack).

So, this is a little more than just “is this PC in the signal trampoline?”.  
This is “If this PC is in the signal trampoline, then return a pointer to the 
context that will be restored when the signal returns, given this SP.”  This 
was uwe’s suggestion.

I can still expose just __sigtramp_check_np(), but the assumption was that you 
would use a “YES!” result from that to to off an find the context-to-restore… 
so we decided to encapsulate some of that work for you as well.  Does that make 
sense?

If that assumption about how you would use __sigtramp_check_np() is invalid, 
please let me know so I can adjust the proposal.

-- thorpej



Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-11-14 Thread John Marino (NetBSD)
So for the purpose of the gdb unwinder, I would pass NULL for the sp
argument?
The unwinder would only be checking the result to see __sigtramp_unwind_np
returns null or not.
John

On Sun, Nov 14, 2021 at 2:49 PM Jason Thorpe  wrote:

>
> > On Oct 28, 2021, at 8:49 AM, Valery Ushakov  wrote:
> >
> > It is ucontext for the siginfo trampoline and sigcontext for the older
> > one, isn't it?
>
> Ok, I’ve settled on the following:
>
> void *__sigtramp_unwind_np(void *pc, void *sp, int *versp);
>
> Given a program counter and stack pointer, return a pointer to the context
> that will be restored when the signal handler returns.  The signal
> trampoline version is returned in *versp; versp must not be NULL, and
> passing NULL will result in undefined behavior.
>
> Returns NULL if the provided pc is not within the signal trampoline, or if
> the location of the context to restore cannot be determined.
>
> If the returned version is within the range __SIGTRAMP_SIGINFO_VERSION_MIN
> … __SIGTRAMP_SIGINFO_VERSION_MAX, then the returned context points to a
> ucontext_t.
>
> If the returned version is within the range
> __SIGTRAMP_SIGCONTEXT_VERSION_MIN … __SIGTRAMP_SIGCONTEXT_VERSION_MAX, then
> the returned context points to a struct sigcontext.
>
> Note that the layout of the context structures is architecture-dependent
> and may also be version-dependent.
>
> Does this sound good to everyone?
>
> -- thorpej
>
>


Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-11-14 Thread Jason Thorpe


> On Oct 28, 2021, at 8:49 AM, Valery Ushakov  wrote:
> 
> It is ucontext for the siginfo trampoline and sigcontext for the older
> one, isn't it?

Ok, I’ve settled on the following:

void *__sigtramp_unwind_np(void *pc, void *sp, int *versp);

Given a program counter and stack pointer, return a pointer to the context that 
will be restored when the signal handler returns.  The signal trampoline 
version is returned in *versp; versp must not be NULL, and passing NULL will 
result in undefined behavior.

Returns NULL if the provided pc is not within the signal trampoline, or if the 
location of the context to restore cannot be determined.

If the returned version is within the range __SIGTRAMP_SIGINFO_VERSION_MIN … 
__SIGTRAMP_SIGINFO_VERSION_MAX, then the returned context points to a 
ucontext_t.

If the returned version is within the range __SIGTRAMP_SIGCONTEXT_VERSION_MIN … 
__SIGTRAMP_SIGCONTEXT_VERSION_MAX, then the returned context points to a struct 
sigcontext.

Note that the layout of the context structures is architecture-dependent and 
may also be version-dependent.

Does this sound good to everyone? 

-- thorpej



Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-29 Thread Jason Thorpe


> On Oct 29, 2021, at 7:54 AM, Jason Thorpe  wrote:
> 
> 
> 
>> On Oct 29, 2021, at 2:45 AM, Martin Husemann  wrote:
>> 
>> On Thu, Oct 28, 2021 at 11:14:39AM -0700, Jason Thorpe wrote:
>>> We really just need to kill the sigcontext stuff completely.  More to the 
>>> point, we should version sigaction() before (or concurrently with) adding 
>>> whatever call we add here so as to ensure that it will only ever be a 
>>> ucontext.
>> 
>> Something there is broken in -current, see PR 56471 (freshly compiled macppc
>> ntpd tries to use compat_16___sigreturn14).
> 
> Yes, and from your description it should have failed to register a 
> “sigcontext” handler in the first place.

Martin, is kern.module.autoload enabled on your machine?  (Sorry, my Mac mini 
is in a state of “has been turned off for a while”, so I need to resurrect it 
today, so asking for info here to get the investigation going…)

The PowerPC signal trampoline also has a case where it can return EINVAL 
because it thinks the saved SRR1 has been tampered with.  I’m wondering if 
somehow the old sigcontext trampoline is getting invoked for a siginfo-style 
handler.

-- thorpej



Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-29 Thread Jason Thorpe



> On Oct 29, 2021, at 2:45 AM, Martin Husemann  wrote:
> 
> On Thu, Oct 28, 2021 at 11:14:39AM -0700, Jason Thorpe wrote:
>> We really just need to kill the sigcontext stuff completely.  More to the 
>> point, we should version sigaction() before (or concurrently with) adding 
>> whatever call we add here so as to ensure that it will only ever be a 
>> ucontext.
> 
> Something there is broken in -current, see PR 56471 (freshly compiled macppc
> ntpd tries to use compat_16___sigreturn14).

Yes, and from your description it should have failed to register a “sigcontext” 
handler in the first place.

I’m looking into it.

-- thorpej



Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-29 Thread Martin Husemann
On Thu, Oct 28, 2021 at 11:14:39AM -0700, Jason Thorpe wrote:
> We really just need to kill the sigcontext stuff completely.  More to the 
> point, we should version sigaction() before (or concurrently with) adding 
> whatever call we add here so as to ensure that it will only ever be a 
> ucontext.

Something there is broken in -current, see PR 56471 (freshly compiled macppc
ntpd tries to use compat_16___sigreturn14).

Martin


Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-28 Thread Jason Thorpe



> On Oct 28, 2021, at 8:49 AM, Valery Ushakov  wrote:
> 
> On Wed, Oct 27, 2021 at 20:59:12 -0700, Jason Thorpe wrote:
> 
>>> On Oct 27, 2021, at 4:01 PM, Jason Thorpe  wrote:
>>> 
>>> 
 On Oct 27, 2021, at 3:44 PM, Valery Ushakov  wrote:
 
 On Wed, Oct 27, 2021 at 07:50:55 -0700, Jason Thorpe wrote:
 
 I was wondering if it might be easier to not put the onus onto the
 caller and instead have a function that returns the interrupted
 ucontext (or NULL, if the pc is not in a trampoline).
 
 ucontext_t *__unwind_sigtramp(return_pc, return_sp)
>>> 
>>> That would certainly be a nicer API.
>> 
>> Thought about it a little more.
>> 
>> To make this really work, we'd definitely have to version
>> sigaction() so that it fully de-supported sigcontext handlers.
>> Otherwise, it's a toss-up whether you have a sigcontext or a
>> ucontext on the stack.
> 
> It is ucontext for the siginfo trampoline and sigcontext for the older
> one, isn't it?

Sure, unless you're a platform that has never supported sigcontext, if you have 
COMPAT_16 disabled, or, ...

We really just need to kill the sigcontext stuff completely.  More to the 
point, we should version sigaction() before (or concurrently with) adding 
whatever call we add here so as to ensure that it will only ever be a ucontext.

-- thorpej



Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-28 Thread Valery Ushakov
On Wed, Oct 27, 2021 at 20:59:12 -0700, Jason Thorpe wrote:

> > On Oct 27, 2021, at 4:01 PM, Jason Thorpe  wrote:
> > 
> > 
> >> On Oct 27, 2021, at 3:44 PM, Valery Ushakov  wrote:
> >> 
> >> On Wed, Oct 27, 2021 at 07:50:55 -0700, Jason Thorpe wrote:
> >> 
> >> I was wondering if it might be easier to not put the onus onto the
> >> caller and instead have a function that returns the interrupted
> >> ucontext (or NULL, if the pc is not in a trampoline).
> >> 
> >> ucontext_t *__unwind_sigtramp(return_pc, return_sp)
> > 
> > That would certainly be a nicer API.
> 
> Thought about it a little more.
> 
> To make this really work, we'd definitely have to version
> sigaction() so that it fully de-supported sigcontext handlers.
> Otherwise, it's a toss-up whether you have a sigcontext or a
> ucontext on the stack.

It is ucontext for the siginfo trampoline and sigcontext for the older
one, isn't it?


> I also see some value in the basic check (which you need to have to
> __sigtramp_unwind() anyway?).

You need internally, obviously, but the only useful thing you can do
with it is get the interrupted context that the trampoline would
restore.  Or do I miss something here (which is entirely possible, as
I'm writing all these remarks from fading memories of implementing the
trampolines for sh3).

-uwe


Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-27 Thread Jason Thorpe


> On Oct 27, 2021, at 4:01 PM, Jason Thorpe  wrote:
> 
> 
>> On Oct 27, 2021, at 3:44 PM, Valery Ushakov  wrote:
>> 
>> On Wed, Oct 27, 2021 at 07:50:55 -0700, Jason Thorpe wrote:
>> 
>> I was wondering if it might be easier to not put the onus onto the
>> caller and instead have a function that returns the interrupted
>> ucontext (or NULL, if the pc is not in a trampoline).
>> 
>> ucontext_t *__unwind_sigtramp(return_pc, return_sp)
> 
> That would certainly be a nicer API.

Thought about it a little more.

To make this really work, we’d definitely have to version sigaction() so that 
it fully de-supported sigcontext handlers.  Otherwise, it’s a toss-up whether 
you have a sigcontext or a ucontext on the stack.  I do think a 
__sigtramp_unwind() would also be a good addition.  But I also see some value 
in the basic check (which you need to have to __sigtramp_unwind() anyway…).

-- thorpej



Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-27 Thread Jason Thorpe


> On Oct 27, 2021, at 3:44 PM, Valery Ushakov  wrote:
> 
> On Wed, Oct 27, 2021 at 07:50:55 -0700, Jason Thorpe wrote:
> 
> I was wondering if it might be easier to not put the onus onto the
> caller and instead have a function that returns the interrupted
> ucontext (or NULL, if the pc is not in a trampoline).
> 
> ucontext_t *__unwind_sigtramp(return_pc, return_sp)

That would certainly be a nicer API.

-- thorpej



Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-27 Thread Valery Ushakov
On Wed, Oct 27, 2021 at 07:50:55 -0700, Jason Thorpe wrote:

> > On Oct 18, 2021, at 9:41 AM, John Marino (NetBSD)  wrote:
> > 
> > yes, it sounds like a __in_signal_trampoline function would work for
> > the GCC unwind, and I would think it would work for GDB as well.
> 
> Ok, I have implemented a new function with this signature:
> 
> /*
>  * __sigtramp_check_np --
>  *
>  *  Non-portable function that checks if the specified program
>  *  counter value is within the signal return trampoline.  Returns
>  *  the trampoline version numnber corresponding to what style of
>  *  trampoline it matches, or -1 if the program value is not within
>  *  the signal return trampoline.
>  */
> int __sigtramp_check_np(void *pc);
> 
> Usage would be like:
[... lots of code ...]

I was wondering if it might be easier to not put the onus onto the
caller and instead have a function that returns the interrupted
ucontext (or NULL, if the pc is not in a trampoline).

ucontext_t *__unwind_sigtramp(return_pc, return_sp)


-uwe


Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-27 Thread John Marino (NetBSD)
Hi Jason,
For the purpose of supporting GCC stack unwinding, the __sigtramp_check_np
function would work as a solution.
Backporting to NetBSD-9 branch would definitely be nice as well.
Thanks,
John

On Wed, Oct 27, 2021 at 9:51 AM Jason Thorpe  wrote:

>
> > On Oct 18, 2021, at 9:41 AM, John Marino (NetBSD) 
> wrote:
> >
> > yes, it sounds like a __in_signal_trampoline function would work for
> > the GCC unwind, and I would think it would work for GDB as well.
>
> Ok, I have implemented a new function with this signature:
>
> /*
>  * __sigtramp_check_np --
>  *
>  *  Non-portable function that checks if the specified program
>  *  counter value is within the signal return trampoline.  Returns
>  *  the trampoline version numnber corresponding to what style of
>  *  trampoline it matches, or -1 if the program value is not within
>  *  the signal return trampoline.
>  */
> int __sigtramp_check_np(void *pc);
>
> Usage would be like:
>
> /*
>  * If you need access to “struct sigcontext” to perform the unwind, you
> must
>  * define _LIBC before including .
>  */
> #define _LIBC
> #include 
>
> .
> .
> .
>
> int rv = __sigtramp_check_np((void *)context->return_address);
>
> if (rv == -1) {
> /* address was not within a signal return trampoline. */
> }
>
> if (rv == __SIGTRAMP_SIGCODE_VERSION) {
> /*
>  * ultra-legacy — this will never be returned for modern
> binaries.
>  *  in fact, the current implementation of
> __sigtramp_check_np()
>  * will never return it and there is no reason to add
> support for it.
>  */
> }
>
> if (rv >= __SIGTRAMP_SIGCONTEXT_VERSION_MIN &&
> rv <= __SIGTRAMP_SIGCONTEXT_VERSION_MAX) {
> #ifdef __HAVE_STRUCT_SIGCONTEXT
> /* do sigcontext stuff, distinguishing between versions,
> as needed */
> #else
> /*
>  * no sigcontext structure is available here, so none of
> these values
>  * should have been returned.
>  */
> #endif
> }
>
> if (rv >= __SIGTRAMP_SIGINFO_VERSION_MIN &&
> rv <= __SIGTRAMP_SIGINFO_VERSION_MAX) {
> /* do siginfo stuff, distinguishing between versions, as
> needed */
> }
>
> /*
>  * Address was within a signal return trampoline, but it’s not a
> version
>  * that this program knows how to decode so *shrug*.
>  */
>
> For the most part, the MIN and MAX values will be the same (currently only
> the VAX has multiple versions of either kind of signal trampoline, and it’s
> for the “sigcontext” type).
>
> Again, to be clear, this can query the current process ONLY.
>
> Is this suitable for your needs?  Looking at your GCC unwinder for FreeBSD
> (link in your original email), it seems it would work fine (that unwinder
> uses getpid() to pass to the sysctl).
>
> You would be able to test for the availability of __sigtramp_check_np() by
> testing for __SIGTRAMP_SIGCODE_VERSION being defined:
>
> #ifdef __SIGTRAMP_SIGCODE_VERSION
> /*
>  * Code to check if we’re in a NetBSD signal trampoline.
>  */
> ...
> #endif /* __SIGTRAMP_SIGCODE_VERSION */
>
> It’s not the prettiest API in the world, but you’re by definition diving
> into implementation details here, and the API does provide all the info
> needed to do the work.
>
> Any other comments?  Christos?
>
> …and while there was a flurry of activity lately around this area, the
> minimal changes to support __sigtramp_check_np() are easily isolated and
> could be back ported to the netbsd-9 branch without much trouble.
>
> -- thorpej
>
>


Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-27 Thread Jason Thorpe


> On Oct 18, 2021, at 9:41 AM, John Marino (NetBSD)  wrote:
> 
> yes, it sounds like a __in_signal_trampoline function would work for
> the GCC unwind, and I would think it would work for GDB as well.

Ok, I have implemented a new function with this signature:

/*
 * __sigtramp_check_np --
 *
 *  Non-portable function that checks if the specified program
 *  counter value is within the signal return trampoline.  Returns
 *  the trampoline version numnber corresponding to what style of
 *  trampoline it matches, or -1 if the program value is not within
 *  the signal return trampoline.
 */
int __sigtramp_check_np(void *pc);

Usage would be like:

/*
 * If you need access to “struct sigcontext” to perform the unwind, you must
 * define _LIBC before including .
 */
#define _LIBC
#include 

.
.
.

int rv = __sigtramp_check_np((void *)context->return_address);

if (rv == -1) {
/* address was not within a signal return trampoline. */
}

if (rv == __SIGTRAMP_SIGCODE_VERSION) {
/*
 * ultra-legacy — this will never be returned for modern 
binaries.
 *  in fact, the current implementation of __sigtramp_check_np()
 * will never return it and there is no reason to add support 
for it.
 */
}

if (rv >= __SIGTRAMP_SIGCONTEXT_VERSION_MIN &&
rv <= __SIGTRAMP_SIGCONTEXT_VERSION_MAX) {
#ifdef __HAVE_STRUCT_SIGCONTEXT
/* do sigcontext stuff, distinguishing between versions, as 
needed */
#else
/*
 * no sigcontext structure is available here, so none of these 
values
 * should have been returned.
 */
#endif
}

if (rv >= __SIGTRAMP_SIGINFO_VERSION_MIN &&
rv <= __SIGTRAMP_SIGINFO_VERSION_MAX) {
/* do siginfo stuff, distinguishing between versions, as needed 
*/
}

/*
 * Address was within a signal return trampoline, but it’s not a version
 * that this program knows how to decode so *shrug*.
 */

For the most part, the MIN and MAX values will be the same (currently only the 
VAX has multiple versions of either kind of signal trampoline, and it’s for the 
“sigcontext” type).

Again, to be clear, this can query the current process ONLY.

Is this suitable for your needs?  Looking at your GCC unwinder for FreeBSD 
(link in your original email), it seems it would work fine (that unwinder uses 
getpid() to pass to the sysctl).

You would be able to test for the availability of __sigtramp_check_np() by 
testing for __SIGTRAMP_SIGCODE_VERSION being defined:

#ifdef __SIGTRAMP_SIGCODE_VERSION
/*
 * Code to check if we’re in a NetBSD signal trampoline.
 */
...
#endif /* __SIGTRAMP_SIGCODE_VERSION */

It’s not the prettiest API in the world, but you’re by definition diving into 
implementation details here, and the API does provide all the info needed to do 
the work.

Any other comments?  Christos?

…and while there was a flurry of activity lately around this area, the minimal 
changes to support __sigtramp_check_np() are easily isolated and could be back 
ported to the netbsd-9 branch without much trouble.

-- thorpej



Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-18 Thread John Marino (NetBSD)
uwe,
I don't know much about CFI attributes.  I had just assumed that
strip(1) would remove those symbols.  Was that a bad assumption?  I'm
still not sure if GCC unwind can use them but if they are permanent
and I took a look at gccgo, maybe it could work.
John

On Mon, Oct 18, 2021 at 12:41 PM Valery Ushakov  wrote:
>
> On Mon, Oct 18, 2021 at 10:41:48 -0500, John Marino (NetBSD) wrote:
>
> > How we did it with libc before is shown in the netbsd-unwind.h link in
> > the original post.  This technique looks for __sigtramp_siginfo_2
> > assembly code but no longer works.  I don't know how to do this any
> > other way.  GDB doesn't either, it uses the debug information to match
> > the function name __sigtramp_siginfo_2 and I am not even sure that's
> > valid for current NetBSD releases based on what we've learned here.
>
> Didn't kamil@ fixed this a while back?  E.g. for amd64:
>
> revision 1.8
> date: 2020-10-12 20:55:54 +0300;  author: kamil;  state: Exp;  lines: +29 -3; 
>  commitid: sz57gQtWi3mGKDrC;
> Decorate the x86_64 signal trampoline with CFI attributes easing unwinding
>
> Combine the approach provided by Nikhil Benesch and Andrew Cagney.
>
> Now, the unwinders (in gccgo, backtrace(3), etc) can unwind properly
> the stack from a signal handler.
>
> Fixes lib/55719 by Nikhil Benesch
>
> -uwe


Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-18 Thread John Marino (NetBSD)
I'm in no way a GDB expert.  I believe it's currently using basic
dwarf debugging information, basically just looking for the
__sigtramp_siginfo_2 function name.  If it finds that, it declares the
sigtramp found.  I do not believe it's using the cfi notations which
are relatively recent if I understand correctly.  What this discussion
has gotten me wondering regarding gdb is if looking for
__sigtramp_siginfo_2 is still a valid method of detection on the newer
NetBSD releases.

The only reason I brought gdb up is because it's the other major user
of KERN_PROC_SIGTRAMP sysctl so I thought NetBSD might benefit there
too.
John


Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-18 Thread Valery Ushakov
On Mon, Oct 18, 2021 at 10:41:48 -0500, John Marino (NetBSD) wrote:

> How we did it with libc before is shown in the netbsd-unwind.h link in
> the original post.  This technique looks for __sigtramp_siginfo_2
> assembly code but no longer works.  I don't know how to do this any
> other way.  GDB doesn't either, it uses the debug information to match
> the function name __sigtramp_siginfo_2 and I am not even sure that's
> valid for current NetBSD releases based on what we've learned here.

Didn't kamil@ fixed this a while back?  E.g. for amd64:

revision 1.8
date: 2020-10-12 20:55:54 +0300;  author: kamil;  state: Exp;  lines: +29 -3;  
commitid: sz57gQtWi3mGKDrC;
Decorate the x86_64 signal trampoline with CFI attributes easing unwinding

Combine the approach provided by Nikhil Benesch and Andrew Cagney.

Now, the unwinders (in gccgo, backtrace(3), etc) can unwind properly
the stack from a signal handler.

Fixes lib/55719 by Nikhil Benesch

-uwe


Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-18 Thread Jason Thorpe



> On Oct 18, 2021, at 9:41 AM, John Marino (NetBSD)  wrote:
> 
> yes, it sounds like a __in_signal_trampoline function would work for
> the GCC unwind, and I would think it would work for GDB as well.

Maybe I missed the GDB use case ... what is it that GDB needs to be able to do 
here?  This hypothetical function would only be able to query "self", not 
another process, so if GDB needs to be able to do the check for another process 
(the debug target, presumably), then it will need to use something else (can't 
it use the CFI notations to do the unwind?)

(Not all of the trampolines are correctly notated, but that's not a 
particularly tough problem to solve, either...)

-- thorpej



Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-18 Thread John Marino (NetBSD)
yes, it sounds like a __in_signal_trampoline function would work for
the GCC unwind, and I would think it would work for GDB as well.
Thanks,
John

On Mon, Oct 18, 2021 at 10:48 AM Jason Thorpe  wrote:
>
>
>
> > On Oct 18, 2021, at 8:41 AM, John Marino (NetBSD)  wrote:
> >
> > For GCC, we've got the return address (context->ra in the unwind
> > programs in the original post).   The "problem" is that we want to
> > know if that address falls on a signal trampoline frame.  If NO,
> > return "end of stack", otherwise unwind the frame.
>
> Seems like a non-portable libc function that could check among the various 
> candidates that libc knows about would not be terribly difficult.
>
> > A sysctl that returns an array of address pairs for all signal
> > trampolines in the process is what I'm requesting.
>
> Would a function like (hypothetically-named) "bool 
> __in_signal_trampoline(uintptr_t addr);" provided by libc be workable for 
> your use case?
>
> > If there's another way to determine if an address falls within a
> > signal trampoline, I'd like to see actual code to see if I can adapt
> > it.
> > Of course, the kernel team could just deny the request, but I won't be
> > able to fix the regression caused when the per-signal trampolines were
> > introduced.
> > Thanks,
> > John
>
> -- thorpej
>


Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-18 Thread Jason Thorpe



> On Oct 18, 2021, at 8:41 AM, John Marino (NetBSD)  wrote:
> 
> For GCC, we've got the return address (context->ra in the unwind
> programs in the original post).   The "problem" is that we want to
> know if that address falls on a signal trampoline frame.  If NO,
> return "end of stack", otherwise unwind the frame.

Seems like a non-portable libc function that could check among the various 
candidates that libc knows about would not be terribly difficult.

> A sysctl that returns an array of address pairs for all signal
> trampolines in the process is what I'm requesting.

Would a function like (hypothetically-named) "bool 
__in_signal_trampoline(uintptr_t addr);" provided by libc be workable for your 
use case?

> If there's another way to determine if an address falls within a
> signal trampoline, I'd like to see actual code to see if I can adapt
> it.
> Of course, the kernel team could just deny the request, but I won't be
> able to fix the regression caused when the per-signal trampolines were
> introduced.
> Thanks,
> John

-- thorpej



Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-18 Thread John Marino (NetBSD)
For GCC, we've got the return address (context->ra in the unwind
programs in the original post).   The "problem" is that we want to
know if that address falls on a signal trampoline frame.  If NO,
return "end of stack", otherwise unwind the frame.

The KERN_PROC_SIGTRAMP of FreeBSD and DragonFly has in its return
structure a pair of addresses representing the starting address and
ending address of that process's signal frame, so it's a simple matter
of checking if context->fa falls between them.

How we did it with libc before is shown in the netbsd-unwind.h link in
the original post.  This technique looks for __sigtramp_siginfo_2
assembly code but no longer works.  I don't know how to do this any
other way.  GDB doesn't either, it uses the debug information to match
the function name __sigtramp_siginfo_2 and I am not even sure that's
valid for current NetBSD releases based on what we've learned here.

A sysctl that returns an array of address pairs for all signal
trampolines in the process is what I'm requesting.
If there's another way to determine if an address falls within a
signal trampoline, I'd like to see actual code to see if I can adapt
it.
Of course, the kernel team could just deny the request, but I won't be
able to fix the regression caused when the per-signal trampolines were
introduced.
Thanks,
John


Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-17 Thread Andrew Cagney
On Sat, 16 Oct 2021 at 09:12, John Marino (NetBSD)  wrote:
>
> Alright, so I think we have established that it's impossible to
> provide KERN_PROC_SIGTRAMP as it functions on FreeBSD and DragonFly.
> The purpose of that sysctl is to get the address range of the single
> signal trampoline.

Backing up a little.  So the problem is that there's an instruction
pointer, a stack pointer, and a random set of other registers, and
we're trying to figure out the caller and its registers (aka unwind).
The way to do that is to find the unwind information.

It sounds like, for NetBSD, and a signal frame that the information
can be found in libc?

> Would it be possible to have a slightly different version on NetBSD
> that accomplishes the same thing?
> For example, call it KERN_PROC_SIGTRAMPS.  The first time it's called
> with a null oldp argument to get the result size, and the second time
> it's called with a properly sized buffer that is returned filled with
> address ranges like an array, one range for each defined signal tramp.
>
> It would be up to the program to iterate through the ranges to
> determine if a stack pointer is within a signal tramp range.
> And the gcc unwinder could be designed to only have to call it those 2
> times since the result is static per process.
>
> John


Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-16 Thread Martin Husemann
On Sat, Oct 16, 2021 at 08:11:47AM -0500, John Marino (NetBSD) wrote:
> Alright, so I think we have established that it's impossible to
> provide KERN_PROC_SIGTRAMP as it functions on FreeBSD and DragonFly.
> The purpose of that sysctl is to get the address range of the single
> signal trampoline.

Is that compiler using libc at all?

You could make the address ranges available via library functions (or maybe
they already have usable symbol names, I didn't check).

Martin


Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-16 Thread John Marino (NetBSD)
Alright, so I think we have established that it's impossible to
provide KERN_PROC_SIGTRAMP as it functions on FreeBSD and DragonFly.
The purpose of that sysctl is to get the address range of the single
signal trampoline.
Would it be possible to have a slightly different version on NetBSD
that accomplishes the same thing?
For example, call it KERN_PROC_SIGTRAMPS.  The first time it's called
with a null oldp argument to get the result size, and the second time
it's called with a properly sized buffer that is returned filled with
address ranges like an array, one range for each defined signal tramp.

It would be up to the program to iterate through the ranges to
determine if a stack pointer is within a signal tramp range.
And the gcc unwinder could be designed to only have to call it those 2
times since the result is static per process.

John


Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-15 Thread Jason Thorpe


> On Oct 15, 2021, at 1:19 PM, Valery Ushakov  wrote:
> 
> On Fri, Oct 15, 2021 at 23:14:39 +0300, Valery Ushakov wrote:
> 
>> On Fri, Oct 15, 2021 at 14:44:16 -0500, John Marino (NetBSD) wrote:
>> 
>>> Is it possible for NetBSD to implement KERN_PROC_SIGTRAMP sysctl?
>> 
>> It's been ages since I touched this area, but don't we have
>> per-sigaction trampolines?  I mean, in practice they all use the same
>> __sigtramp_siginfo_$version trampoline, that sigaction passes to the
>> actual syscall, but in principle the process can have different
>> trampolines for different signals, can't it?
>> 
>> struct sys___sigaction_sigtramp_args {
>>  syscallarg(int) signum;
>>  syscallarg(const struct sigaction *) nsa;
>>  syscallarg(struct sigaction *) osa;
>>  syscallarg(const void *) tramp; // <-
>>  syscallarg(int) vers;
>> };
> 
> PS: We used to have a trampoline that the kernel copied out into the
> process address space (bottom of the stack, iirc) - and that would be
> something for KERN_PROC_SIGTRAMP to return indeed.  But that was like
> before netbsd 2.0, iirc.

Right, that was The Very Old Way.  There is an API contract between libc and 
the kernel vis a vis the signal trampoline, but the trampoline itself is in 
libc, which could get mapped anywhere.  So there is not a single static address 
that could be returned, even per-process, because, as uwe points out, the 
trampoline can be specified on a per-signal basis (there is one for "siginfo" 
signal delivery and another for the old-style "sigcontext" signal delivery).

-- thorpej



Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-15 Thread Valery Ushakov
On Fri, Oct 15, 2021 at 23:14:39 +0300, Valery Ushakov wrote:

> On Fri, Oct 15, 2021 at 14:44:16 -0500, John Marino (NetBSD) wrote:
> 
> > Is it possible for NetBSD to implement KERN_PROC_SIGTRAMP sysctl?
> 
> It's been ages since I touched this area, but don't we have
> per-sigaction trampolines?  I mean, in practice they all use the same
> __sigtramp_siginfo_$version trampoline, that sigaction passes to the
> actual syscall, but in principle the process can have different
> trampolines for different signals, can't it?
> 
> struct sys___sigaction_sigtramp_args {
>   syscallarg(int) signum;
>   syscallarg(const struct sigaction *) nsa;
>   syscallarg(struct sigaction *) osa;
>   syscallarg(const void *) tramp; // <-
>   syscallarg(int) vers;
> };

PS: We used to have a trampoline that the kernel copied out into the
process address space (bottom of the stack, iirc) - and that would be
something for KERN_PROC_SIGTRAMP to return indeed.  But that was like
before netbsd 2.0, iirc.

-uwe


Re: Request for implementation of KERN_PROC_SIGTRAMP sysctl

2021-10-15 Thread Valery Ushakov
On Fri, Oct 15, 2021 at 14:44:16 -0500, John Marino (NetBSD) wrote:

> Is it possible for NetBSD to implement KERN_PROC_SIGTRAMP sysctl?
> 
> TLDR;
> For several years, the GNAT Ada compiler has not been able to unwind a
> stack containing a signal trampoline.  The unwinder I wrote for gcc
> several years ago just stopped working on newer NetBSD release even
> though the signal trampoline code itself did not change.  FreeBSD and
> DragonFly BSD are immune to sigtramp location changes because they've
> introduced the KERN_PROC_SIGTRAMP sysctl which provides the location
> of the signal tramp of the process.

It's been ages since I touched this area, but don't we have
per-sigaction trampolines?  I mean, in practice they all use the same
__sigtramp_siginfo_$version trampoline, that sigaction passes to the
actual syscall, but in principle the process can have different
trampolines for different signals, can't it?

struct sys___sigaction_sigtramp_args {
syscallarg(int) signum;
syscallarg(const struct sigaction *) nsa;
syscallarg(struct sigaction *) osa;
syscallarg(const void *) tramp; // <-
syscallarg(int) vers;
};


-uwe