Re: Segfault in _Unwind_* code called from pthread_exit

[Gerald Pfeifer]

Hi everyone,

just a heads-up that...

On Sun, 5 Nov 2017, Gerald Pfeifer wrote:
> I should have gcc8-devel updated in the next 24 hours, gcc7-devel 
> and gcc6-devel over the week as new snapshots are released.
:
> Once the respective -devel ports are updated, I'll take care of 
> the corresponding stable ports.

...lang/gcc6, lang/gcc6-devel (via upstream), lang/gcc7, and
lang/gcc7-devel and lang/gcc8-devel (both via upstream) should
have addressed this issue now.

Thanks to Andreas for getting the fix into upstream GCC and
backporting into active release branches!

Gerald
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Gerald Pfeifer]

On Sun, 5 Nov 2017, Andreas Tobler wrote:
> Pushed on all active branches, 8/7/6.

Saw that, thank you.  Very well done, Andreas!

I should have gcc8-devel updated in the next 24 hours, gcc7-devel 
and gcc6-devel over the week as new snapshots are released.

> If you could do the gcc* branches, yes please.

Once the respective -devel ports are updated, I'll take care of 
the corresponding stable ports.

> I got one request to have the patch also in gcc5

Let's see.  First I want to address the five newer ports, then
possibly this one (though gcc5 is hardly used).

Gerald
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Andreas Tobler]

On 05.11.17 21:31, Gerald Pfeifer wrote:

On Sun, 5 Nov 2017, Andreas Tobler wrote:

Pushed on all active branches, 8/7/6.


Saw that, thank you.  Very well done, Andreas!

I should have gcc8-devel updated in the next 24 hours, gcc7-devel
and gcc6-devel over the week as new snapshots are released.


Cool thx!


If you could do the gcc* branches, yes please.


Once the respective -devel ports are updated, I'll take care of
the corresponding stable ports.


Patch available if you want. (applicable to all gcc* ports, 5-8)


I got one request to have the patch also in gcc5


Let's see.  First I want to address the five newer ports, then
possibly this one (though gcc5 is hardly used).


Sure, but I can understand comment #13 of the pr mentioned. Even if 
hardly used, consistency is always good :)


Andreas
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Andreas Tobler]

On 01.11.17 14:21, Gerald Pfeifer wrote:

On Wed, 1 Nov 2017, Tijl Coosemans wrote:

Please commit it to the ports tree as well, because there are reports
that ftp/curl can trigger the problem.


What Andreas and me usually are doing is that he commits fixes
upstream (from HEAD down to release branches), I pick them up when
updating the gcc*-devel ports, and if important he temporarily adds
patches to the gcc* ports until the next release obsoletes them.

Andreas, I can take of the gcc* ports this time once you have
pushed things upstream.


Pushed on all active branches, 8/7/6.

If you could do the gcc* branches, yes please. I got one request to have 
the patch also in gcc5, see in the original 
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82635).
It's up on you if we will support this. I don't mind running a test on 
gcc-5.5. Let me know.


Gruss,
Andreas
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Gerald Pfeifer]

On Wed, 1 Nov 2017, Tijl Coosemans wrote:
> Please commit it to the ports tree as well, because there are reports
> that ftp/curl can trigger the problem.

What Andreas and me usually are doing is that he commits fixes 
upstream (from HEAD down to release branches), I pick them up when 
updating the gcc*-devel ports, and if important he temporarily adds
patches to the gcc* ports until the next release obsoletes them.

Andreas, I can take of the gcc* ports this time once you have 
pushed things upstream.

Gerald
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Tijl Coosemans]

On Tue, 31 Oct 2017 22:52:33 +0100 Andreas Tobler  
wrote:
> On 31.10.17 22:36, Gerald Pfeifer wrote:
>> On Tue, 31 Oct 2017, Andreas Tobler wrote:
>> Those possibly still stuck on obsolete versions of FreeBSD don't
>> need/want fancy new compilers and GCC 4.9 is still available for
>> use and does not exhibit this issue, correct?  (If it does, nobody
>> reported any problems.)
> 
> It is limited to gcc >=5, gcc-4.9 does not have the 
> MD_FALLBACK_FRAME_STATE_FOR defined.
> 
>>> I can 'ifdef' the new code and in the 'else' case we fall back to
>>> the already existing path.
>> 
>> If it's "cheap", that might be nice.
> 
> Attached, the test is running on gcc trunk and gcc-6. gcc-6 is the last 
> one with java support and there we have quite extensive test cases which 
> really test for this MD_FALLBACK_FRAME_STATE_FOR macro. These test 
> cases, Throw_2 and co do pass. So I think the new bits should be fine.
> Also some coming asan test cases do pass with this addition too.

Please commit it to the ports tree as well, because there are reports
that ftp/curl can trigger the problem.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Gerald Pfeifer]

On Tue, 31 Oct 2017, Andreas Tobler wrote:
> Do we, FreeBSD'ers, want to have gcc unwind support on older than 
> FreeBSD 9.3 releases? I think the gcc folks do not care, but we are the 
> ones who might have an need for such a support?
> @Gerald, do you have an opinion?

Yes. No. :-)

Those possibly still stuck on obsolete versions of FreeBSD don't
need/want fancy new compilers and GCC 4.9 is still available for
use and does not exhibit this issue, correct?  (If it does, nobody
reported any problems.)

> I can 'ifdef' the new code and in the 'else' case we fall back to 
> the already existing path.

If it's "cheap", that might be nice.

Thanks to the three of you - Tijl, Konstantin, and Andreas!

Gerald
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Andreas Tobler]

On 31.10.17 22:36, Gerald Pfeifer wrote:

On Tue, 31 Oct 2017, Andreas Tobler wrote:

Do we, FreeBSD'ers, want to have gcc unwind support on older than
FreeBSD 9.3 releases? I think the gcc folks do not care, but we are the
ones who might have an need for such a support?
@Gerald, do you have an opinion?


Yes. No. :-)

Those possibly still stuck on obsolete versions of FreeBSD don't
need/want fancy new compilers and GCC 4.9 is still available for
use and does not exhibit this issue, correct?  (If it does, nobody
reported any problems.)


It is limited to gcc >=5, gcc-4.9 does not have the 
MD_FALLBACK_FRAME_STATE_FOR defined.



I can 'ifdef' the new code and in the 'else' case we fall back to
the already existing path.


If it's "cheap", that might be nice.


Attached, the test is running on gcc trunk and gcc-6. gcc-6 is the last 
one with java support and there we have quite extensive test cases which 
really test for this MD_FALLBACK_FRAME_STATE_FOR macro. These test 
cases, Throw_2 and co do pass. So I think the new bits should be fine.

Also some coming asan test cases do pass with this addition too.


Thanks to the three of you - Tijl, Konstantin, and Andreas!


Gruss,
Andreas

Index: libgcc/config/i386/freebsd-unwind.h
===
--- libgcc/config/i386/freebsd-unwind.h (revision 254205)
+++ libgcc/config/i386/freebsd-unwind.h (working copy)
@@ -28,7 +28,10 @@
 
 #include 
 #include 
+#include 
+#include 
 #include 
+#include 
 #include 
 
 #define REG_NAME(reg)  sf_uc.uc_mcontext.mc_## reg
@@ -36,6 +39,39 @@
 #ifdef __x86_64__
 #define MD_FALLBACK_FRAME_STATE_FOR x86_64_freebsd_fallback_frame_state
 
+#ifdef KERN_PROC_SIGTRAMP
+ /* Newer versions of FreeBSD, > FreeBSD 9.3, provide a
+kern.proc.sigtramp. sysctl that returns the location of the
+signal trampoline. We use this information to find out if we're in
+a trampoline or not.
+ */
+static int
+x86_64_outside_sigtramp_range (unsigned char *pc)
+{
+  static int sigtramp_range_determined = 0;
+  static unsigned char *sigtramp_start, *sigtramp_end;
+
+  if (sigtramp_range_determined == 0)
+{
+  struct kinfo_sigtramp kst = {0};
+  size_t len = sizeof (kst);
+  int mib[4] = { CTL_KERN, KERN_PROC, KERN_PROC_SIGTRAMP, getpid() };
+
+  sigtramp_range_determined = 1;
+  if (sysctl (mib, 4, , , NULL, 0) == 0)
+  {
+sigtramp_range_determined = 2;
+sigtramp_start = kst.ksigtramp_start;
+sigtramp_end   = kst.ksigtramp_end;
+  }
+}
+  if (sigtramp_range_determined < 2)  /* sysctl failed if < 2 */
+return 1;
+
+  return (pc < sigtramp_start || pc >= sigtramp_end);
+}
+#endif
+
 static _Unwind_Reason_Code
 x86_64_freebsd_fallback_frame_state
 (struct _Unwind_Context *context, _Unwind_FrameState *fs)
@@ -43,6 +79,7 @@
   struct sigframe *sf;
   long new_cfa;
 
+#ifndef KERN_PROC_SIGTRAMP
   /* Prior to FreeBSD 9, the signal trampoline was located immediately
  before the ps_strings.  To support non-executable stacks on AMD64,
  the sigtramp was moved to a shared page for FreeBSD 9.  Unfortunately
@@ -62,12 +99,15 @@
 && *(unsigned int *)(context->ra +  8) == 0x01a1c0c7
 && *(unsigned int *)(context->ra + 12) == 0x050f ))
 return _URC_END_OF_STACK;
+#else
+  if (x86_64_outside_sigtramp_range(context->ra))
+return _URC_END_OF_STACK;
+#endif
 
   sf = (struct sigframe *) context->cfa;
   new_cfa = sf->REG_NAME(rsp);
   fs->regs.cfa_how = CFA_REG_OFFSET;
-  /* Register 7 is rsp  */
-  fs->regs.cfa_reg = 7;
+  fs->regs.cfa_reg =  __LIBGCC_STACK_POINTER_REGNUM__;
   fs->regs.cfa_offset = new_cfa - (long) context->cfa;
 
   /* The SVR4 register numbering macros aren't usable in libgcc.  */
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Konstantin Belousov]

