[prev in list] [next in list] [prev in thread] [next in thread]
List: wine-devel
Subject: Re: [PATCH 2/7] msvcrt: Clean up registered C++ object in handler.
From: Piotr Caban <piotr.caban () gmail ! com>
Date: 2017-05-31 9:48:44
Message-ID: 42c616a7-ec6b-cab9-c270-10f342b39c18 () gmail ! com
[Download RAW message or body]
On 05/31/17 00:59, Daniel Lehman wrote:
>
> On 05/24/17 02:55, Daniel Lehman wrote:
> > +static DWORD cxx_catch_cleanup(EXCEPTION_RECORD *rec, \
> > EXCEPTION_REGISTRATION_RECORD *frame, + CONTEXT \
> > *context, +EXCEPTION_REGISTRATION_RECORD **pdispatcher) {
> > + if (rec->ExceptionFlags & (EH_UNWINDING | EH_EXIT_UNWIND))
> > + {
> > + thread_data_t *data = msvcrt_get_thread_data();
> > + frame_info *cur;
> > +
> > + if (cxx_is_consolidate(rec))
> > Is this condition really needed? Shouldn't we clean the object no matter what's \
> > the reason of unwind?
>
> Yeah. That's covered by patch 7/7. The original code only cleaned up if \
> consolidating.
> Since this 2/7 patch was somewhat of a refactoring for the later patches, I kept \
> the consolidate-only cleanup logic. I can merge them if you want
Yes, it would be preferable to merge the patches.
> > > + if ((ULONG64)cur <= (ULONG64)frame)
> > This condition is not working. It's making assumption about order of catch_frame \
> > and frame_info variables on stack while they are declared this way:
>
> I see what you mean. If I forcefully reverse them, my tests crash; my version of \
> gcc was always putting them in the same place on the stack, regardless of where \
> they were declared
> > > + EXCEPTION_REGISTRATION_RECORD catch_frame;
> > > cxx_frame_info frame_info;
> > Shouldn't the cxx_catch_cleanup just unregister the object that was registered in \
> > call_catch_block >
> I didn't find a way to be call __CxxUnregisterObject on that one specifically (I'll \
> try suggestions if you got em)
I think that the easiest solution would be to define something like:
struct catch_cleanup_frame {
EXCEPTION_REGISTRATION_RECORD frame;
cxx_frame_info frame_info;
}
and just call __CxxUnregisterObject with passed
catch_cleanup_frame->frame_info in handler.
> > Here's a test case that demonstrate the problem with cur <= frame
> > comparison:
> >
> > try {
> > try { int *p = NULL; *p = 0x42; }
> > catch (klass x) { throw 1; }
> > } catch (int i) { }
> >
> > try { throw 1; }
> > catch(...) {}
>
> This crashes for me even on Windows because the SEGV is uncaught. It 'works' if I \
> set an seh translator that throws int, but I get identical results on Wine with my \
> series applied. Do I need to add something?
It was meant to be run with seh translator set. It might be
compiler/optimization level specific. On my computer it leads to
incorrect stack usage caused by "cur <= frame" condition. The
__CxxUnregisterObject is not called in my case and the registered
objects list points to invalid stack space.
Thanks,
Piotr
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic