Sebastian Lackner sebastian@fds-team.de writes:
Signed-off-by: Sebastian Lackner sebastian@fds-team.de
When applications switch to a custom stack, exit_frame is not necessarily valid anymore.
How does it become invalid? Does the app unmap the previous stack?
On 18.04.2016 12:27, Alexandre Julliard wrote:
Sebastian Lackner sebastian@fds-team.de writes:
Signed-off-by: Sebastian Lackner sebastian@fds-team.de
When applications switch to a custom stack, exit_frame is not necessarily valid anymore.
How does it become invalid? Does the app unmap the previous stack?
Yes, MSYS2/Cygwin will switch to its own stack and deallocate the old one. This patch alone is not sufficient because there are still a couple of other references to the old stack (like the debug_info struct in start_thread).
If preferred, this issue can also be solved differently by calculting exit_frame dynamically based on StackBase (after a couple of other bugs are fixed). Disadvantage is that if applications mess around with StackBase, I'm not sure if we should trust that the new stack is set up properly.
Sebastian Lackner sebastian@fds-team.de writes:
On 18.04.2016 12:27, Alexandre Julliard wrote:
Sebastian Lackner sebastian@fds-team.de writes:
Signed-off-by: Sebastian Lackner sebastian@fds-team.de
When applications switch to a custom stack, exit_frame is not necessarily valid anymore.
How does it become invalid? Does the app unmap the previous stack?
Yes, MSYS2/Cygwin will switch to its own stack and deallocate the old one. This patch alone is not sufficient because there are still a couple of other references to the old stack (like the debug_info struct in start_thread).
If preferred, this issue can also be solved differently by calculting exit_frame dynamically based on StackBase (after a couple of other bugs are fixed). Disadvantage is that if applications mess around with StackBase, I'm not sure if we should trust that the new stack is set up properly.
The reason for exit_frame is to prevent pthread_exit from trying to unwind the stack. I'm wondering how that will work if it was deallocated. Did you look into this? Does pthread detect that we are on a different stack? And doesn't pthread also put its own structures on the stack?
On 18.04.2016 12:53, Alexandre Julliard wrote:
Yes, MSYS2/Cygwin will switch to its own stack and deallocate the old one. This patch alone is not sufficient because there are still a couple of other references to the old stack (like the debug_info struct in start_thread).
If preferred, this issue can also be solved differently by calculting exit_frame dynamically based on StackBase (after a couple of other bugs are fixed). Disadvantage is that if applications mess around with StackBase, I'm not sure if we should trust that the new stack is set up properly.
The reason for exit_frame is to prevent pthread_exit from trying to unwind the stack. I'm wondering how that will work if it was deallocated. Did you look into this? Does pthread detect that we are on a different stack? And doesn't pthread also put its own structures on the stack?
Some versions of pthread indeed put internal structures at the top of the stack. To solve that, the current Staging patchset allocates threads with a dummy pthread, and then switches to the real stack, see: https://github.com/wine-compholio/wine-staging/blob/master/patches/ntdll-Thr... Unless there is something weird going on, thanks to the CFI annotations in wine_switch_to_stack() pthread should be able to unwind the stack correctly despite the stack switch inbetween. At least I have not found any problems so far ;)
You are right that ignoring exit_frame might (under specific circumstances) run language handlers, but I'm not yet aware of a better way to deal with this. The only alternative would be to calculate it based on StackBase, but its also far from perfect.
Sebastian Lackner sebastian@fds-team.de writes:
Some versions of pthread indeed put internal structures at the top of the stack. To solve that, the current Staging patchset allocates threads with a dummy pthread, and then switches to the real stack, see: https://github.com/wine-compholio/wine-staging/blob/master/patches/ntdll-Thr... Unless there is something weird going on, thanks to the CFI annotations in wine_switch_to_stack() pthread should be able to unwind the stack correctly despite the stack switch inbetween. At least I have not found any problems so far ;)
For this specific stack switch, sure, but it can't unwind across Win32 code, which was the reason for the exit frame in the first place.
You are right that ignoring exit_frame might (under specific circumstances) run language handlers, but I'm not yet aware of a better way to deal with this. The only alternative would be to calculate it based on StackBase, but its also far from perfect.
I'd suggest to instead try to keep the initial stack around.
On 19.04.2016 07:11, Alexandre Julliard wrote:
Sebastian Lackner sebastian@fds-team.de writes:
Some versions of pthread indeed put internal structures at the top of the stack. To solve that, the current Staging patchset allocates threads with a dummy pthread, and then switches to the real stack, see: https://github.com/wine-compholio/wine-staging/blob/master/patches/ntdll-Thr... Unless there is something weird going on, thanks to the CFI annotations in wine_switch_to_stack() pthread should be able to unwind the stack correctly despite the stack switch inbetween. At least I have not found any problems so far ;)
For this specific stack switch, sure, but it can't unwind across Win32 code, which was the reason for the exit frame in the first place.
You are right that ignoring exit_frame might (under specific circumstances) run language handlers, but I'm not yet aware of a better way to deal with this. The only alternative would be to calculate it based on StackBase, but its also far from perfect.
I'd suggest to instead try to keep the initial stack around.
This was one of my initial approaches, but first of all its very hacky, and moreover that alone is not sufficient. The 64-bit version of MSYS2 will try to reuse the existing stack at one point, and memset()s a specific amount of memory at the top for its internal use as thread local storage. Depending on the exact memory layout, this might corrupt pthread or wine internal structures like debug_info... ;)
Sebastian Lackner sebastian@fds-team.de writes:
This was one of my initial approaches, but first of all its very hacky, and moreover that alone is not sufficient. The 64-bit version of MSYS2 will try to reuse the existing stack at one point, and memset()s a specific amount of memory at the top for its internal use as thread local storage. Depending on the exact memory layout, this might corrupt pthread or wine internal structures like debug_info... ;)
Couldn't we ask them to stop doing such things?
-
Alexandre Julliard -
Sebastian Lackner