On Tue, Oct 31, 2017 at 08:37:29PM +0100, Andreas Tobler wrote:
> On 31.10.17 10:28, Konstantin Belousov wrote:
> > On Mon, Oct 30, 2017 at 10:54:05PM +0100, Andreas Tobler wrote:
> >> On 30.10.17 15:32, Tijl Coosemans wrote:
> >>> On Sun, 29 Oct 2017 20:40:46 +0100 Andreas Tobler 
> >>>  wrote:
>  Attached what I have for libgcc. It can be applied to gcc5-8, should
>  give no issues. The mentioned tc from this thread and mine,
>  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82635 do pass.
> 
>  What do you think?
> >>>
> >>> Like I said before the return address can be anything.  It could for
> >>> instance point to some instruction in a random function and then the
> >>> stack unwinder will think thread_start was called from that function.
> >>> There's no check you can add to libgcc to distinguish that from a
> >>> normal valid return address.
> >>>
> >> Maybe not, and most probably I do not understand what is happening. But
> >> with my modification I survive the test case.
> >>
> >> If no objections from your or Konstantin's side come up I will commit it
> >> to the gcc repo. It will not 'fix' the issue, but it will improve the
> >> gcc behavior.
> > 
> > I posted something similar when the discussion thread started. From the
> > cursory look, your patch is better than mine. The only difference that
> > makes me wonder is that I used #ifdef KERN_PROC_SIGTRAMP around the
> > block because I believe gcc has more relaxed policy about supporting
> > obsoleted OS versions.
> 
> I am aware about KERN_PROC_SIGTRAMP and older OS releases, that's why I 
> asked for feedback.
> Do we, FreeBSD'ers, want to have gcc unwind support on older than 
> FreeBSD 9.3 releases? I think the gcc folks do not care, but we are the 
> ones who might have an need for such a support?
Well, I put the #ifdef because I suspected that gcc folks cared, if
anybody.  For instance I know that perl people do.

Is there some specific configuration bits in gcc that are only relevant for 
older releases ?  If yes, then we perhaps should not break them until
removed.  If not, then it does not matter, most likely.

> @Gerald, do you have an opinion?
> 
> I can 'ifdef' the new code and in the 'else' case we fall back to the 
> already existing path.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Andreas Tobler]

On 31.10.17 10:28, Konstantin Belousov wrote:

On Mon, Oct 30, 2017 at 10:54:05PM +0100, Andreas Tobler wrote:

On 30.10.17 15:32, Tijl Coosemans wrote:

On Sun, 29 Oct 2017 20:40:46 +0100 Andreas Tobler  
wrote:

Attached what I have for libgcc. It can be applied to gcc5-8, should
give no issues. The mentioned tc from this thread and mine,
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82635 do pass.

What do you think?


Like I said before the return address can be anything.  It could for
instance point to some instruction in a random function and then the
stack unwinder will think thread_start was called from that function.
There's no check you can add to libgcc to distinguish that from a
normal valid return address.


Maybe not, and most probably I do not understand what is happening. But
with my modification I survive the test case.

If no objections from your or Konstantin's side come up I will commit it
to the gcc repo. It will not 'fix' the issue, but it will improve the
gcc behavior.


I posted something similar when the discussion thread started. From the
cursory look, your patch is better than mine. The only difference that
makes me wonder is that I used #ifdef KERN_PROC_SIGTRAMP around the
block because I believe gcc has more relaxed policy about supporting
obsoleted OS versions.


I am aware about KERN_PROC_SIGTRAMP and older OS releases, that's why I 
asked for feedback.
Do we, FreeBSD'ers, want to have gcc unwind support on older than 
FreeBSD 9.3 releases? I think the gcc folks do not care, but we are the 
ones who might have an need for such a support?

@Gerald, do you have an opinion?

I can 'ifdef' the new code and in the 'else' case we fall back to the 
already existing path.


Thank you both for the feedback.
Andreas

___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Tijl Coosemans]

On Mon, 30 Oct 2017 22:54:05 +0100 Andreas Tobler  
wrote:
> On 30.10.17 15:32, Tijl Coosemans wrote:
>> On Sun, 29 Oct 2017 20:40:46 +0100 Andreas Tobler  
>> wrote:  
>>> Attached what I have for libgcc. It can be applied to gcc5-8, should
>>> give no issues. The mentioned tc from this thread and mine,
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82635 do pass.
>>>
>>> What do you think?  
>> 
>> Like I said before the return address can be anything.  It could for
>> instance point to some instruction in a random function and then the
>> stack unwinder will think thread_start was called from that function.
>> There's no check you can add to libgcc to distinguish that from a
>> normal valid return address.
>
> Maybe not, and most probably I do not understand what is happening. But 
> with my modification I survive the test case.
> 
> If no objections from your or Konstantin's side come up I will commit it 
> to the gcc repo. It will not 'fix' the issue, but it will improve the 
> gcc behavior.

The patch looks good to me.  KERN_PROC_SIGTRAMP was added in 9.3 it
seems.  If gcc wants to support older versions you may have to use an
#ifdef like Konstantin did in his first reply in this thread.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Konstantin Belousov]

On Mon, Oct 30, 2017 at 10:54:05PM +0100, Andreas Tobler wrote:
> On 30.10.17 15:32, Tijl Coosemans wrote:
> > On Sun, 29 Oct 2017 20:40:46 +0100 Andreas Tobler  
> > wrote:
> >> Attached what I have for libgcc. It can be applied to gcc5-8, should
> >> give no issues. The mentioned tc from this thread and mine,
> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82635 do pass.
> >>
> >> What do you think?
> > 
> > Like I said before the return address can be anything.  It could for
> > instance point to some instruction in a random function and then the
> > stack unwinder will think thread_start was called from that function.
> > There's no check you can add to libgcc to distinguish that from a
> > normal valid return address.
> > 
> Maybe not, and most probably I do not understand what is happening. But 
> with my modification I survive the test case.
> 
> If no objections from your or Konstantin's side come up I will commit it 
> to the gcc repo. It will not 'fix' the issue, but it will improve the 
> gcc behavior.

I posted something similar when the discussion thread started. From the
cursory look, your patch is better than mine. The only difference that
makes me wonder is that I used #ifdef KERN_PROC_SIGTRAMP around the
block because I believe gcc has more relaxed policy about supporting
obsoleted OS versions.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Andreas Tobler]

On 30.10.17 15:32, Tijl Coosemans wrote:

On Sun, 29 Oct 2017 20:40:46 +0100 Andreas Tobler  
wrote:

Attached what I have for libgcc. It can be applied to gcc5-8, should
give no issues. The mentioned tc from this thread and mine,
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82635 do pass.

What do you think?


Like I said before the return address can be anything.  It could for
instance point to some instruction in a random function and then the
stack unwinder will think thread_start was called from that function.
There's no check you can add to libgcc to distinguish that from a
normal valid return address.

Maybe not, and most probably I do not understand what is happening. But 
with my modification I survive the test case.


If no objections from your or Konstantin's side come up I will commit it 
to the gcc repo. It will not 'fix' the issue, but it will improve the 
gcc behavior.


Andreas
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Tijl Coosemans]

On Sun, 29 Oct 2017 20:40:46 +0100 Andreas Tobler  
wrote:
> Attached what I have for libgcc. It can be applied to gcc5-8, should 
> give no issues. The mentioned tc from this thread and mine, 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82635 do pass.
> 
> What do you think?

Like I said before the return address can be anything.  It could for
instance point to some instruction in a random function and then the
stack unwinder will think thread_start was called from that function.
There's no check you can add to libgcc to distinguish that from a
normal valid return address.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Tijl Coosemans]

On Sun, 29 Oct 2017 21:13:58 +0200 Konstantin Belousov  
wrote:
> On Sun, Oct 29, 2017 at 06:23:51PM +0100, Tijl Coosemans wrote:
>> On Sat, 26 Aug 2017 21:40:34 +0300 Konstantin Belousov  
>> wrote:  
>>> On Sat, Aug 26, 2017 at 08:28:13PM +0200, Tijl Coosemans wrote:  
 I did consider using
 a CFI directive (see patch below) and it works, but it's architecture
 specific and it's inserted after the function prologue so there's still
 a window of a few instructions where a stack unwinder will try to use
 the return address.
 
 Index: lib/libthr/thread/thr_create.c
 ===
 --- lib/libthr/thread/thr_create.c  (revision 322802)
 +++ lib/libthr/thread/thr_create.c  (working copy)
 @@ -251,6 +251,7 @@ create_stack(struct pthread_attr *pattr)
  static void
  thread_start(struct pthread *curthread)
  {
 +   __asm(".cfi_undefined %rip");
 sigset_t set;
  
 if (curthread->attr.suspend == THR_CREATE_SUSPENDED)
>>> 
>>> I like this approach much more than the previous patch.  What can be
>>> done is to provide asm trampoline which calls thread_start().  There you
>>> can add the .cfi_undefined right at the entry.
>>>
>>> It is somewhat more work than just setting the return address on the
>>> kernel-constructed pseudo stack frame, but I believe this is ultimately
>>> correct way.  You still can do it only on some arches, if you do not
>>> have incentive to code asm for all of them.  
>> 
>> Ok, but then there are two ways to implement the trampoline:
>> 
>> 1)
>>  movq $0,(%rsp)
>>  jmp thread_start
>> 2)
>>  subq $8,%rsp
>>  call thread_start
>>  /* NOTREACHED */
>> 
>> With 1) you're setting the return address to zero anyway, so you might
>> as well do that in the kernel like my first patch.  With 2) you're
>> setting up a new call frame, basically redoing what the kernel already
>> did and on i386 this also means copying the function argument.  
> I do not quite understand the second variant, because the stack is not
> guaranteed to be zeroed, and it is often not if reused after the previously
> exited thread.

