Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash after winpthread update #8094

Closed
omichel opened this issue Mar 9, 2021 · 42 comments · Fixed by #8584
Closed

Crash after winpthread update #8094

omichel opened this issue Mar 9, 2021 · 42 comments · Fixed by #8584

Comments

@omichel
Copy link
Contributor

omichel commented Mar 9, 2021

Since the update to mingw-w64-x86_64-libwinpthread-git-9.0.0.6128.07922837-1 / mingw-w64-x86_64-winpthreads-git-9.0.0.6128.07922837-1, my app is crashing with the following gdb stack:

Thread 43 received signal ?, Unknown signal.
[Switching to Thread 2160.0x21f0]
0x00007ffea5b31f46 in ntdll!RtlRaiseStatus ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
(gdb) #0  0x00007ffea5b31f46 in ntdll!RtlRaiseStatus ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
#1  0x00007ffea5ae0052 in ntdll!memset () from C:\WINDOWS\SYSTEM32\ntdll.dll
#2  0x00007ffea55a2eed in msvcrt!_setjmpex ()
   from C:\WINDOWS\System32\msvcrt.dll
#3  0x00007ffe9d8a5c31 in ?? () from D:\msys64\mingw64\bin\libwinpthread-1.dll
#4  0x00007ffe6cb88330 in ode!_ZN22dxThreadManagerPthread12simLoopChildEPv ()
   from D:\msys64\home\Olivier\webots\msys64\mingw64\bin\ode.dll
#5  0x00007ffe9d8a4f35 in ?? () from D:\msys64\mingw64\bin\libwinpthread-1.dll
#6  0x00007ffea556af5a in msvcrt!_beginthreadex ()
   from C:\WINDOWS\System32\msvcrt.dll
#7  0x00007ffea556b02c in msvcrt!_endthreadex ()
   from C:\WINDOWS\System32\msvcrt.dll
#8  0x00007ffea43b6fd4 in KERNEL32!BaseThreadInitThunk ()
   from C:\WINDOWS\System32\kernel32.dll
#9  0x00007ffea5a7cec1 in ntdll!RtlUserThreadStart ()
   from C:\WINDOWS\SYSTEM32\ntdll.dll
#10 0x0000000000000000 in ?? ()
Backtrace stopped: previous frame inner to this frame (corrupt stack?)
(gdb) A debugging session is active.

        Inferior 1 [process 2160] will be killed.
@omichel omichel changed the title Crash with the winpthread update Crash after winpthread update Mar 9, 2021
@lazka
Copy link
Member

lazka commented Mar 9, 2021

@omichel
Copy link
Contributor Author

omichel commented Mar 9, 2021

Yes, I confirm that reverting to this previous version does fix the crash.

@Biswa96
Copy link
Member

Biswa96 commented Mar 9, 2021

Any minimal sample code to reproduce this issue? If I see the git history winpthread was not changed between those two commits.

@omichel
Copy link
Contributor Author

omichel commented Mar 9, 2021

Unfortunately, the crash is inside a quite large library and the gdb stack is not very helpful, so, it doesn't seem easy to develop a minimal sample code that reproduces this crash...

@omichel
Copy link
Contributor Author

omichel commented Mar 9, 2021

If I see the git history winpthread was not changed between those two commits.

Yes, the two versions of libwinpthread-1.dll have exactly the same file size, however their content differ according to diff. It's quite odd...

@lhmouse
Copy link
Contributor

lhmouse commented Mar 9, 2021

The only change in that interval seems this one: mingw-w64/mingw-w64@84c950b

@Biswa96
Copy link
Member

Biswa96 commented Mar 9, 2021

That's before ea40a87a commit.

@lazka
Copy link
Member

lazka commented Mar 9, 2021

@lhmouse
Copy link
Contributor

lhmouse commented Mar 9, 2021

That commit affects i686-w64-mingw32 only. __cdecl, __stdcall and __fastcall conventions have no effect on x86_64-w64-mingw32.

@lazka
Copy link
Member

lazka commented Mar 9, 2021

yeah, I'm going to revert all those later

edit: done in d08f61e

@omichel
Copy link
Contributor Author

omichel commented Mar 9, 2021

I am happy to test any intermediate version if that helps.

lazka added a commit that referenced this issue Mar 9, 2021
@revelator
Copy link
Contributor

So the backports were useless ? ouch guess i got fooled as well.

@jeremyd2019
Copy link
Member

Not useless. The specific commit mentioned only affected i686. However the other commit(s) addressed an issue with registers saved on the stack for SEH getting corrupted, as I understand it. Apparently it had some adverse impacts on setjmp.

@lazka
Copy link
Member

lazka commented Mar 9, 2021

what about now?

@revelator
Copy link
Contributor

aye i was seing a lot of segfaults and pointed lazka at the last backport which atleast fixed the segfaults for me but i guess it only went that well for me because i use an sjlj build.

@revelator
Copy link
Contributor

