Bug 87183 – Web Inspector: CodeGeneratorInspector.py: protect typed API from C++ implicit float to int cast

• Component: Web Inspector
• Status: RESOLVED WONTFIX

[Peter Rybin]

C++ implicit cast from float or double to int creates a breach in inspector typed API.
For example one can mistakenly have protocol attribute type "int" while actual value type in C++ is "float". The value will be silently truncated to integer without a warning.
Make sure that passing float or double to inspector typed API won't compile.

[Peter Rybin]

Created attachment 143419 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 143419 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=143419&action=review

> Source/WebCore/inspector/CodeGeneratorInspector.py:2240
> +// A small trancient wrapper around int type, that can be used as a funciton parameter type

typo: transient

> Source/WebCore/inspector/CodeGeneratorInspector.py:2241
> +// cleverly diallowing C++ implicit casts from float or double.

typo: disallowing

[Andrey Kosyakov]

Comment on attachment 143419 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=143419&action=review

I don't like the way we black-list types that should not be converted. How about more generic approach, e.g.

emplate<typename T1, typename T2> struct no_implicit_cast {
};

template<typename T> struct no_implicit_cast<T, T> {
    static void assign(T& l, const T& r) { l = r; }
};

template<typename T> class exact_type {
private:
    T value;

public:
    template<typename T2> exact_type(T2 v) { no_implicit_cast<T, T2>::assign(value, v); }
    operator T() const { return value; }
};

> Source/WebCore/inspector/CodeGeneratorInspector.py:2262
> +template<> class ExactlyIntHelper<float> { /* no cast method for float */ };
> +template<> class ExactlyIntHelper<double> { /* no cast method for double */ };

So why just float and double? What about char?

[Yury Semikhatsky]

Comment on attachment 143419 [details]
Patch

r- until Andrey's comments are addressed.

[Peter Rybin]

(In reply to comment #3)
> (From update of attachment 143419 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=143419&action=review
> 
> I don't like the way we black-list types that should not be converted. How about more generic approach, e.g.

Can you whitelist enum to int cast this way? This is a quite popular case.
Plus unsigned to int cast and similar ones should be possible.

[Peter Rybin]

Yurys,
we have a general question here. Current approach filters out only floats and doubles.
Andrey suggests more consistent solution, that would also disallow enums and longs. That would require explicit static_cast<> in several places throughout our code. I found this a bit unrequested effect.
What would you say?

[Yury Semikhatsky]

(In reply to comment #6)
> Yurys,
> we have a general question here. Current approach filters out only floats and doubles.
> Andrey suggests more consistent solution, that would also disallow enums and longs. That would require explicit static_cast<> in several places throughout our code. I found this a bit unrequested effect.
> What would you say?
I think adding explicit type casts for longs would be fine as we can loose precision in those places and should probably switch the code from long to int or fix the protocol. As for the enums - all of them a generated and we could generate no_implicit_cast specializations for them as a bonus we wouldn't be able to pass enum values not from the protocol to the protocol messages. What do you think?

[Peter Rybin]

(In reply to comment #7)
> (In reply to comment #6)
> > Yurys,
> > we have a general question here. Current approach filters out only floats and doubles.
> > Andrey suggests more consistent solution, that would also disallow enums and longs. That would require explicit static_cast<> in several places throughout our code. I found this a bit unrequested effect.
> > What would you say?

> I think adding explicit type casts for longs would be fine as we can loose precision in those places and should probably switch the code from long to int or fix the protocol. As for the enums - all of them a generated and we could generate no_implicit_cast specializations for them as a bonus we wouldn't be able to pass enum values not from the protocol to the protocol messages. What do you think?

Enums are not generated, all enums in question are hand-written. Disallowing implicit casts for them is not a problem. Quite opposite, I worked for allowing implicit casts for them.
If you feel like it's ok to do explicit casts for enums and longs, I'm fine. 

That would make my part easier. My concerns were about burdening the codebase with out-of-nowhere casts.

[Yury Semikhatsky]

(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > Yurys,
> > > we have a general question here. Current approach filters out only floats and doubles.
> > > Andrey suggests more consistent solution, that would also disallow enums and longs. That would require explicit static_cast<> in several places throughout our code. I found this a bit unrequested effect.
> > > What would you say?
> 
> > I think adding explicit type casts for longs would be fine as we can loose precision in those places and should probably switch the code from long to int or fix the protocol. As for the enums - all of them a generated and we could generate no_implicit_cast specializations for them as a bonus we wouldn't be able to pass enum values not from the protocol to the protocol messages. What do you think?
> 
> Enums are not generated, all enums in question are hand-written. Disallowing implicit casts for them is not a problem. Quite opposite, I worked for allowing implicit casts for them.
Do you have an estimate on how many enums are passed as integers through the protocol(should be easy to detect them with the compiler)? As Andrey suggests we should probably use enums explicitly declared in the protocol instead in those cases.

[Peter Rybin]

Created attachment 144255 [details]
Patch

[Peter Rybin]

(In reply to comment #10)
> Created an attachment (id=144255) [details]
> Patch

All types are now disallowed but int and unsigned int.

11 static-casts in the codebase, including 3 for enums.

[WebKit Review Bot]

Comment on attachment 144255 [details]
Patch

Clearing flags on attachment: 144255

Committed r118652: <http://trac.webkit.org/changeset/118652>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Alexis Menard (darktears)]

(In reply to comment #13)
> All reviewed patches have been landed.  Closing bug.

Hi,

As http://build.webkit.org/builders/Lion%20Debug%20%28Build%29 and many other Mac bots (http://build.webkit.org/waterfall?category=AppleMac) the build was broken by this patch (Mac OS Lion apparently).

I landed a build fix (hopefully!) with http://trac.webkit.org/changeset/118681.

Please watch the bots :).

Thanks.

[Peter Rybin]

Thanks a lot for the fix. Looks totally legit.
I'm sorry, indeed I forget to watch this land.


(In reply to comment #14)
> (In reply to comment #13)
> > All reviewed patches have been landed.  Closing bug.
> 
> Hi,
> 
> As http://build.webkit.org/builders/Lion%20Debug%20%28Build%29 and many other Mac bots (http://build.webkit.org/waterfall?category=AppleMac) the build was broken by this patch (Mac OS Lion apparently).
> 
> I landed a build fix (hopefully!) with http://trac.webkit.org/changeset/118681.
> 
> Please watch the bots :).
> 
> Thanks.

[Jessie Berlin]

This appears to have caused lots of crashes on Windows WK2: 
http://build.webkit.org/results/Windows%207%20Release%20(WebKit2%20Tests)/r118801%20(18680)/inspector/cookie-parser-crash-log.txt

PROBLEM_CLASSES: 

NULL_CLASS_PTR_DEREFERENCE
    Tid    [0x0]
    Frame  [0x00]

ONE_BIT
    Failure Bucketing

INVALID_POINTER_READ
    Tid    [0x1ba4]
    Frame  [0x00]: ntdll!ZwRaiseException

SHUTDOWN
    Tid    [0x1ba4]
    Frame  [0x00]: ntdll!ZwRaiseException
    Failure Bucketing


BUGCHECK_STR:  APPLICATION_FAULT_NULL_CLASS_PTR_DEREFERENCE_ONE_BIT_INVALID_POINTER_READ_SHUTDOWN

PRIMARY_PROBLEM_CLASS:  NULL_CLASS_PTR_DEREFERENCE_ONE_BIT_SHUTDOWN

DEFAULT_BUCKET_ID:  NULL_CLASS_PTR_DEREFERENCE_ONE_BIT_SHUTDOWN

STACK_TEXT:  
0041f4c4 6fab3ef5 00000001 0041f4f8 0041f530 WebKit!WebCore::TypeBuilder::Memory::MemoryBlock::Builder<0>::setName+0x3c [c:\cygwin\home\buildbot\slave\win-release\build\webkitbuild\release\obj\webcore\derivedsources\inspectortypebuilder.h @ 625]
0041f4ec 6fab6a53 0041f530 7ee942e8 7e0dd210 WebKit!WebCore::PageRuntimeAgent::notifyContextCreated+0xe5 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\inspector\pageruntimeagent.cpp @ 161]
0041f524 6fdbe91a 0041f554 7da712d0 7d9f1440 WebKit!WebCore::PageRuntimeAgent::setReportExecutionContextCreation+0xf3 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\inspector\pageruntimeagent.cpp @ 101]
0041f558 6fdd58a6 00000016 7e0e02e0 7e64b730 WebKit!WebCore::InspectorBackendDispatcherImpl::Runtime_setReportExecutionContextCreation+0x17a [c:\cygwin\home\buildbot\slave\win-release\build\webkitbuild\release\obj\webcore\derivedsources\inspectorbackenddispatcher.cpp @ 1305]
0041f5b8 6fa8f153 7e0dd180 7ee94810 7e64b708 WebKit!WebCore::InspectorBackendDispatcherImpl::dispatch+0x1206 [c:\cygwin\home\buildbot\slave\win-release\build\webkitbuild\release\obj\webcore\derivedsources\inspectorbackenddispatcher.cpp @ 4856]
0041f5d0 6fd6caab 7e64b708 6fded0bf 6fe40d80 WebKit!WebCore::InspectorBackendDispatchTask::onTimer+0x43 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\inspector\inspectorfrontendclientlocal.cpp @ 92]
0041f5d8 6fded0bf 6fe40d80 00000000 dc06b8f9 WebKit!WebCore::Timer<WebCore::CachedResourceLoader>::fired+0xb [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\platform\timer.h @ 100]
0041f5f8 6fe40dcd 0041f62c 75e362fa 02c601b0 WebKit!WebCore::ThreadTimers::sharedTimerFiredInternal+0x7f [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\platform\threadtimers.cpp @ 118]
0041f600 75e362fa 02c601b0 0000c126 00000000 WebKit!WebCore::TimerWindowWndProc+0x4d [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\platform\win\sharedtimerwin.cpp @ 104]
0041f62c 75e36d3a 6fe40d80 02c601b0 0000c126 USER32!InternalCallWinProc+0x23
0041f6a4 75e377c4 00000000 6fe40d80 02c601b0 USER32!UserCallWinProcCheckWow+0x109
0041f704 75e3788a 6fe40d80 00000000 0041f748 USER32!DispatchMessageWorker+0x3bc
0041f714 6f7b3a01 0041f72c 0041f790 00000000 USER32!DispatchMessageW+0xf
0041f748 6f7511ce 76051222 7ee90488 7ee913c0 WebKit!WebCore::RunLoop::run+0x41 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\platform\win\runloopwin.cpp @ 76]
0041f75c 6f7268d6 0041f790 00000000 7ee951f0 WebKit!WebKit::WebProcessMain+0xde [c:\cygwin\home\buildbot\slave\win-release\build\source\webkit2\webprocess\win\webprocessmainwin.cpp @ 84]
0041f77c 6f72697c 00000000 013c0000 004714ce WebKit!WebKitMain+0x116 [c:\cygwin\home\buildbot\slave\win-release\build\source\webkit2\webprocess\webkitmain.cpp @ 59]
0041f7a8 013c1098 013c0000 00000000 004714ce WebKit!WebKitMain+0x9c [c:\cygwin\home\buildbot\slave\win-release\build\source\webkit2\webprocess\webkitmain.cpp @ 187]
0041f9d8 013c1258 013c0000 00000000 004714ce WebKit2WebProcess!wWinMain+0x98 [c:\cygwin\home\buildbot\slave\win-release\build\source\webkit2\win\mainwin.cpp @ 67]
0041fa6c 7605339a 7efde000 0041fab8 77d89ef2 WebKit2WebProcess!__tmainCRTStartup+0x150 [f:\dd\vctools\crt_bld\self_x86\crt\src\crtexe.c @ 589]
0041fa78 77d89ef2 7efde000 f97d0110 00000000 kernel32!BaseThreadInitThunk+0xe
0041fab8 77d89ec5 013c13c4 7efde000 00000000 ntdll!__RtlUserThreadStart+0x70
0041fad0 00000000 013c13c4 7efde000 00000000 ntdll!_RtlUserThreadStart+0x1b

[Peter Rybin]

Thank you very much. Looks very relevant, yet I don't see what could go wrong.
Looking into it.

(In reply to comment #16)
> This appears to have caused lots of crashes on Windows WK2: 
> http://build.webkit.org/results/Windows%207%20Release%20(WebKit2%20Tests)/r118801%20(18680)/inspector/cookie-parser-crash-log.txt
> 
> PROBLEM_CLASSES: 
> 
> NULL_CLASS_PTR_DEREFERENCE
>     Tid    [0x0]
>     Frame  [0x00]
> 
> ONE_BIT
>     Failure Bucketing
> 
> INVALID_POINTER_READ
>     Tid    [0x1ba4]
>     Frame  [0x00]: ntdll!ZwRaiseException
> 
> SHUTDOWN
>     Tid    [0x1ba4]
>     Frame  [0x00]: ntdll!ZwRaiseException
>     Failure Bucketing
> 
> 
> BUGCHECK_STR:  APPLICATION_FAULT_NULL_CLASS_PTR_DEREFERENCE_ONE_BIT_INVALID_POINTER_READ_SHUTDOWN
> 
> PRIMARY_PROBLEM_CLASS:  NULL_CLASS_PTR_DEREFERENCE_ONE_BIT_SHUTDOWN
> 
> DEFAULT_BUCKET_ID:  NULL_CLASS_PTR_DEREFERENCE_ONE_BIT_SHUTDOWN
> 
> STACK_TEXT:  
> 0041f4c4 6fab3ef5 00000001 0041f4f8 0041f530 WebKit!WebCore::TypeBuilder::Memory::MemoryBlock::Builder<0>::setName+0x3c [c:\cygwin\home\buildbot\slave\win-release\build\webkitbuild\release\obj\webcore\derivedsources\inspectortypebuilder.h @ 625]
> 0041f4ec 6fab6a53 0041f530 7ee942e8 7e0dd210 WebKit!WebCore::PageRuntimeAgent::notifyContextCreated+0xe5 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\inspector\pageruntimeagent.cpp @ 161]
> 0041f524 6fdbe91a 0041f554 7da712d0 7d9f1440 WebKit!WebCore::PageRuntimeAgent::setReportExecutionContextCreation+0xf3 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\inspector\pageruntimeagent.cpp @ 101]
> 0041f558 6fdd58a6 00000016 7e0e02e0 7e64b730 WebKit!WebCore::InspectorBackendDispatcherImpl::Runtime_setReportExecutionContextCreation+0x17a [c:\cygwin\home\buildbot\slave\win-release\build\webkitbuild\release\obj\webcore\derivedsources\inspectorbackenddispatcher.cpp @ 1305]
> 0041f5b8 6fa8f153 7e0dd180 7ee94810 7e64b708 WebKit!WebCore::InspectorBackendDispatcherImpl::dispatch+0x1206 [c:\cygwin\home\buildbot\slave\win-release\build\webkitbuild\release\obj\webcore\derivedsources\inspectorbackenddispatcher.cpp @ 4856]
> 0041f5d0 6fd6caab 7e64b708 6fded0bf 6fe40d80 WebKit!WebCore::InspectorBackendDispatchTask::onTimer+0x43 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\inspector\inspectorfrontendclientlocal.cpp @ 92]
> 0041f5d8 6fded0bf 6fe40d80 00000000 dc06b8f9 WebKit!WebCore::Timer<WebCore::CachedResourceLoader>::fired+0xb [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\platform\timer.h @ 100]
> 0041f5f8 6fe40dcd 0041f62c 75e362fa 02c601b0 WebKit!WebCore::ThreadTimers::sharedTimerFiredInternal+0x7f [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\platform\threadtimers.cpp @ 118]
> 0041f600 75e362fa 02c601b0 0000c126 00000000 WebKit!WebCore::TimerWindowWndProc+0x4d [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\platform\win\sharedtimerwin.cpp @ 104]
> 0041f62c 75e36d3a 6fe40d80 02c601b0 0000c126 USER32!InternalCallWinProc+0x23
> 0041f6a4 75e377c4 00000000 6fe40d80 02c601b0 USER32!UserCallWinProcCheckWow+0x109
> 0041f704 75e3788a 6fe40d80 00000000 0041f748 USER32!DispatchMessageWorker+0x3bc
> 0041f714 6f7b3a01 0041f72c 0041f790 00000000 USER32!DispatchMessageW+0xf
> 0041f748 6f7511ce 76051222 7ee90488 7ee913c0 WebKit!WebCore::RunLoop::run+0x41 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\platform\win\runloopwin.cpp @ 76]
> 0041f75c 6f7268d6 0041f790 00000000 7ee951f0 WebKit!WebKit::WebProcessMain+0xde [c:\cygwin\home\buildbot\slave\win-release\build\source\webkit2\webprocess\win\webprocessmainwin.cpp @ 84]
> 0041f77c 6f72697c 00000000 013c0000 004714ce WebKit!WebKitMain+0x116 [c:\cygwin\home\buildbot\slave\win-release\build\source\webkit2\webprocess\webkitmain.cpp @ 59]
> 0041f7a8 013c1098 013c0000 00000000 004714ce WebKit!WebKitMain+0x9c [c:\cygwin\home\buildbot\slave\win-release\build\source\webkit2\webprocess\webkitmain.cpp @ 187]
> 0041f9d8 013c1258 013c0000 00000000 004714ce WebKit2WebProcess!wWinMain+0x98 [c:\cygwin\home\buildbot\slave\win-release\build\source\webkit2\win\mainwin.cpp @ 67]
> 0041fa6c 7605339a 7efde000 0041fab8 77d89ef2 WebKit2WebProcess!__tmainCRTStartup+0x150 [f:\dd\vctools\crt_bld\self_x86\crt\src\crtexe.c @ 589]
> 0041fa78 77d89ef2 7efde000 f97d0110 00000000 kernel32!BaseThreadInitThunk+0xe
> 0041fab8 77d89ec5 013c13c4 7efde000 00000000 ntdll!__RtlUserThreadStart+0x70
> 0041fad0 00000000 013c13c4 7efde000 00000000 ntdll!_RtlUserThreadStart+0x1b

(In reply to comment #16)
> This appears to have caused lots of crashes on Windows WK2: 
> http://build.webkit.org/results/Windows%207%20Release%20(WebKit2%20Tests)/r118801%20(18680)/inspector/cookie-parser-crash-log.txt
> 
> PROBLEM_CLASSES: 
> 
> NULL_CLASS_PTR_DEREFERENCE
>     Tid    [0x0]
>     Frame  [0x00]
> 
> ONE_BIT
>     Failure Bucketing
> 
> INVALID_POINTER_READ
>     Tid    [0x1ba4]
>     Frame  [0x00]: ntdll!ZwRaiseException
> 
> SHUTDOWN
>     Tid    [0x1ba4]
>     Frame  [0x00]: ntdll!ZwRaiseException
>     Failure Bucketing
> 
> 
> BUGCHECK_STR:  APPLICATION_FAULT_NULL_CLASS_PTR_DEREFERENCE_ONE_BIT_INVALID_POINTER_READ_SHUTDOWN
> 
> PRIMARY_PROBLEM_CLASS:  NULL_CLASS_PTR_DEREFERENCE_ONE_BIT_SHUTDOWN
> 
> DEFAULT_BUCKET_ID:  NULL_CLASS_PTR_DEREFERENCE_ONE_BIT_SHUTDOWN
> 
> STACK_TEXT:  
> 0041f4c4 6fab3ef5 00000001 0041f4f8 0041f530 WebKit!WebCore::TypeBuilder::Memory::MemoryBlock::Builder<0>::setName+0x3c [c:\cygwin\home\buildbot\slave\win-release\build\webkitbuild\release\obj\webcore\derivedsources\inspectortypebuilder.h @ 625]
> 0041f4ec 6fab6a53 0041f530 7ee942e8 7e0dd210 WebKit!WebCore::PageRuntimeAgent::notifyContextCreated+0xe5 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\inspector\pageruntimeagent.cpp @ 161]
> 0041f524 6fdbe91a 0041f554 7da712d0 7d9f1440 WebKit!WebCore::PageRuntimeAgent::setReportExecutionContextCreation+0xf3 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\inspector\pageruntimeagent.cpp @ 101]
> 0041f558 6fdd58a6 00000016 7e0e02e0 7e64b730 WebKit!WebCore::InspectorBackendDispatcherImpl::Runtime_setReportExecutionContextCreation+0x17a [c:\cygwin\home\buildbot\slave\win-release\build\webkitbuild\release\obj\webcore\derivedsources\inspectorbackenddispatcher.cpp @ 1305]
> 0041f5b8 6fa8f153 7e0dd180 7ee94810 7e64b708 WebKit!WebCore::InspectorBackendDispatcherImpl::dispatch+0x1206 [c:\cygwin\home\buildbot\slave\win-release\build\webkitbuild\release\obj\webcore\derivedsources\inspectorbackenddispatcher.cpp @ 4856]
> 0041f5d0 6fd6caab 7e64b708 6fded0bf 6fe40d80 WebKit!WebCore::InspectorBackendDispatchTask::onTimer+0x43 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\inspector\inspectorfrontendclientlocal.cpp @ 92]
> 0041f5d8 6fded0bf 6fe40d80 00000000 dc06b8f9 WebKit!WebCore::Timer<WebCore::CachedResourceLoader>::fired+0xb [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\platform\timer.h @ 100]
> 0041f5f8 6fe40dcd 0041f62c 75e362fa 02c601b0 WebKit!WebCore::ThreadTimers::sharedTimerFiredInternal+0x7f [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\platform\threadtimers.cpp @ 118]
> 0041f600 75e362fa 02c601b0 0000c126 00000000 WebKit!WebCore::TimerWindowWndProc+0x4d [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\platform\win\sharedtimerwin.cpp @ 104]
> 0041f62c 75e36d3a 6fe40d80 02c601b0 0000c126 USER32!InternalCallWinProc+0x23
> 0041f6a4 75e377c4 00000000 6fe40d80 02c601b0 USER32!UserCallWinProcCheckWow+0x109
> 0041f704 75e3788a 6fe40d80 00000000 0041f748 USER32!DispatchMessageWorker+0x3bc
> 0041f714 6f7b3a01 0041f72c 0041f790 00000000 USER32!DispatchMessageW+0xf
> 0041f748 6f7511ce 76051222 7ee90488 7ee913c0 WebKit!WebCore::RunLoop::run+0x41 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\platform\win\runloopwin.cpp @ 76]
> 0041f75c 6f7268d6 0041f790 00000000 7ee951f0 WebKit!WebKit::WebProcessMain+0xde [c:\cygwin\home\buildbot\slave\win-release\build\source\webkit2\webprocess\win\webprocessmainwin.cpp @ 84]
> 0041f77c 6f72697c 00000000 013c0000 004714ce WebKit!WebKitMain+0x116 [c:\cygwin\home\buildbot\slave\win-release\build\source\webkit2\webprocess\webkitmain.cpp @ 59]
> 0041f7a8 013c1098 013c0000 00000000 004714ce WebKit!WebKitMain+0x9c [c:\cygwin\home\buildbot\slave\win-release\build\source\webkit2\webprocess\webkitmain.cpp @ 187]
> 0041f9d8 013c1258 013c0000 00000000 004714ce WebKit2WebProcess!wWinMain+0x98 [c:\cygwin\home\buildbot\slave\win-release\build\source\webkit2\win\mainwin.cpp @ 67]
> 0041fa6c 7605339a 7efde000 0041fab8 77d89ef2 WebKit2WebProcess!__tmainCRTStartup+0x150 [f:\dd\vctools\crt_bld\self_x86\crt\src\crtexe.c @ 589]
> 0041fa78 77d89ef2 7efde000 f97d0110 00000000 kernel32!BaseThreadInitThunk+0xe
> 0041fab8 77d89ec5 013c13c4 7efde000 00000000 ntdll!__RtlUserThreadStart+0x70
> 0041fad0 00000000 013c13c4 7efde000 00000000 ntdll!_RtlUserThreadStart+0x1b

