Bug 196132 - Enable IPC sending and receiving non-default-constructible types
Summary: Enable IPC sending and receiving non-default-constructible types
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-03-21 23:25 PDT by Alex Christensen
Modified: 2019-03-26 18:56 PDT (History)
13 users (show)

See Also:


Attachments
Patch (39.94 KB, patch)
2019-03-21 23:28 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-highsierra-wk2 (1.21 MB, application/zip)
2019-03-22 00:38 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews124 for ios-simulator-wk2 (1.27 MB, application/zip)
2019-03-22 00:59 PDT, EWS Watchlist
no flags Details
Patch (41.85 KB, patch)
2019-03-22 16:44 PDT, Alex Christensen
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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