The problem is that the return address of thread_start can be garbage.
Solution 1 sets it to zero.  Nothing else changes, once thread_start runs
it's as if the trampoline never existed.  Solution 2 creates a new frame
and calls thread_start giving it a valid return address back to the
trampoline.  The trampoline still has a return address that can be garbage,
but it has a CFI directive saying its return address is undefined so
that's ok.

> The first variant is what I like, but perhaps we need to emulate the
> frame as well, i.e. push two zero longs.

That would be a hybrid of 1 and 2.

> Currently kernel does not access the usermode stack for the new thread
> unless dictated by ABI (i.e. it does not touch it for 64bit process
> on amd64, but have to for 32bit).  I like this property.  Also, the
> previous paragraph is indicative: we do not really know in kernel
> what ABI the userspace follows.  It might want frame, may be it does
> not need it.  It could use other register than %rbp as the frame base,
> etc.

Yes, but then the kernel should just set the stack pointer to the first
byte after the stack and pass the argument in a register.  Then userspace
can align the stack and store function arguments the way it likes and
call a C function or whatever else it wants to do.  Now the kernel is
already setting up the stack so the entry point can be a C function.

But anyway, we're both talking about hypothetical situations now so I'm
just going to make the decision and go with my earlier patch.  Then it's
clear that the kernel sets up the frame and there isn't some sort of split
responsibility between kernel and userland.  If there's ever a situation
where the kernel should not set up a call frame a new syscall can be
added.  In the end we're talking about the best way to set a word to zero
here.  We've spent too many emails on that already.

>> Do you have any preference (or better alternatives), because I think I
>> still prefer my first patch.  It's the caller's job to set up the call
>> frame, in this case the kernel.  And if the kernel handles it then it
>> also works with (hypothetical) implementations other than libthr.
>> 
>>> Also crt1 probably should get the same treatment, despite we already set
>>> %rbp to zero AFAIR.
>> 
>> I haven't checked but I imagine the return address of the process entry
>> point is always zero because the stack is all zeros.
> Stack is not zero. The environment and argument strings and auxv are copied
> at top, and at the bottom the ps_strings structure is located, so it
> is not.

Yes, what I meant is that it's newly allocated so even if the return
address is uninitialised it will be zero.  If it wasn't people would
have reported problems with 

Re: Segfault in _Unwind_* code called from pthread_exit

[Andreas Tobler]

On 29.10.17 20:13, Konstantin Belousov wrote:

On Sun, Oct 29, 2017 at 06:23:51PM +0100, Tijl Coosemans wrote:

On Sat, 26 Aug 2017 21:40:34 +0300 Konstantin Belousov  
wrote:

On Sat, Aug 26, 2017 at 08:28:13PM +0200, Tijl Coosemans wrote:

I did consider using
a CFI directive (see patch below) and it works, but it's architecture
specific and it's inserted after the function prologue so there's still
a window of a few instructions where a stack unwinder will try to use
the return address.

Index: lib/libthr/thread/thr_create.c
===
--- lib/libthr/thread/thr_create.c  (revision 322802)
+++ lib/libthr/thread/thr_create.c  (working copy)
@@ -251,6 +251,7 @@ create_stack(struct pthread_attr *pattr)
  static void
  thread_start(struct pthread *curthread)
  {
+   __asm(".cfi_undefined %rip");
 sigset_t set;
  
 if (curthread->attr.suspend == THR_CREATE_SUSPENDED)


I like this approach much more than the previous patch.  What can be
done is to provide asm trampoline which calls thread_start().  There you
can add the .cfi_undefined right at the entry.

It is somewhat more work than just setting the return address on the
kernel-constructed pseudo stack frame, but I believe this is ultimately
correct way.  You still can do it only on some arches, if you do not
have incentive to code asm for all of them.


Ok, but then there are two ways to implement the trampoline:

1)
movq $0,(%rsp)
jmp thread_start
2)
subq $8,%rsp
call thread_start
/* NOTREACHED */

With 1) you're setting the return address to zero anyway, so you might
as well do that in the kernel like my first patch.  With 2) you're
setting up a new call frame, basically redoing what the kernel already
did and on i386 this also means copying the function argument.

I do not quite understand the second variant, because the stack is not
guaranteed to be zeroed, and it is often not if reused after the previously
exited thread.

The first variant is what I like, but perhaps we need to emulate the
frame as well, i.e. push two zero longs.

Currently kernel does not access the usermode stack for the new thread
unless dictated by ABI (i.e. it does not touch it for 64bit process
on amd64, but have to for 32bit).  I like this property.  Also, the
previous paragraph is indicative: we do not really know in kernel
what ABI the userspace follows.  It might want frame, may be it does
not need it.  It could use other register than %rbp as the frame base,
etc.



Do you have any preference (or better alternatives), because I think I
still prefer my first patch.  It's the caller's job to set up the call
frame, in this case the kernel.  And if the kernel handles it then it
also works with (hypothetical) implementations other than libthr.


Also crt1 probably should get the same treatment, despite we already set
%rbp to zero AFAIR.


I haven't checked but I imagine the return address of the process entry
point is always zero because the stack is all zeros.

Stack is not zero. The environment and argument strings and auxv are copied
at top, and at the bottom the ps_strings structure is located, so it
is not.

If you commit your existing patch as is, I will not resent.  But I do think
that stuff that can be done in usermode, should be done in usermode, esp.
when the amount of efforts is same.


Attached what I have for libgcc. It can be applied to gcc5-8, should 
give no issues. The mentioned tc from this thread and mine, 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82635 do pass.


What do you think?

Andreas

Index: libgcc/config/i386/freebsd-unwind.h
===
--- libgcc/config/i386/freebsd-unwind.h (revision 254205)
+++ libgcc/config/i386/freebsd-unwind.h (working copy)
@@ -28,7 +28,10 @@
 
 #include 
 #include 
+#include 
+#include 
 #include 
+#include 
 #include 
 
 #define REG_NAME(reg)  sf_uc.uc_mcontext.mc_## reg
@@ -36,38 +39,46 @@
 #ifdef __x86_64__
 #define MD_FALLBACK_FRAME_STATE_FOR x86_64_freebsd_fallback_frame_state
 
+static int
+x86_64_outside_sigtramp_range (unsigned char *pc)
+{
+  static int sigtramp_range_determined = 0;
+  static unsigned char *sigtramp_start, *sigtramp_end;
+
+  if (sigtramp_range_determined == 0)
+{
+  struct kinfo_sigtramp kst = {0};
+  size_t len = sizeof (kst);
+  int mib[4] = { CTL_KERN, KERN_PROC, KERN_PROC_SIGTRAMP, getpid() };
+
+  sigtramp_range_determined = 1;
+  if (sysctl (mib, 4, , , NULL, 0) == 0)
+  {
+sigtramp_range_determined = 2;
+sigtramp_start = kst.ksigtramp_start;
+sigtramp_end   = kst.ksigtramp_end;
+  }
+}
+  if (sigtramp_range_determined < 2)  /* sysctl failed if < 2 */
+return 1;
+
+  return (pc < sigtramp_start || pc >= sigtramp_end);
+}
+
 static _Unwind_Reason_Code
 x86_64_freebsd_fallback_frame_state
 (struct _Unwind_Context *context, 

Re: Segfault in _Unwind_* code called from pthread_exit

[Konstantin Belousov]

On Sun, Oct 29, 2017 at 06:23:51PM +0100, Tijl Coosemans wrote:
> On Sat, 26 Aug 2017 21:40:34 +0300 Konstantin Belousov  
> wrote:
> > On Sat, Aug 26, 2017 at 08:28:13PM +0200, Tijl Coosemans wrote:
> >> I did consider using
> >> a CFI directive (see patch below) and it works, but it's architecture
> >> specific and it's inserted after the function prologue so there's still
> >> a window of a few instructions where a stack unwinder will try to use
> >> the return address.
> >> 
> >> Index: lib/libthr/thread/thr_create.c
> >> ===
> >> --- lib/libthr/thread/thr_create.c  (revision 322802)
> >> +++ lib/libthr/thread/thr_create.c  (working copy)
> >> @@ -251,6 +251,7 @@ create_stack(struct pthread_attr *pattr)
> >>  static void
> >>  thread_start(struct pthread *curthread)
> >>  {
> >> +   __asm(".cfi_undefined %rip");
> >> sigset_t set;
> >>  
> >> if (curthread->attr.suspend == THR_CREATE_SUSPENDED)  
> > 
> > I like this approach much more than the previous patch.  What can be
> > done is to provide asm trampoline which calls thread_start().  There you
> > can add the .cfi_undefined right at the entry.
> >
> > It is somewhat more work than just setting the return address on the
> > kernel-constructed pseudo stack frame, but I believe this is ultimately
> > correct way.  You still can do it only on some arches, if you do not
> > have incentive to code asm for all of them.
> 
> Ok, but then there are two ways to implement the trampoline:
> 
> 1)
>   movq $0,(%rsp)
>   jmp thread_start
> 2)
>   subq $8,%rsp
>   call thread_start
>   /* NOTREACHED */
> 
> With 1) you're setting the return address to zero anyway, so you might
> as well do that in the kernel like my first patch.  With 2) you're
> setting up a new call frame, basically redoing what the kernel already
> did and on i386 this also means copying the function argument.
I do not quite understand the second variant, because the stack is not
guaranteed to be zeroed, and it is often not if reused after the previously
exited thread.

The first variant is what I like, but perhaps we need to emulate the
frame as well, i.e. push two zero longs.

Currently kernel does not access the usermode stack for the new thread
unless dictated by ABI (i.e. it does not touch it for 64bit process
on amd64, but have to for 32bit).  I like this property.  Also, the
previous paragraph is indicative: we do not really know in kernel
what ABI the userspace follows.  It might want frame, may be it does
not need it.  It could use other register than %rbp as the frame base,
etc.

> 
> Do you have any preference (or better alternatives), because I think I
> still prefer my first patch.  It's the caller's job to set up the call
> frame, in this case the kernel.  And if the kernel handles it then it
> also works with (hypothetical) implementations other than libthr.
> 
> > Also crt1 probably should get the same treatment, despite we already set
> > %rbp to zero AFAIR.
> 
> I haven't checked but I imagine the return address of the process entry
> point is always zero because the stack is all zeros.
Stack is not zero. The environment and argument strings and auxv are copied
at top, and at the bottom the ps_strings structure is located, so it
is not.

If you commit your existing patch as is, I will not resent.  But I do think
that stuff that can be done in usermode, should be done in usermode, esp.
when the amount of efforts is same.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Tijl Coosemans]

On Sat, 26 Aug 2017 21:40:34 +0300 Konstantin Belousov  
wrote:
> On Sat, Aug 26, 2017 at 08:28:13PM +0200, Tijl Coosemans wrote:
>> I did consider using
>> a CFI directive (see patch below) and it works, but it's architecture
>> specific and it's inserted after the function prologue so there's still
>> a window of a few instructions where a stack unwinder will try to use
>> the return address.
>> 
>> Index: lib/libthr/thread/thr_create.c
>> ===
>> --- lib/libthr/thread/thr_create.c  (revision 322802)
>> +++ lib/libthr/thread/thr_create.c  (working copy)
>> @@ -251,6 +251,7 @@ create_stack(struct pthread_attr *pattr)
>>  static void
>>  thread_start(struct pthread *curthread)
>>  {
>> +   __asm(".cfi_undefined %rip");
>> sigset_t set;
>>  
>> if (curthread->attr.suspend == THR_CREATE_SUSPENDED)  
> 
> I like this approach much more than the previous patch.  What can be
> done is to provide asm trampoline which calls thread_start().  There you
> can add the .cfi_undefined right at the entry.
>
> It is somewhat more work than just setting the return address on the
> kernel-constructed pseudo stack frame, but I believe this is ultimately
> correct way.  You still can do it only on some arches, if you do not
> have incentive to code asm for all of them.

Ok, but then there are two ways to implement the trampoline:

1)
movq $0,(%rsp)
jmp thread_start
2)
subq $8,%rsp
call thread_start
/* NOTREACHED */

With 1) you're setting the return address to zero anyway, so you might
as well do that in the kernel like my first patch.  With 2) you're
setting up a new call frame, basically redoing what the kernel already
did and on i386 this also means copying the function argument.

Do you have any preference (or better alternatives), because I think I
still prefer my first patch.  It's the caller's job to set up the call
frame, in this case the kernel.  And if the kernel handles it then it
also works with (hypothetical) implementations other than libthr.

> Also crt1 probably should get the same treatment, despite we already set
> %rbp to zero AFAIR.

I haven't checked but I imagine the return address of the process entry
point is always zero because the stack is all zeros.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Tijl Coosemans]

On Sun, 22 Oct 2017 23:05:15 +0200 Andreas Tobler  wrote:
> On 22.10.17 02:18, Tijl Coosemans wrote:
>> On Sat, 21 Oct 2017 22:02:38 +0200 Andreas Tobler  
>> wrote:  
>>> On 26.08.17 20:40, Konstantin Belousov wrote:  
 On Sat, Aug 26, 2017 at 08:28:13PM +0200, Tijl Coosemans wrote:  
> On Sat, 26 Aug 2017 02:44:42 +0300 Konstantin Belousov 
>  wote:  
>> How does llvm unwinder detects that the return address is a garbage ?  
>
> It just stops unwinding when it can't find frame information (stored in
> .eh_frame sections).  GCC unwinder doesn't give up yet and checks if the
> return address points to the signal trampoline (which means the current
> frame is that of a signal handler).  It has built-in knowledge of how to
> unwind to the signal trampoline frame.  
 So llvm just gives up on signal frames ?
  
> A noreturn attribute isn't enough.  You can still unwind such functions.
> They are allowed to throw exceptions for example.  
 Ok.
  
> I did consider using
> a CFI directive (see patch below) and it works, but it's architecture
> specific and it's inserted after the function prologue so there's still
> a window of a few instructions where a stack unwinder will try to use
> the return address.
>
> Index: lib/libthr/thread/thr_create.c
> ===
> --- lib/libthr/thread/thr_create.c  (revision 322802)
> +++ lib/libthr/thread/thr_create.c  (working copy)
> @@ -251,6 +251,7 @@ create_stack(struct pthread_attr *pattr)
>static void
>thread_start(struct pthread *curthread)
>{
> +   __asm(".cfi_undefined %rip");
>   sigset_t set;
>
>   if (curthread->attr.suspend == THR_CREATE_SUSPENDED)  

 I like this approach much more than the previous patch.  What can be
 done is to provide asm trampoline which calls thread_start().  There you
 can add the .cfi_undefined right at the entry.

 It is somewhat more work than just setting the return address on the
 kernel-constructed pseudo stack frame, but I believe this is ultimately
 correct way.  You still can do it only on some arches, if you do not
 have incentive to code asm for all of them.

 Also crt1 probably should get the same treatment, despite we already set
 %rbp to zero AFAIR.  
>>>
>>> Did some commit result out of this discussion or is this subject still
>>> under investigation?
>>>
>>> Curious because I got this gcc PR:
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82635  
> 
> If I add the above to lib/libthr/thread/thr_create.c the mentioned PR works.
> 
>> Sorry, but I didn't and won't have time to work on this.  
> 
> Np.
> 
>> Ideally I think there should be a function attribute to mark functions
>> as entry points.  The compiler would add ".cfi_undefined %rip" to such
>> functions (and maybe optimise the function prologue because there are
>> no caller registers that need to be preserved).  If you have connections
>> in the GCC community maybe you could discuss that with them.  
> 
> Well, from my understanding I'd have to teach every compiler to do so, 
> right? (Beside that I do not know how to.)
> 
> I think we need another solution to find out if an unwind context is 
> garbage.
> 
> I'll take a look at how llvm does this w/o segfaulting.

Both LLVM and GCC libgcc_s lookup the return address in the CFI tables
to see which function it belongs to.  Both fail to find the address in
the table.  LLVM then stops unwinding.  GCC looks at the memory to see
if the instructions are those of the signal trampoline and segfaults if
the memory is unreadable.

The return address of the entry point can be anything including addresses
that do appear in the CFI table, because thread stacks can be supplied
by the user (pthread_attr_setstack(3)).  So it's not enough to avoid the
segfault.  Either the kernel has to set the return address to NULL like
my first patch or the entry point has to be annotated with .cfi_undefined
%rip to mark the return address invalid.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Andreas Tobler]

On 22.10.17 02:18, Tijl Coosemans wrote:

On Sat, 21 Oct 2017 22:02:38 +0200 Andreas Tobler  wrote:

On 26.08.17 20:40, Konstantin Belousov wrote:

On Sat, Aug 26, 2017 at 08:28:13PM +0200, Tijl Coosemans wrote:

On Sat, 26 Aug 2017 02:44:42 +0300 Konstantin Belousov  
wrote:

How does llvm unwinder detects that the return address is a garbage ?


It just stops unwinding when it can't find frame information (stored in
.eh_frame sections).  GCC unwinder doesn't give up yet and checks if the
return address points to the signal trampoline (which means the current
frame is that of a signal handler).  It has built-in knowledge of how to
unwind to the signal trampoline frame.

So llvm just gives up on signal frames ?
   

A noreturn attribute isn't enough.  You can still unwind such functions.
They are allowed to throw exceptions for example.

Ok.
   

I did consider using
a CFI directive (see patch below) and it works, but it's architecture
specific and it's inserted after the function prologue so there's still
a window of a few instructions where a stack unwinder will try to use
the return address.

Index: lib/libthr/thread/thr_create.c
===
--- lib/libthr/thread/thr_create.c  (revision 322802)
+++ lib/libthr/thread/thr_create.c  (working copy)
@@ -251,6 +251,7 @@ create_stack(struct pthread_attr *pattr)
   static void
   thread_start(struct pthread *curthread)
   {
+   __asm(".cfi_undefined %rip");
  sigset_t set;
   
  if (curthread->attr.suspend == THR_CREATE_SUSPENDED)


I like this approach much more than the previous patch.  What can be
done is to provide asm trampoline which calls thread_start().  There you
can add the .cfi_undefined right at the entry.

It is somewhat more work than just setting the return address on the
kernel-constructed pseudo stack frame, but I believe this is ultimately
correct way.  You still can do it only on some arches, if you do not
have incentive to code asm for all of them.

Also crt1 probably should get the same treatment, despite we already set
%rbp to zero AFAIR.


Did some commit result out of this discussion or is this subject still
under investigation?

Curious because I got this gcc PR:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82635


If I add the above to lib/libthr/thread/thr_create.c the mentioned PR works.


Sorry, but I didn't and won't have time to work on this.


Np.


Ideally I think there should be a function attribute to mark functions
as entry points.  The compiler would add ".cfi_undefined %rip" to such
functions (and maybe optimise the function prologue because there are
no caller registers that need to be preserved).  If you have connections
in the GCC community maybe you could discuss that with them.


Well, from my understanding I'd have to teach every compiler to do so, 
right? (Beside that I do not know how to.)


I think we need another solution to find out if an unwind context is 
garbage.


I'll take a look at how llvm does this w/o segfaulting.

Thx,
Andreas

___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Tijl Coosemans]

On Sat, 21 Oct 2017 22:02:38 +0200 Andreas Tobler  wrote:
> On 26.08.17 20:40, Konstantin Belousov wrote:
>> On Sat, Aug 26, 2017 at 08:28:13PM +0200, Tijl Coosemans wrote:  
>>> On Sat, 26 Aug 2017 02:44:42 +0300 Konstantin Belousov 
>>>  wrote:  
 How does llvm unwinder detects that the return address is a garbage ?  
>>>
>>> It just stops unwinding when it can't find frame information (stored in
>>> .eh_frame sections).  GCC unwinder doesn't give up yet and checks if the
>>> return address points to the signal trampoline (which means the current
>>> frame is that of a signal handler).  It has built-in knowledge of how to
>>> unwind to the signal trampoline frame.  
>> So llvm just gives up on signal frames ?
>>   
>>> A noreturn attribute isn't enough.  You can still unwind such functions.
>>> They are allowed to throw exceptions for example.  
>> Ok.
>>   
>>> I did consider using
>>> a CFI directive (see patch below) and it works, but it's architecture
>>> specific and it's inserted after the function prologue so there's still
>>> a window of a few instructions where a stack unwinder will try to use
>>> the return address.
>>>
>>> Index: lib/libthr/thread/thr_create.c
>>> ===
>>> --- lib/libthr/thread/thr_create.c  (revision 322802)
>>> +++ lib/libthr/thread/thr_create.c  (working copy)
>>> @@ -251,6 +251,7 @@ create_stack(struct pthread_attr *pattr)
>>>   static void
>>>   thread_start(struct pthread *curthread)
>>>   {
>>> +   __asm(".cfi_undefined %rip");
>>>  sigset_t set;
>>>   
>>>  if (curthread->attr.suspend == THR_CREATE_SUSPENDED)  
>> 
>> I like this approach much more than the previous patch.  What can be
>> done is to provide asm trampoline which calls thread_start().  There you
>> can add the .cfi_undefined right at the entry.
>> 
>> It is somewhat more work than just setting the return address on the
>> kernel-constructed pseudo stack frame, but I believe this is ultimately
>> correct way.  You still can do it only on some arches, if you do not
>> have incentive to code asm for all of them.
>> 
>> Also crt1 probably should get the same treatment, despite we already set
>> %rbp to zero AFAIR.  
> 
> Did some commit result out of this discussion or is this subject still 
> under investigation?
> 
> Curious because I got this gcc PR:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82635

Sorry, but I didn't and won't have time to work on this.

Ideally I think there should be a function attribute to mark functions
as entry points.  The compiler would add ".cfi_undefined %rip" to such
functions (and maybe optimise the function prologue because there are
no caller registers that need to be preserved).  If you have connections
in the GCC community maybe you could discuss that with them.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Konstantin Belousov]

On Sat, Oct 21, 2017 at 10:02:38PM +0200, Andreas Tobler wrote:
> On 26.08.17 20:40, Konstantin Belousov wrote:
> > On Sat, Aug 26, 2017 at 08:28:13PM +0200, Tijl Coosemans wrote:
> >> On Sat, 26 Aug 2017 02:44:42 +0300 Konstantin Belousov 
> >>  wrote:
> >>> How does llvm unwinder detects that the return address is a garbage ?
> >>
> >> It just stops unwinding when it can't find frame information (stored in
> >> .eh_frame sections).  GCC unwinder doesn't give up yet and checks if the
> >> return address points to the signal trampoline (which means the current
> >> frame is that of a signal handler).  It has built-in knowledge of how to
> >> unwind to the signal trampoline frame.
> > So llvm just gives up on signal frames ?
> > 
> >> A noreturn attribute isn't enough.  You can still unwind such functions.
> >> They are allowed to throw exceptions for example.
> > Ok.
> > 
> >> I did consider using
> >> a CFI directive (see patch below) and it works, but it's architecture
> >> specific and it's inserted after the function prologue so there's still
> >> a window of a few instructions where a stack unwinder will try to use
> >> the return address.
> >>
> >> Index: lib/libthr/thread/thr_create.c
> >> ===
> >> --- lib/libthr/thread/thr_create.c  (revision 322802)
> >> +++ lib/libthr/thread/thr_create.c  (working copy)
> >> @@ -251,6 +251,7 @@ create_stack(struct pthread_attr *pattr)
> >>   static void
> >>   thread_start(struct pthread *curthread)
> >>   {
> >> +   __asm(".cfi_undefined %rip");
> >>  sigset_t set;
> >>   
> >>  if (curthread->attr.suspend == THR_CREATE_SUSPENDED)
> > 
> > I like this approach much more than the previous patch.  What can be
> > done is to provide asm trampoline which calls thread_start().  There you
> > can add the .cfi_undefined right at the entry.
> > 
> > It is somewhat more work than just setting the return address on the
> > kernel-constructed pseudo stack frame, but I believe this is ultimately
> > correct way.  You still can do it only on some arches, if you do not
> > have incentive to code asm for all of them.
> > 
> > Also crt1 probably should get the same treatment, despite we already set
> > %rbp to zero AFAIR.
> 
> Did some commit result out of this discussion or is this subject still 
> under investigation?
Nothing was done AFAIK.

> 
> Curious because I got this gcc PR:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82635
> 
> Tia,
> Andreas
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Andreas Tobler]

On 26.08.17 20:40, Konstantin Belousov wrote:

On Sat, Aug 26, 2017 at 08:28:13PM +0200, Tijl Coosemans wrote:

On Sat, 26 Aug 2017 02:44:42 +0300 Konstantin Belousov  
wrote:

How does llvm unwinder detects that the return address is a garbage ?


It just stops unwinding when it can't find frame information (stored in
.eh_frame sections).  GCC unwinder doesn't give up yet and checks if the
return address points to the signal trampoline (which means the current
frame is that of a signal handler).  It has built-in knowledge of how to
unwind to the signal trampoline frame.

So llvm just gives up on signal frames ?


A noreturn attribute isn't enough.  You can still unwind such functions.
They are allowed to throw exceptions for example.

Ok.


I did consider using
a CFI directive (see patch below) and it works, but it's architecture
specific and it's inserted after the function prologue so there's still
a window of a few instructions where a stack unwinder will try to use
the return address.

Index: lib/libthr/thread/thr_create.c
===
--- lib/libthr/thread/thr_create.c  (revision 322802)
+++ lib/libthr/thread/thr_create.c  (working copy)
@@ -251,6 +251,7 @@ create_stack(struct pthread_attr *pattr)
  static void
  thread_start(struct pthread *curthread)
  {
+   __asm(".cfi_undefined %rip");
 sigset_t set;
  
 if (curthread->attr.suspend == THR_CREATE_SUSPENDED)


I like this approach much more than the previous patch.  What can be
done is to provide asm trampoline which calls thread_start().  There you
can add the .cfi_undefined right at the entry.

It is somewhat more work than just setting the return address on the
kernel-constructed pseudo stack frame, but I believe this is ultimately
correct way.  You still can do it only on some arches, if you do not
have incentive to code asm for all of them.

Also crt1 probably should get the same treatment, despite we already set
%rbp to zero AFAIR.


Did some commit result out of this discussion or is this subject still 
under investigation?


Curious because I got this gcc PR:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82635

Tia,
Andreas

___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Tijl Coosemans]

On Sat, 26 Aug 2017 21:40:34 +0300 Konstantin Belousov  
wrote:
> On Sat, Aug 26, 2017 at 08:28:13PM +0200, Tijl Coosemans wrote:
>> On Sat, 26 Aug 2017 02:44:42 +0300 Konstantin Belousov  
>> wrote:  
>>> How does llvm unwinder detects that the return address is a garbage ?  
>> 
>> It just stops unwinding when it can't find frame information (stored in
>> .eh_frame sections).  GCC unwinder doesn't give up yet and checks if the
>> return address points to the signal trampoline (which means the current
>> frame is that of a signal handler).  It has built-in knowledge of how to
>> unwind to the signal trampoline frame.  
> So llvm just gives up on signal frames ?

Looks like it.  This program doesn't print anything when using base
libgcc_s.  With gcc libgcc_s it prints:

0x400904  at /usr/home/tijl/testsig
0x7173 <_fini+0x7fbfe7bb> at ???

cc -o test test.c -lexecinfo -lgcc_s -rpath /usr/local/lib/gcc5


#include 
#include 

void *buf[ 20 ];
size_t s;

void
handler( int sig ) {
s = backtrace( buf, 20 );
}

int
main( void ) {
signal( SIGINT, handler );
raise( SIGINT );
backtrace_symbols_fd( buf, s, 1 );
return( 0 );
}

___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Konstantin Belousov]

On Sat, Aug 26, 2017 at 08:28:13PM +0200, Tijl Coosemans wrote:
> On Sat, 26 Aug 2017 02:44:42 +0300 Konstantin Belousov  
> wrote:
> > How does llvm unwinder detects that the return address is a garbage ?
> 
> It just stops unwinding when it can't find frame information (stored in
> .eh_frame sections).  GCC unwinder doesn't give up yet and checks if the
> return address points to the signal trampoline (which means the current
> frame is that of a signal handler).  It has built-in knowledge of how to
> unwind to the signal trampoline frame.
So llvm just gives up on signal frames ?

> A noreturn attribute isn't enough.  You can still unwind such functions.
> They are allowed to throw exceptions for example.
Ok.