[Peter Rybin]

(In reply to comment #17)
> Thank you very much. Looks very relevant, yet I don't see what could go wrong.
> Looking into it.

What bugs me here is that stacktraces in all crashes look quite correct, but the top frame:

1. WebKit!WebCore::TypeBuilder::Memory::MemoryBlock::Builder<0>::setName+0x3c [c:\cygwin\home\buildbot\slave\win-release\build\webkitbuild\release\obj\webcore\derivedsources\inspectortypebuilder.h @ 625]

2. WebKit!WebCore::PageRuntimeAgent::notifyContextCreated+0xe5 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\inspector\pageruntimeagent.cpp @ 161]

...

While "MemoryBlock::Builder<0>::setName" method exists and line #625 is absolutely meaningful, PageRuntimeAgent.cpp has nothing to do with this method at all. The file never mentions MemoryBlock type. It does call "setName" method of completely different type (Runtime::ExecutionContextDescription::Builder). Moreover, MemoryBlock::Builder<0> is a template instantiation and this instantiation could only occur from the different file – InspectorMemoryAgent.cpp.

As crazy as it sounds, right now it looks like a corrupted stacktrace or a linker problem, not even compiler (weird).

[Peter Rybin]

I looked further and found more: in various builds the crash is reported at different methods (but all crashes within one build point to the method).

