Re: Correction to crash inside RtlCaptureStackBackTrace() + test case

[André Hentschel]

Am 23.01.2011 18:20, schrieb Janne Hakonen:
> Hi,
>
> This is my first attempt to post patches to Wine and it would be great if you could review these.
>
> First patch (0001-2-2-ntdll-Added-try-except-blocks-to-RtlCaptureStackBa.txt) is a correction to a crash situation where application crashes if RtlCaptureStackBackTrace() is called and one of the frame pointers of call stack is invalid, pointing usually to 0xffffffff, sometimes to other invalid locations.
> The correction wraps the offending code inside __try � __except blocks so that a resulting access violation is caught before it can crash the calling application.
>
> Second patch (0002-1-2-ntdll-tests-Added-a-test-for-RtlCaptureStackBackTr.txt) is a test case for the correction.
> It changes one of the frame pointers to invalid value and then calls RtlCaptureStackBackTrace(). The test fails on Wine�s HEAD (without the first patch), but passes successfully on Windows 7. Haven�t tested this on other OSes. I created a new signal.c file for the test case under ntdll/tests since I couldn�t find more fitting place.
>
> This crash can be reproduced with Anarchy Online game when trying to open an in-game browser. Also reproducible with AwesomiumGL sample application from Awesomium�s SDK. The crash (among other problems) is reported to bugzilla at: http://bugs.winehq.org/show_bug.cgi?id=25206.
>
> Best Regards,
> Janne Hakonen

Hi and welcome to wine development!

First i see is that your mail application wrapped the patch ( that is when long likes brake down into the next line ).
Then you have a whitespace issue in the makefile i think...
further you should consider using IsBadReadPtr instead of try...except, but i'm not sure if that would work.

If you solved the issues, you can send the patch to [e-mail] and can track the status at source.winehq.org/patches

--

Best Regards, André Hentschel

Correction to crash inside RtlCaptureStackBackTrace() + test case

[Vitaliy Margolen]

On 01/23/2011 10:54 AM, André Hentschel wrote:
> further you should consider using IsBadReadPtr instead of try...except, but i'm not sure if that would work.

No, one should never ever use those functions. Even MS says they ave evil. 
Try .. except is the right thing to use here.

On 01/23/2011 10:20 AM, Janne Hakonen wrote:
> I created a new signal.c file for the test case under ntdll/tests since I couldn’t find more fitting place.

Not a good idea. Please put the test into rtl.c
> +static void test_pRtlCaptureStackBackTrace(void)
> +{
> - ok(TRUE == get_back_trace_with_invalid_frame_pointer(),"get_back_trace_with_invalid_frame_pointer() failed\n" );
>
> +}

Please put ok() check inside get_back_trace_with_invalid_frame_pointer().
> +static void capture_stack_back_trace(void)
> +{
> - void** callers = (void**) calloc(maxCallers, sizeof(void*));
> - free(callers);
>
> +}

Please use HeapAlloc/HeapFree. Don't cast void pointers. Here you don't need 
to allocate anything, just use a stack variable.

Also you not checking the return value from RtlCaptureStackBackTrace(). 
Please do.

Vitaliy.