Bug 196132

Summary: Enable IPC sending and receiving non-default-constructible types
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, andersca, ap, cdumez, ews-watchlist, ggaren, Hironori.Fujii, rniwa, ryanhaddad, sam, tsavell, webkit-bot-watchers-bugzilla, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews105 for mac-highsierra-wk2
none
Archive of layout-test-results from ews124 for ios-simulator-wk2
none
Patch ggaren: review+

Description Alex Christensen 2019-03-21 23:25:40 PDT
Enable IPC sending and receiving non-default-constructible types
Comment 1 Alex Christensen 2019-03-21 23:28:36 PDT
Created attachment 365684 [details]
Patch
Comment 2 EWS Watchlist 2019-03-22 00:38:27 PDT
Comment on attachment 365684 [details]
Patch

Attachment 365684 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11610196

Number of test failures exceeded the failure limit.
Comment 3 EWS Watchlist 2019-03-22 00:38:28 PDT
Created attachment 365694 [details]
Archive of layout-test-results from ews105 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 4 EWS Watchlist 2019-03-22 00:59:10 PDT
Comment on attachment 365684 [details]
Patch

Attachment 365684 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11610091

Number of test failures exceeded the failure limit.
Comment 5 EWS Watchlist 2019-03-22 00:59:12 PDT
Created attachment 365695 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 6 Aakash Jain 2019-03-22 06:28:44 PDT
This patch seems to break lot of API tests.

iOS: https://ews-build.webkit-uat.org/#/builders/20/builds/2243 (94 new API Tests failures)

macOS: https://ews-build.webkit-uat.org/#/builders/19/builds/2398 (61 new API Tests failures)

If this is a false positive, please let us know.
Comment 7 Aakash Jain 2019-03-22 07:58:30 PDT
Retried the builds to confirm (by clicking 'Rebuild' button on Buildbot build page). Failed again:

iOS: https://ews-build.webkit-uat.org/#/builders/20/builds/2250 (93 new API Tests failures)

mac: https://ews-build.webkit-uat.org/#/builders/19/builds/2409 (61 new API Tests failures)

If this is a false positive, please let us know.
Comment 8 Alex Christensen 2019-03-22 16:44:35 PDT
Created attachment 365778 [details]
Patch
Comment 9 Alex Christensen 2019-03-22 16:46:01 PDT
(In reply to Aakash Jain from comment #7)
> If this is a false positive, please let us know.
It was definitely not a false positive.  With my first patch, anything on Cocoa platforms or Windows that sent a SharedMemory::Handle would crash because the default move constructor and operator= do not zero out the moved-from object.  My second patch fixes this.
Comment 10 Aakash Jain 2019-03-24 04:13:28 PDT
(In reply to Alex Christensen from comment #9)
> (In reply to Aakash Jain from comment #7)
> > If this is a false positive, please let us know.
> It was definitely not a false positive.  With my first patch, anything on Cocoa platforms or Windows that sent a SharedMemory::Handle would crash because the default move constructor and operator= do not zero out the moved-from object.
> My second patch fixes this.
Thanks. API tests pass now.

iOS: https://ews-build.webkit-uat.org/#/builders/20/builds/2281
macOS: https://ews-build.webkit-uat.org/#/builders/19/builds/2441
Comment 11 Geoffrey Garen 2019-03-25 13:04:52 PDT
Comment on attachment 365778 [details]
Patch

r=me
Comment 12 Alex Christensen 2019-03-25 14:24:03 PDT
http://trac.webkit.org/r243460
Comment 13 Radar WebKit Bug Importer 2019-03-25 14:26:06 PDT
<rdar://problem/49229221>
Comment 14 Fujii Hironori 2019-03-25 18:25:28 PDT
WinCairo port builds get broken.

https://build.webkit.org/builders/WinCairo%2064-bit%20WKL%20Debug%20%28Build%29/builds/7627

> FAILED: Source/WebKit/CMakeFiles/WebKit.dir/Platform/win/SharedMemoryWin.cpp.obj 
> "C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\VC\Tools\MSVC\14.14.26428\bin\Hostx64\x64\cl.exe" (...) -c ..\..\Source\WebKit\Platform\win\SharedMemoryWin.cpp
> C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\VC\Tools\MSVC\14.14.26428\include\utility(688): error C2440: '=': cannot convert from 'int' to 'HANDLE '
> C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\VC\Tools\MSVC\14.14.26428\include\utility(688): note: Conversion from integral type to pointer type requires reinterpret_cast, C-style cast or function-style cast
> ..\..\Source\WebKit\Platform\win\SharedMemoryWin.cpp(44): note: see reference to function template instantiation '_Ty *std::exchange<HANDLE,int>(_Ty &,_Other &&)' being compiled
>         with
>         [
>             _Ty=HANDLE,
>             _Other=int
>         ]
Comment 15 Fujii Hironori 2019-03-25 18:38:10 PDT
Committed r243479: <https://trac.webkit.org/changeset/243479>
Comment 16 Alex Christensen 2019-03-26 14:29:09 PDT
It's interesting that the constructor on line 37 worked but operator= did not.
Comment 17 Fujii Hironori 2019-03-26 18:56:07 PDT
Yeah. But, GCC and Clang also can't compile.
https://godbolt.org/z/Rz_fVO