The method is always called "setName" and is from types called Builder<int i>. Line is always "inspectortypebuilder.h @ 625", which corresponds to Memory::MemoryBlock::Builder (coincidentally a first method "setName" in InspectorTypeBuilder.h source file).

Here's a table of method changes with build:
r118742   Database::Database::Builder<3>::setName
r118747   Database::Database::Builder<3>::setName
r118801   Memory::MemoryBlock::Builder<0>::setName
r118804   Database::Database::Builder<3>::setName
r118806   Database::Database::Builder<3>::setName
r118809   CSS::CSSProperty::Builder<0>::setName

This looks as if each the method changes chaotically with any change to build offsets.

Still it doesn't explain why it crashes.

Here's a snippet from the call site:

    m_frontend->isolatedContextCreated(ExecutionContextDescription::create()
        .setId(static_cast<int>(executionContextId))  <<<***** a changed line
        .setIsPageContext(isPageContext)
        .setName(name)
        .setFrameId(frameId)
        .release());

[Peter Rybin]

As investigation shows, it looks very much like compiler bug. Incorrect method "create" is called too.

All inspector .cpp sources are concatenated in one and compiled together. This probably makes compiler go crazy from the number of templates.


Further investigation: whether debug build gets the same bug.

[Peter Rybin]

Should be fixed in https://bugs.webkit.org/show_bug.cgi?id=87857

[Yury Semikhatsky]

Reopening the bug as the feature was disabled in http://trac.webkit.org/changeset/118925 due to layout test crashes on Windows.

[Brian Burg]

Needs to be re-evaluated in light of the new protocol code generator and upcoming changes in C++-side protocol type safety.

[Radar WebKit Bug Importer]

<rdar://problem/19107094>

[Blaze Burg]

I don't think we should fix this. I am not aware of any bugs where floats have been truncated or converted to ints in our backend.