[PATCH 2/7] msvcrt: Clean up registered C++ object in handler.
Piotr Caban
piotr.caban at gmail.com
Wed May 31 04:48:44 CDT 2017
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
More information about the wine-devel
mailing list