> I did consider using
> a CFI directive (see patch below) and it works, but it's architecture
> specific and it's inserted after the function prologue so there's still
> a window of a few instructions where a stack unwinder will try to use
> the return address.
> 
> Index: lib/libthr/thread/thr_create.c
> ===
> --- lib/libthr/thread/thr_create.c  (revision 322802)
> +++ lib/libthr/thread/thr_create.c  (working copy)
> @@ -251,6 +251,7 @@ create_stack(struct pthread_attr *pattr)
>  static void
>  thread_start(struct pthread *curthread)
>  {
> +   __asm(".cfi_undefined %rip");
> sigset_t set;
>  
> if (curthread->attr.suspend == THR_CREATE_SUSPENDED)

I like this approach much more than the previous patch.  What can be
done is to provide asm trampoline which calls thread_start().  There you
can add the .cfi_undefined right at the entry.

It is somewhat more work than just setting the return address on the
kernel-constructed pseudo stack frame, but I believe this is ultimately
correct way.  You still can do it only on some arches, if you do not
have incentive to code asm for all of them.

Also crt1 probably should get the same treatment, despite we already set
%rbp to zero AFAIR.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Tijl Coosemans]

On Sat, 26 Aug 2017 02:44:42 +0300 Konstantin Belousov  
wrote:
> On Fri, Aug 25, 2017 at 05:38:51PM +0200, Tijl Coosemans wrote:
>> So both GCC and LLVM unwinding look up the return address in the CFI
>> table and fail when the return address is garbage, but LLVM treats this
>> as an end-of-stack condition while GCC further tries to see if the
>> return address points to a signal trampoline by testing the instruction
>> bytes at that address.  On amd64 the garbage address is unreadable so it
>> segfaults.  On i386 it is readable, the test fails and GCC returns
>> end-of-stack.  
> How does llvm unwinder detects that the return address is a garbage ?

It just stops unwinding when it can't find frame information (stored in
.eh_frame sections).  GCC unwinder doesn't give up yet and checks if the
return address points to the signal trampoline (which means the current
frame is that of a signal handler).  It has built-in knowledge of how to
unwind to the signal trampoline frame.