Im actually using the build with all 3 backport patches and no problems here so far.

@jeremyd2019
Copy link
Member

was winpthread built with the last backport, or before that was applied? It could be that it would have benefitted from that fix also

@revelator
Copy link
Contributor

revelator commented Mar 9, 2021

i built it after all 3 patches where applied no problems so far with winpthreads.

but i guess you ment to hear from the bug reporter ?

@omichel
Copy link
Contributor Author

omichel commented Mar 10, 2021

The issue has been fixed. I can compile and run correctly again after new updates from MSYS2.

@omichel omichel closed this as completed Mar 10, 2021
@lazka
Copy link
Member

lazka commented Mar 10, 2021

was winpthread built with the last backport, or before that was applied? It could be that it would have benefitted from that fix also

It was build without the last backport (you can look at the .BUILDINFO file in each package for which versions were current at that time). Is it worth re-trying, or should we just wait for an official release?

Anyway, thanks for testing @omichel, glad we could pin down the cause.

@revelator
Copy link
Contributor

aye i was wondering to since it works here with all 3 patches applied :)

@lhmouse
Copy link
Contributor

lhmouse commented Mar 10, 2021

There is a call to RaiseException() inside pthread_setname_np() in winpthreads. Maybe it was the cause?

@revelator
Copy link
Contributor

well what we need to know is if you updated mingw-w64-gcc in the last days before you built winpthread as the last patch was just recently added and fixed a segfault caused by the other backports :)

@revelator
Copy link
Contributor

the patch in question is this one 0202-backport-357c435.patch it was added this week so it is rather recent.

@revelator
Copy link
Contributor

guess we will have to do a ci build and then check it out ourself, not sure but would RaiseException() not also crash on a build with sjlj exceptions ?.

@lhmouse
Copy link
Contributor

lhmouse commented Mar 11, 2021 via email

@revelator
Copy link
Contributor

hmm guess at most we can do some testing to see if it also happens with all 3 patches in place then.

@revelator
Copy link
Contributor

@jeremyd2019
Copy link
Member

and the particular case of setting thread name using RaiseException is a 'hack' to communicate with an attached debugger. it shouldn't be invoked if there is not a debugger attached, and there's a vectored exception handler registered to catch it just in case.

mingwandroid pushed a commit to mingwandroid/MINGW-packages that referenced this issue Apr 8, 2021
@lazka
Copy link
Member

lazka commented Apr 11, 2021

Was there any way to reproduce this? The gcc 10.3 update is pending which contains one of the reverted commits and I'd like to check if that problem still exists.

@omichel
Copy link
Contributor Author

omichel commented Apr 11, 2021

You can either compile Webots from the sources or replace to libwinpthread-1.dll from a recent nightly build. Webots should crash at start-up if the bug is still there in libwinpthread-1.dll.

@lazka
Copy link
Member

lazka commented Apr 11, 2021

ok, thanks!

@Astrum-polaris
Copy link
Contributor

So I rebuilt winpthread 9.0.0.6158 with gcc 10.3.0 and tested Webots nightly, it crash at start-up just as described

Will revert the commit in 0202-backport-357c435.patch to see if the problem persists.

@Astrum-polaris
Copy link
Contributor

Astrum-polaris commented Apr 12, 2021

Here's my test results

Revert 0202-backport-357c435.patch only → still crash
Revert 0201-backport-49a714e.patch and 0202-backport-357c435.patch → success

Edit: Realized 0202 is a fix on top of 0201 and previous comments suggest that when the initial crash happens, 0202 is not applied, so the first testcase is useless.

Next I will try to revert 0201 only and see if that works
on second thought, keeping 0202 without 0201 does not make much sense

@lazka lazka reopened this Apr 12, 2021
lazka pushed a commit to lazka/MINGW-packages that referenced this issue May 2, 2021
@ssbssa
Copy link

ssbssa commented May 4, 2021

Since this is a crash in _setjmpex, I think it's the same bug as (or related to) gcc PR100402.

@lazka
Copy link
Member

lazka commented May 4, 2021

Thanks. Good to have a minimal reproducer at least.

@ssbssa
Copy link

ssbssa commented May 5, 2021

A tentative fix was posted in the gcc bug, maybe someone can check if it solves this problem here as well.

@lazka
Copy link
Member

lazka commented May 5, 2021

Thanks. I'll try to test the patch.

@lazka
Copy link
Member

lazka commented May 6, 2021

@omichel I have three variants of libwinpthread-1.dll: mingw-w64-winpthreads-tests.zip Could you give them a quick run? I kinda failed to figure out where the Windows version is for your nightly build.

@omichel
Copy link
Contributor Author

omichel commented May 6, 2021

Thank you. I just tested and here are the results:

  • gcc-10.3-plain → crash
  • gcc-10.3-with-new-fix → OK
  • gcc-10.3-with-reverts → OK

@lazka
Copy link
Member

lazka commented May 6, 2021

perfect, that confirms the cause and the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants