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 wine-patches(a)winehq.org 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.