>> To fix the crash and get predictable behaviour in the other cases I
>> propose always setting the return address to 0.  The attached patch does
>> this for i386 and amd64.  I don't know if other architectures need a
>> similar patch.  
>>
>> Index: sys/amd64/amd64/vm_machdep.c
>> ===
>> --- sys/amd64/amd64/vm_machdep.c (revision 322802)
>> +++ sys/amd64/amd64/vm_machdep.c (working copy)
>> @@ -507,6 +507,9 @@ cpu_set_upcall(struct thread *td, void (*entry)(void *
>> (((uintptr_t)stack->ss_sp + stack->ss_size - 4) & ~0x0f) - 4;
>>  td->td_frame->tf_rip = (uintptr_t)entry;
>>  
>> +/* Sentinel return address to stop stack unwinding. */
>> +suword32((void *)td->td_frame->tf_rsp, 0);
>> +
>>  /* Pass the argument to the entry point. */
>>  suword32((void *)(td->td_frame->tf_rsp + sizeof(int32_t)),
>>  (uint32_t)(uintptr_t)arg);
>> @@ -529,6 +532,9 @@ cpu_set_upcall(struct thread *td, void (*entry)(void *
>>  td->td_frame->tf_fs = _ufssel;
>>  td->td_frame->tf_gs = _ugssel;
>>  td->td_frame->tf_flags = TF_HASSEGS;
>> +
>> +/* Sentinel return address to stop stack unwinding. */
>> +suword((void *)td->td_frame->tf_rsp, 0);
>>  
>>  /* Pass the argument to the entry point. */
>>  td->td_frame->tf_rdi = (register_t)arg;
>> Index: sys/i386/i386/vm_machdep.c
>> ===
>> --- sys/i386/i386/vm_machdep.c   (revision 322802)
>> +++ sys/i386/i386/vm_machdep.c   (working copy)
>> @@ -524,6 +524,9 @@ cpu_set_upcall(struct thread *td, void (*entry)(void *
>>  (((int)stack->ss_sp + stack->ss_size - 4) & ~0x0f) - 4;
>>  td->td_frame->tf_eip = (int)entry;
>>  
>> +/* Sentinel return address to stop stack unwinding. */
>> +suword((void *)td->td_frame->tf_esp, 0);
>> +
>>  /* Pass the argument to the entry point. */
>>  suword((void *)(td->td_frame->tf_esp + sizeof(void *)),
>>  (int)arg);  
> 
> I do not object against this, but I believe that a better solution exists
> for the system side (putting my change for gcc unwinder to detect the
> signal frame aside).  The thread_start() sentinel in libthr should get
> proper dwarf annotation of not having the return address.  May be
> normal function attributes of no return are enough to force compilers
> to generate required unwind data.  Might be some more magic with inline
> asm and .cfi_return_column set to undefined.

A noreturn attribute isn't enough.  You can still unwind such functions.
They are allowed to throw exceptions for example.  I did consider using
a CFI directive (see patch below) and it works, but it's architecture
specific and it's inserted after the function prologue so there's still
a window of a few instructions where a stack unwinder will try to use
the return address.

Index: lib/libthr/thread/thr_create.c
===
--- lib/libthr/thread/thr_create.c  (revision 322802)
+++ lib/libthr/thread/thr_create.c  (working copy)
@@ -251,6 +251,7 @@ create_stack(struct pthread_attr *pattr)
 static void
 thread_start(struct pthread *curthread)
 {
+   __asm(".cfi_undefined %rip");
sigset_t set;
 
if (curthread->attr.suspend == THR_CREATE_SUSPENDED)
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Konstantin Belousov]

On Fri, Aug 25, 2017 at 05:38:51PM +0200, Tijl Coosemans wrote:
> So both GCC and LLVM unwinding look up the return address in the CFI
> table and fail when the return address is garbage, but LLVM treats this
> as an end-of-stack condition while GCC further tries to see if the
> return address points to a signal trampoline by testing the instruction
> bytes at that address.  On amd64 the garbage address is unreadable so it
> segfaults.  On i386 it is readable, the test fails and GCC returns
> end-of-stack.
How does llvm unwinder detects that the return address is a garbage ?

> 
> To fix the crash and get predictable behaviour in the other cases I
> propose always setting the return address to 0.  The attached patch does
> this for i386 and amd64.  I don't know if other architectures need a
> similar patch.

> Index: sys/amd64/amd64/vm_machdep.c
> ===
> --- sys/amd64/amd64/vm_machdep.c  (revision 322802)
> +++ sys/amd64/amd64/vm_machdep.c  (working copy)
> @@ -507,6 +507,9 @@ cpu_set_upcall(struct thread *td, void (*entry)(void *
>  (((uintptr_t)stack->ss_sp + stack->ss_size - 4) & ~0x0f) - 4;
>   td->td_frame->tf_rip = (uintptr_t)entry;
>  
> + /* Sentinel return address to stop stack unwinding. */
> + suword32((void *)td->td_frame->tf_rsp, 0);
> +
>   /* Pass the argument to the entry point. */
>   suword32((void *)(td->td_frame->tf_rsp + sizeof(int32_t)),
>   (uint32_t)(uintptr_t)arg);
> @@ -529,6 +532,9 @@ cpu_set_upcall(struct thread *td, void (*entry)(void *
>   td->td_frame->tf_fs = _ufssel;
>   td->td_frame->tf_gs = _ugssel;
>   td->td_frame->tf_flags = TF_HASSEGS;
> +
> + /* Sentinel return address to stop stack unwinding. */
> + suword((void *)td->td_frame->tf_rsp, 0);
>  
>   /* Pass the argument to the entry point. */
>   td->td_frame->tf_rdi = (register_t)arg;
> Index: sys/i386/i386/vm_machdep.c
> ===
> --- sys/i386/i386/vm_machdep.c(revision 322802)
> +++ sys/i386/i386/vm_machdep.c(working copy)
> @@ -524,6 +524,9 @@ cpu_set_upcall(struct thread *td, void (*entry)(void *
>   (((int)stack->ss_sp + stack->ss_size - 4) & ~0x0f) - 4;
>   td->td_frame->tf_eip = (int)entry;
>  
> + /* Sentinel return address to stop stack unwinding. */
> + suword((void *)td->td_frame->tf_esp, 0);
> +
>   /* Pass the argument to the entry point. */
>   suword((void *)(td->td_frame->tf_esp + sizeof(void *)),
>   (int)arg);

I do not object against this, but I believe that a better solution exists
for the system side (putting my change for gcc unwinder to detect the
signal frame aside).  The thread_start() sentinel in libthr should get
proper dwarf annotation of not having the return address.  May be
normal function attributes of no return are enough to force compilers
to generate required unwind data.  Might be some more magic with inline
asm and .cfi_return_column set to undefined.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Mark Millard]

Tijl Coosemans tijl at FreeBSD.org wrote on
Fri Aug 25 15:40:10 UTC 2017 :

> So both GCC and LLVM unwinding look up the return address in the CFI
> table and fail when the return address is garbage, but LLVM treats this
> as an end-of-stack condition while GCC further tries to see if the
> return address points to a signal trampoline by testing the instruction
> bytes at that address.  On amd64 the garbage address is unreadable so it
> segfaults.  On i386 it is readable, the test fails and GCC returns
> end-of-stack.
> 
> To fix the crash and get predictable behaviour in the other cases I
> propose always setting the return address to 0.  The attached patch does
> this for i386 and amd64.  I don't know if other architectures need a
> similar patch.

If this is fixed it is possibly the fix for bugzilla report:

Bug 221423 - gcc std::locale(LocaleName) crashes instead of
throwing an exception


It may also fix some examples mentioned in comments for:

Bug 221288 - lang/gcc5 links against libsupc++ when compiling

but the original description did not happen to involve
exception handling from what I can see. Instead __dynamic_cast
failed.

===
Mark Millard
markmi at dsl-only.net

___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Tijl Coosemans]

On Thu, 24 Aug 2017 18:08:30 +0200 Tijl Coosemans  wrote:
> On Thu, 24 Aug 2017 18:42:35 +0300 Konstantin Belousov  
> wrote:
>> On Wed, Aug 23, 2017 at 04:37:07PM +0200, Tijl Coosemans wrote:  
>>> The following program segfaults for me on amd64 when linked like this:
>>> 
>>> cc -o test test.c -lpthread -L/usr/local/lib/gcc5 -lgcc_s -rpath 
>>> /usr/local/lib/gcc5
>>> 
>>> 
>>> #include 
>>> #include 
>>> 
>>> void *
>>> thr( void *arg ) {
>>> return( NULL );
>>> }
>>> 
>>> int
>>> main( void ) {
>>> pthread_t thread;
>>> 
>>> for( int i = 1; i < 20; i++ ) {
>>> fprintf( stderr, "%d\n", i );
>>> pthread_create( , NULL, thr, NULL );
>>> pthread_join( thread, NULL );
>>> }
>>> return( 0 );
>>> }
>>> 
>>> 
>>> The backtrace looks like this:
>>> 
>>> Thread 7 received signal SIGSEGV, Segmentation fault.
>>> [Switching to LWP 100511 of process 1886]
>>> uw_frame_state_for (context=context@entry=0x7fffdfffddc0, 
>>> fs=fs@entry=0x7fffdfffdb10)
>>> at /usr/ports/lang/gcc5/work/gcc-5.4.0/libgcc/unwind-dw2.c:1249
>>> 1249/usr/ports/lang/gcc5/work/gcc-5.4.0/libgcc/unwind-dw2.c: No 
>>> such file or directory.
>>> (gdb) bt
>>> #0  uw_frame_state_for (context=context@entry=0x7fffdfffddc0, 
>>> fs=fs@entry=0x7fffdfffdb10)
>>> at /usr/ports/lang/gcc5/work/gcc-5.4.0/libgcc/unwind-dw2.c:1249
>>> #1  0x000800a66ecb in _Unwind_ForcedUnwind_Phase2 (
>>> exc=exc@entry=0x800658730, context=context@entry=0x7fffdfffddc0)
>>> at /usr/ports/lang/gcc5/work/gcc-5.4.0/libgcc/unwind.inc:155
>>> #2  0x000800a67200 in _Unwind_ForcedUnwind (exc=0x800658730, 
>>> stop=0x8008428b0 , stop_argument=0x0)
>>> at /usr/ports/lang/gcc5/work/gcc-5.4.0/libgcc/unwind.inc:207
>>> #3  0x000800842224 in _Unwind_ForcedUnwind (ex=0x800658730, 
>>> stop_func=0x8008428b0 , stop_arg=0x0)
>>> at /usr/src/lib/libthr/thread/thr_exit.c:106
>>> #4  0x00080084269f in thread_unwind ()
>>> at /usr/src/lib/libthr/thread/thr_exit.c:172
>>> #5  0x0008008424d6 in _pthread_exit_mask (status=0x0, mask=0x0)
>>> at /usr/src/lib/libthr/thread/thr_exit.c:254
>>> #6  0x000800842359 in _pthread_exit (status=0x0)
>>> at /usr/src/lib/libthr/thread/thr_exit.c:206
>>> #7  0x00080082ccb1 in thread_start (curthread=0x800658500)
>>> at /usr/src/lib/libthr/thread/thr_create.c:289
>>> #8  0x7fffdfdfe000 in ?? ()
>>> Backtrace stopped: Cannot access memory at address 0x7fffdfffe000
>>> 
>>> 
>>> It happens with gcc6 as well, but not with base libgcc_s.
>>> Can anyone reproduce this?  Have there been any changes to stack
>>> unwinding recently (last few months)?
>> 
>> I can reproduce this, and there was a change in gcc unwinder, it seems.
>> Below is a patch which I did not even compiled.  Still, it should give
>> an idea how it might be approached.  The patch is against gcc head.  
> 
> Currently I'm thinking to patch our cpu_set_upcall in vm_machdep.c to set
> the return address for the thread entry point to NULL (#8 in the backtrace
> above).  For new stacks this is implicitly NULL, but "Thread 7" (as gdb
> calls it) uses a recycled stack and libthr stores a 'struct stack' at the
> end of such stacks (to keep them in a linked list).  I'm still looking at
> how base libgcc_s which uses LLVM libunwind avoids this problem.

So both GCC and LLVM unwinding look up the return address in the CFI
table and fail when the return address is garbage, but LLVM treats this
as an end-of-stack condition while GCC further tries to see if the
return address points to a signal trampoline by testing the instruction
bytes at that address.  On amd64 the garbage address is unreadable so it
segfaults.  On i386 it is readable, the test fails and GCC returns
end-of-stack.

To fix the crash and get predictable behaviour in the other cases I
propose always setting the return address to 0.  The attached patch does
this for i386 and amd64.  I don't know if other architectures need a
similar patch.Index: sys/amd64/amd64/vm_machdep.c
===
--- sys/amd64/amd64/vm_machdep.c	(revision 322802)
+++ sys/amd64/amd64/vm_machdep.c	(working copy)
@@ -507,6 +507,9 @@ cpu_set_upcall(struct thread *td, void (*entry)(void *
 		   (((uintptr_t)stack->ss_sp + stack->ss_size - 4) & ~0x0f) - 4;
 		td->td_frame->tf_rip = (uintptr_t)entry;
 
+		/* Sentinel return address to stop stack unwinding. */
+		suword32((void *)td->td_frame->tf_rsp, 0);
+
 		/* Pass the argument to the entry point. */
 		suword32((void *)(td->td_frame->tf_rsp + sizeof(int32_t)),
 		(uint32_t)(uintptr_t)arg);
@@ -529,6 +532,9 @@ cpu_set_upcall(struct thread *td, void (*entry)(void *
 	td->td_frame->tf_fs = _ufssel;
 	td->td_frame->tf_gs = _ugssel;
 	td->td_frame->tf_flags = TF_HASSEGS;
+
+	/* Sentinel return address to stop stack unwinding. 

Re: Segfault in _Unwind_* code called from pthread_exit

[Tijl Coosemans]

On Thu, 24 Aug 2017 18:42:35 +0300 Konstantin Belousov  
wrote:
> On Wed, Aug 23, 2017 at 04:37:07PM +0200, Tijl Coosemans wrote:
>> The following program segfaults for me on amd64 when linked like this:
>> 
>> cc -o test test.c -lpthread -L/usr/local/lib/gcc5 -lgcc_s -rpath 
>> /usr/local/lib/gcc5
>> 
>> 
>> #include 
>> #include 
>> 
>> void *
>> thr( void *arg ) {
>>  return( NULL );
>> }
>> 
>> int
>> main( void ) {
>>  pthread_t thread;
>> 
>>  for( int i = 1; i < 20; i++ ) {
>>  fprintf( stderr, "%d\n", i );
>>  pthread_create( , NULL, thr, NULL );
>>  pthread_join( thread, NULL );
>>  }
>>  return( 0 );
>> }
>> 
>> 
>> The backtrace looks like this:
>> 
>> Thread 7 received signal SIGSEGV, Segmentation fault.
>> [Switching to LWP 100511 of process 1886]
>> uw_frame_state_for (context=context@entry=0x7fffdfffddc0, 
>> fs=fs@entry=0x7fffdfffdb10)
>> at /usr/ports/lang/gcc5/work/gcc-5.4.0/libgcc/unwind-dw2.c:1249
>> 1249 /usr/ports/lang/gcc5/work/gcc-5.4.0/libgcc/unwind-dw2.c: No such file 
>> or directory.
>> (gdb) bt
>> #0  uw_frame_state_for (context=context@entry=0x7fffdfffddc0, 
>> fs=fs@entry=0x7fffdfffdb10)
>> at /usr/ports/lang/gcc5/work/gcc-5.4.0/libgcc/unwind-dw2.c:1249
>> #1  0x000800a66ecb in _Unwind_ForcedUnwind_Phase2 (
>> exc=exc@entry=0x800658730, context=context@entry=0x7fffdfffddc0)
>> at /usr/ports/lang/gcc5/work/gcc-5.4.0/libgcc/unwind.inc:155
>> #2  0x000800a67200 in _Unwind_ForcedUnwind (exc=0x800658730, 
>> stop=0x8008428b0 , stop_argument=0x0)
>> at /usr/ports/lang/gcc5/work/gcc-5.4.0/libgcc/unwind.inc:207
>> #3  0x000800842224 in _Unwind_ForcedUnwind (ex=0x800658730, 
>> stop_func=0x8008428b0 , stop_arg=0x0)
>> at /usr/src/lib/libthr/thread/thr_exit.c:106
>> #4  0x00080084269f in thread_unwind ()
>> at /usr/src/lib/libthr/thread/thr_exit.c:172
>> #5  0x0008008424d6 in _pthread_exit_mask (status=0x0, mask=0x0)
>> at /usr/src/lib/libthr/thread/thr_exit.c:254
>> #6  0x000800842359 in _pthread_exit (status=0x0)
>> at /usr/src/lib/libthr/thread/thr_exit.c:206
>> #7  0x00080082ccb1 in thread_start (curthread=0x800658500)
>> at /usr/src/lib/libthr/thread/thr_create.c:289
>> #8  0x7fffdfdfe000 in ?? ()
>> Backtrace stopped: Cannot access memory at address 0x7fffdfffe000
>> 
>> 
>> It happens with gcc6 as well, but not with base libgcc_s.
>> Can anyone reproduce this?  Have there been any changes to stack
>> unwinding recently (last few months)?  
> 
> I can reproduce this, and there was a change in gcc unwinder, it seems.
> Below is a patch which I did not even compiled.  Still, it should give
> an idea how it might be approached.  The patch is against gcc head.

Currently I'm thinking to patch our cpu_set_upcall in vm_machdep.c to set
the return address for the thread entry point to NULL (#8 in the backtrace
above).  For new stacks this is implicitly NULL, but "Thread 7" (as gdb
calls it) uses a recycled stack and libthr stores a 'struct stack' at the
end of such stacks (to keep them in a linked list).  I'm still looking at
how base libgcc_s which uses LLVM libunwind avoids this problem.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Konstantin Belousov]

On Wed, Aug 23, 2017 at 04:37:07PM +0200, Tijl Coosemans wrote:
> Hi,
> 
> The following program segfaults for me on amd64 when linked like this:
> 
> cc -o test test.c -lpthread -L/usr/local/lib/gcc5 -lgcc_s -rpath 
> /usr/local/lib/gcc5
> 
> 
> #include 
> #include 
> 
> void *
> thr( void *arg ) {
>   return( NULL );
> }
> 
> int
> main( void ) {
>   pthread_t thread;
> 
>   for( int i = 1; i < 20; i++ ) {
>   fprintf( stderr, "%d\n", i );
>   pthread_create( , NULL, thr, NULL );
>   pthread_join( thread, NULL );
>   }
>   return( 0 );
> }
> 
> 
> The backtrace looks like this:
> 
> Thread 7 received signal SIGSEGV, Segmentation fault.
> [Switching to LWP 100511 of process 1886]
> uw_frame_state_for (context=context@entry=0x7fffdfffddc0, 
> fs=fs@entry=0x7fffdfffdb10)
> at /usr/ports/lang/gcc5/work/gcc-5.4.0/libgcc/unwind-dw2.c:1249
> 1249  /usr/ports/lang/gcc5/work/gcc-5.4.0/libgcc/unwind-dw2.c: No such file 
> or directory.
> (gdb) bt
> #0  uw_frame_state_for (context=context@entry=0x7fffdfffddc0, 
> fs=fs@entry=0x7fffdfffdb10)
> at /usr/ports/lang/gcc5/work/gcc-5.4.0/libgcc/unwind-dw2.c:1249
> #1  0x000800a66ecb in _Unwind_ForcedUnwind_Phase2 (
> exc=exc@entry=0x800658730, context=context@entry=0x7fffdfffddc0)
> at /usr/ports/lang/gcc5/work/gcc-5.4.0/libgcc/unwind.inc:155
> #2  0x000800a67200 in _Unwind_ForcedUnwind (exc=0x800658730, 
> stop=0x8008428b0 , stop_argument=0x0)
> at /usr/ports/lang/gcc5/work/gcc-5.4.0/libgcc/unwind.inc:207
> #3  0x000800842224 in _Unwind_ForcedUnwind (ex=0x800658730, 
> stop_func=0x8008428b0 , stop_arg=0x0)
> at /usr/src/lib/libthr/thread/thr_exit.c:106
> #4  0x00080084269f in thread_unwind ()
> at /usr/src/lib/libthr/thread/thr_exit.c:172
> #5  0x0008008424d6 in _pthread_exit_mask (status=0x0, mask=0x0)
> at /usr/src/lib/libthr/thread/thr_exit.c:254
> #6  0x000800842359 in _pthread_exit (status=0x0)
> at /usr/src/lib/libthr/thread/thr_exit.c:206
> #7  0x00080082ccb1 in thread_start (curthread=0x800658500)
> at /usr/src/lib/libthr/thread/thr_create.c:289
> #8  0x7fffdfdfe000 in ?? ()
> Backtrace stopped: Cannot access memory at address 0x7fffdfffe000
> 
> 
> It happens with gcc6 as well, but not with base libgcc_s.
> Can anyone reproduce this?  Have there been any changes to stack
> unwinding recently (last few months)?

I can reproduce this, and there was a change in gcc unwinder, it seems.
Below is a patch which I did not even compiled.  Still, it should give
an idea how it might be approached.  The patch is against gcc head.

Index: libgcc/config/i386/freebsd-unwind.h
===
--- libgcc/config/i386/freebsd-unwind.h (revision 251293)
+++ libgcc/config/i386/freebsd-unwind.h (working copy)
@@ -28,6 +28,8 @@ see the files COPYING3 and COPYING.RUNTIME respect
 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 
@@ -42,7 +44,29 @@ x86_64_freebsd_fallback_frame_state
 {
   struct sigframe *sf;
   long new_cfa;
+#ifdef KERN_PROC_SIGTRAMP
+  static long sigtramp_addr = 0;
 
+  if (sigtramp_addr == 0) {
+struct kinfo_sigtramp kst;
+int error, mib[4];
+size_t len;
+
+mib[0] = CTL_KERN;
+mib[1] = KERN_PROC;
+mib[2] = KERN_PROC_SIGTRAMP;
+mib[3] = getpid();
+len = sizeof(kst);
+error = sysctl(mib, sizeof(mib) / sizeof(mib[0]), , , NULL, 0);
+if (error == 0)
+  sigtramp_addr = kst.ksigtramp_start;
+  }
+
+  if (sigtramp_addr != 0 && (uintptr_t)(context->ra) == sigtramp_addr)
+;
+  else
+#endif
+
   /* Prior to FreeBSD 9, the signal trampoline was located immediately
  before the ps_strings.  To support non-executable stacks on AMD64,
  the sigtramp was moved to a shared page for FreeBSD 9.  Unfortunately
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: Segfault in _Unwind_* code called from pthread_exit

[Mark Millard]

Tijl Coosemans tijl at FreeBSD.org wrote on
Wed Aug 23 14:38:27 UTC 2017 :

> The following program segfaults for me on amd64 when linked like this:
> 
> cc -o test test.c -lpthread -L/usr/local/lib/gcc5 -lgcc_s -rpath 
> /usr/local/lib/gcc5
> 
> 
> #include 
> #include 
> 
> void *
> thr( void *arg ) {
>   return( NULL );
> }
> 
> int
> main( void ) {
>   pthread_t thread;
> 
>   for( int i = 1; i < 20; i++ ) {
>   fprintf( stderr, "%d\n", i );
>   pthread_create( , NULL, thr, NULL );
>   pthread_join( thread, NULL );
>   }
>   return( 0 );
> }
> 
> 
> The backtrace looks like this:
> 
> Thread 7 received signal SIGSEGV, Segmentation fault.
> [Switching to LWP 100511 of process 1886]
> uw_frame_state_for (context=
> context at entry
> =0x7fffdfffddc0, 
> fs=
> fs at entry
> =0x7fffdfffdb10)
> at /usr/ports/lang/gcc5/work/gcc-5.4.0/libgcc/unwind-dw2.c:1249
> 1249  /usr/ports/lang/gcc5/work/gcc-5.4.0/libgcc/unwind-dw2.c: No such file 
> or directory.
> (gdb) bt
> #0  uw_frame_state_for (context=
> context at entry
> =0x7fffdfffddc0, 
> fs=
> fs at entry
> =0x7fffdfffdb10)
> at /usr/ports/lang/gcc5/work/gcc-5.4.0/libgcc/unwind-dw2.c:1249
> #1  0x000800a66ecb in _Unwind_ForcedUnwind_Phase2 (
> exc=
> exc at entry=0x800658730, context=context at entry
> =0x7fffdfffddc0)
> at /usr/ports/lang/gcc5/work/gcc-5.4.0/libgcc/unwind.inc:155
> #2  0x000800a67200 in _Unwind_ForcedUnwind (exc=0x800658730, 
> stop=0x8008428b0 , stop_argument=0x0)
> at /usr/ports/lang/gcc5/work/gcc-5.4.0/libgcc/unwind.inc:207
> #3  0x000800842224 in _Unwind_ForcedUnwind (ex=0x800658730, 
> stop_func=0x8008428b0 , stop_arg=0x0)
> at /usr/src/lib/libthr/thread/thr_exit.c:106
> #4  0x00080084269f in thread_unwind ()
> at /usr/src/lib/libthr/thread/thr_exit.c:172
> #5  0x0008008424d6 in _pthread_exit_mask (status=0x0, mask=0x0)
> at /usr/src/lib/libthr/thread/thr_exit.c:254
> #6  0x000800842359 in _pthread_exit (status=0x0)
> at /usr/src/lib/libthr/thread/thr_exit.c:206
> #7  0x00080082ccb1 in thread_start (curthread=0x800658500)
> at /usr/src/lib/libthr/thread/thr_create.c:289
> #8  0x7fffdfdfe000 in ?? ()
> Backtrace stopped: Cannot access memory at address 0x7fffdfffe000
> 
> 
> It happens with gcc6 as well, but not with base libgcc_s.
> Can anyone reproduce this?  Have there been any changes to stack
> unwinding recently (last few months)?

This example might make a good addition to bugzilla 221288 that
has some material from a more complicated example of problems
mixing /usr/local/lib/gcc7/libgcc_s.so.1 and pthread. (Threading
need not be the only problem context.)

Here the source code is nice and short where the C++ example was
large enough that I did not bother to submit it and I've not made
a smaller example.

The bigger C++ example had:

# ldd a.out
a.out:
libstdc++.so.6 => /usr/local/lib/gcc7/libstdc++.so.6 (0x800844000)
libm.so.5 => /lib/libm.so.5 (0x800bd8000)
libgcc_s.so.1 => /usr/local/lib/gcc7/libgcc_s.so.1 (0x800e05000)
libthr.so.3 => /lib/libthr.so.3 (0x80101c000)
libc.so.7 => /lib/libc.so.7 (0x801244000)

# ./a.out
. . . (omitted) . . .
Segmentation fault (core dumped)

It was the -Wl,-rpath=/usr/local/lib/gcc7 that forced the
gcc7 variant of libgcc_s to be used. Any combination that
had /lib/libthr.so.3 mixed with /usr/local/lib/gcc7/libgcc_s.so.1
failed. Any combination that had /lib/libthr.so.3 mixed with
/lib/libgcc_s.so.1 worked.

Of course /lib/libthr.so.3 was built based on /lib/libgcc_s.so.1 .


===
Mark Millard
markmi at dsl-only.net

___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"