202077 2019-09-21 14:16:39 -0700 Safari 13 may launch leftover 32-bit plug-in process from Safari 12’s WebKit, which crashes 2019-09-21 23:13:29 -0700 1 1 1 Unclassified WebKit Plug-ins WebKit Local Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 mitz webkit-unassigned cdumez commit-queue ryanhaddad sam sihui_liu oldest_to_newest 1573088 0 mitz 2019-09-21 14:16:39 -0700 rdar://problem/55547063 On macOS High Sierra, if Safari 13 was installed after Safari 12 had been installed, Safari may end up launching the 32-bit plug-in service left behind in the staged WebKit framework, and the service will crash immediately (because as of Safari 13, its dependencies don’t have a 32-bit slice). This can happen, for example, if there is still a 32-bit plug-in in /Library/Internet Plug-Ins, and the user chooses to manage website data. This is because even though the 32-bit plug-in service has been removed from WebKit, the code paths that launch it haven’t been removed. Patch forthcoming. 1573094 1 379323 mitz 2019-09-21 14:32:06 -0700 Created attachment 379323 Remove 32-bit plug-in code paths 1573095 2 379323 sam 2019-09-21 14:42:46 -0700 Comment on attachment 379323 Remove 32-bit plug-in code paths View in context: https://bugs.webkit.org/attachment.cgi?id=379323&action=review > Source/WebKit/UIProcess/Launcher/mac/ProcessLauncherMac.mm:60 > case ProcessLauncher::ProcessType::Plugin32: What is the reason for keeping this enum case at all? 1573096 3 mitz 2019-09-21 14:48:37 -0700 (In reply to Sam Weinig from comment #2) > Comment on attachment 379323 [details] > Remove 32-bit plug-in code paths > > View in context: > https://bugs.webkit.org/attachment.cgi?id=379323&action=review > > > Source/WebKit/UIProcess/Launcher/mac/ProcessLauncherMac.mm:60 > > case ProcessLauncher::ProcessType::Plugin32: > > What is the reason for keeping this enum case at all? ProcessType is a cross-platform type, and I didn’t want to expand the scope of this patch beyond Mac code. 1573097 4 sam 2019-09-21 14:58:14 -0700 Gotcha. 1573098 5 379323 commit-queue 2019-09-21 15:57:53 -0700 Comment on attachment 379323 Remove 32-bit plug-in code paths Clearing flags on attachment: 379323 Committed r250186: <https://trac.webkit.org/changeset/250186> 1573099 6 commit-queue 2019-09-21 15:57:54 -0700 All reviewed patches have been landed. Closing bug. 1573164 7 cdumez 2019-09-21 23:05:48 -0700 This is causing a lot of crashes on the bots (They exit early): https://build.webkit.org/results/Apple%20Mojave%20Debug%20WK2%20(Tests)/r250186%20(4826)/results.html 1573165 8 379323 cdumez 2019-09-21 23:06:53 -0700 Comment on attachment 379323 Remove 32-bit plug-in code paths View in context: https://bugs.webkit.org/attachment.cgi?id=379323&action=review > Source/WebKit/UIProcess/Plugins/mac/PluginProcessProxyMac.mm:63 > + ASSERT(pluginProcessAttributes.moduleInfo.pluginArchitecture == CPU_TYPE_X86); Looks like this assertion is hitting. 1573166 9 mitz 2019-09-21 23:07:27 -0700 (In reply to Chris Dumez from comment #8) > Comment on attachment 379323 [details] > Remove 32-bit plug-in code paths > > View in context: > https://bugs.webkit.org/attachment.cgi?id=379323&action=review > > > Source/WebKit/UIProcess/Plugins/mac/PluginProcessProxyMac.mm:63 > > + ASSERT(pluginProcessAttributes.moduleInfo.pluginArchitecture == CPU_TYPE_X86); > > Looks like this assertion is hitting. I’ll see if I can reproduce this locally. 1573167 10 mitz 2019-09-21 23:10:33 -0700 (In reply to mitz from comment #9) > (In reply to Chris Dumez from comment #8) > > Comment on attachment 379323 [details] > > Remove 32-bit plug-in code paths > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=379323&action=review > > > > > Source/WebKit/UIProcess/Plugins/mac/PluginProcessProxyMac.mm:63 > > > + ASSERT(pluginProcessAttributes.moduleInfo.pluginArchitecture == CPU_TYPE_X86); Silly mistake, this should say CPU_TYPE_X86_64. I’ll fix it right away. Thanks for noticing this! 1573168 11 379323 cdumez 2019-09-21 23:11:14 -0700 Comment on attachment 379323 Remove 32-bit plug-in code paths View in context: https://bugs.webkit.org/attachment.cgi?id=379323&action=review >>> Source/WebKit/UIProcess/Plugins/mac/PluginProcessProxyMac.mm:63 >>> + ASSERT(pluginProcessAttributes.moduleInfo.pluginArchitecture == CPU_TYPE_X86); >> >> Looks like this assertion is hitting. > > I’ll see if I can reproduce this locally. Shouldn’t this be != instead of == ? 1573169 12 cdumez 2019-09-21 23:13:19 -0700 (In reply to mitz from comment #10) > (In reply to mitz from comment #9) > > (In reply to Chris Dumez from comment #8) > > > Comment on attachment 379323 [details] > > > Remove 32-bit plug-in code paths > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=379323&action=review > > > > > > > Source/WebKit/UIProcess/Plugins/mac/PluginProcessProxyMac.mm:63 > > > > + ASSERT(pluginProcessAttributes.moduleInfo.pluginArchitecture == CPU_TYPE_X86); > > Silly mistake, this should say CPU_TYPE_X86_64. I’ll fix it right away. > Thanks for noticing this! Oh just saw your comment, glad this is an easy fix. 1573170 13 mitz 2019-09-21 23:13:29 -0700 Corrected the assertion in r250197. 379323 2019-09-21 14:32:06 -0700 2019-09-21 15:57:53 -0700 Remove 32-bit plug-in code paths file_202077.txt text/plain 3682 mitz SW5kZXg6IFNvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nDQo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09DQotLS0gU291cmNlL1dl YktpdC9DaGFuZ2VMb2cJKHJldmlzaW9uIDI1MDE4NSkNCisrKyBTb3VyY2UvV2ViS2l0L0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQ0KQEAgLTEsMyArMSwyMSBAQA0KKzIwMTktMDktMjEgIERhbiBC ZXJuc3RlaW4gIDxtaXR6QGFwcGxlLmNvbT4NCisNCisgICAgICAgIFNhZmFyaSAxMyBtYXkgbGF1 bmNoIGxlZnRvdmVyIDMyLWJpdCBwbHVnLWluIHByb2Nlc3MgZnJvbSBTYWZhcmkgMTLigJlzIFdl YktpdCwgd2hpY2ggY3Jhc2hlcw0KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTIwMjA3Nw0KKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzU1NTQ3MDYzPg0K Kw0KKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuDQorDQorICAgICAgICAqIFNo YXJlZC9QbHVnaW5zL05ldHNjYXBlL21hYy9OZXRzY2FwZVBsdWdpbk1vZHVsZU1hYy5tbToNCisg ICAgICAgIChXZWJLaXQ6OmdldFBsdWdpbkFyY2hpdGVjdHVyZSk6IFJlbW92ZWQgc3VwcG9ydCBm b3IgaTM4NiBwbHVnLWlucy4NCisNCisgICAgICAgICogVUlQcm9jZXNzL0xhdW5jaGVyL21hYy9Q cm9jZXNzTGF1bmNoZXJNYWMubW06DQorICAgICAgICAoV2ViS2l0OjpzZXJ2aWNlTmFtZSk6IEFz c2VydCB0aGF0IHRoZSByZXF1ZXN0ZWQgcHJvY2VzcyB0eXBlIGlzIG5vdCBQbHVnaW4zMi4NCisN CisgICAgICAgICogVUlQcm9jZXNzL1BsdWdpbnMvbWFjL1BsdWdpblByb2Nlc3NQcm94eU1hYy5t bToNCisgICAgICAgIChXZWJLaXQ6OlBsdWdpblByb2Nlc3NQcm94eTo6cGxhdGZvcm1HZXRMYXVu Y2hPcHRpb25zV2l0aEF0dHJpYnV0ZXMpOiBBc3NlcnQgdGhhdCB0aGUNCisgICAgICAgICAgcGx1 Zy1pbiBhcmNoaXRlY3R1cmUgaXMgeDg2XzY0Lg0KKw0KIDIwMTktMDktMjEgIEFudG9pbmUgUXVp bnQgIDxncmFvdXRzQGFwcGxlLmNvbT4NCiANCiAgICAgICAgIFtQb2ludGVyIEV2ZW50c10gdG91 Y2gtYWN0aW9uIHNldCB0byBwYW4teCBvciBwYW4teSBhbG9uZSBzaG91bGQgZGlzYWJsZSBzY3Jv bGxpbmcgYWx0b2dldGhlciBpZiB0aGUgaW50aWFsIGdlc3R1cmUgaXMgaW4gdGhlIGRpc2FsbG93 ZWQgZGlyZWN0aW9uDQpJbmRleDogU291cmNlL1dlYktpdC9TaGFyZWQvUGx1Z2lucy9OZXRzY2Fw ZS9tYWMvTmV0c2NhcGVQbHVnaW5Nb2R1bGVNYWMubW0NCj09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0NCi0tLSBTb3VyY2Uv V2ViS2l0L1NoYXJlZC9QbHVnaW5zL05ldHNjYXBlL21hYy9OZXRzY2FwZVBsdWdpbk1vZHVsZU1h Yy5tbQkocmV2aXNpb24gMjUwMTg1KQ0KKysrIFNvdXJjZS9XZWJLaXQvU2hhcmVkL1BsdWdpbnMv TmV0c2NhcGUvbWFjL05ldHNjYXBlUGx1Z2luTW9kdWxlTWFjLm1tCSh3b3JraW5nIGNvcHkpDQpA QCAtNjAsMTIgKzYwLDYgQEAgc3RhdGljIGJvb2wgZ2V0UGx1Z2luQXJjaGl0ZWN0dXJlKENGQnVu ZA0KICAgICAgICAgcGx1Z2luLnBsdWdpbkFyY2hpdGVjdHVyZSA9IENQVV9UWVBFX1g4Nl82NDsN CiAgICAgICAgIHJldHVybiB0cnVlOw0KICAgICB9DQotDQotICAgIC8vIFdlIGFsc28gc3VwcG9y dCAzMi1iaXQgSW50ZWwgcGx1Zy1pbnMgb24gNjQtYml0IEludGVsLg0KLSAgICBpZiAoYXJjaGl0 ZWN0dXJlcy5jb250YWlucyhrQ0ZCdW5kbGVFeGVjdXRhYmxlQXJjaGl0ZWN0dXJlSTM4NikpIHsN Ci0gICAgICAgIHBsdWdpbi5wbHVnaW5BcmNoaXRlY3R1cmUgPSBDUFVfVFlQRV9YODY7DQotICAg ICAgICByZXR1cm4gdHJ1ZTsNCi0gICAgfQ0KICNlbmRpZg0KIA0KICAgICByZXR1cm4gZmFsc2U7 DQpJbmRleDogU291cmNlL1dlYktpdC9VSVByb2Nlc3MvTGF1bmNoZXIvbWFjL1Byb2Nlc3NMYXVu Y2hlck1hYy5tbQ0KPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PQ0KLS0tIFNvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL0xhdW5j aGVyL21hYy9Qcm9jZXNzTGF1bmNoZXJNYWMubW0JKHJldmlzaW9uIDI1MDE4NSkNCisrKyBTb3Vy Y2UvV2ViS2l0L1VJUHJvY2Vzcy9MYXVuY2hlci9tYWMvUHJvY2Vzc0xhdW5jaGVyTWFjLm1tCSh3 b3JraW5nIGNvcHkpDQpAQCAtNTgsNyArNTgsOCBAQCBzdGF0aWMgY29uc3QgY2hhciogc2Vydmlj ZU5hbWUoY29uc3QgUHJvDQogICAgICAgICByZXR1cm4gImNvbS5hcHBsZS5XZWJLaXQuTmV0d29y a2luZyI7DQogI2lmIEVOQUJMRShORVRTQ0FQRV9QTFVHSU5fQVBJKQ0KICAgICBjYXNlIFByb2Nl c3NMYXVuY2hlcjo6UHJvY2Vzc1R5cGU6OlBsdWdpbjMyOg0KLSAgICAgICAgcmV0dXJuICJjb20u YXBwbGUuV2ViS2l0LlBsdWdpbi4zMiI7DQorICAgICAgICBBU1NFUlRfTk9UX1JFQUNIRUQoKTsN CisgICAgICAgIHJldHVybiBudWxscHRyOw0KICAgICBjYXNlIFByb2Nlc3NMYXVuY2hlcjo6UHJv Y2Vzc1R5cGU6OlBsdWdpbjY0Og0KICAgICAgICAgcmV0dXJuICJjb20uYXBwbGUuV2ViS2l0LlBs dWdpbi42NCI7DQogI2VuZGlmDQpJbmRleDogU291cmNlL1dlYktpdC9VSVByb2Nlc3MvUGx1Z2lu cy9tYWMvUGx1Z2luUHJvY2Vzc1Byb3h5TWFjLm1tDQo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09DQotLS0gU291cmNlL1dl YktpdC9VSVByb2Nlc3MvUGx1Z2lucy9tYWMvUGx1Z2luUHJvY2Vzc1Byb3h5TWFjLm1tCShyZXZp c2lvbiAyNTAxODUpDQorKysgU291cmNlL1dlYktpdC9VSVByb2Nlc3MvUGx1Z2lucy9tYWMvUGx1 Z2luUHJvY2Vzc1Byb3h5TWFjLm1tCSh3b3JraW5nIGNvcHkpDQpAQCAtNjAsMTAgKzYwLDggQEAg bmFtZXNwYWNlIFdlYktpdCB7DQogICAgIA0KIHZvaWQgUGx1Z2luUHJvY2Vzc1Byb3h5OjpwbGF0 Zm9ybUdldExhdW5jaE9wdGlvbnNXaXRoQXR0cmlidXRlcyhQcm9jZXNzTGF1bmNoZXI6OkxhdW5j aE9wdGlvbnMmIGxhdW5jaE9wdGlvbnMsIGNvbnN0IFBsdWdpblByb2Nlc3NBdHRyaWJ1dGVzJiBw bHVnaW5Qcm9jZXNzQXR0cmlidXRlcykNCiB7DQotICAgIGlmIChwbHVnaW5Qcm9jZXNzQXR0cmli dXRlcy5tb2R1bGVJbmZvLnBsdWdpbkFyY2hpdGVjdHVyZSA9PSBDUFVfVFlQRV9YODYpDQotICAg ICAgICBsYXVuY2hPcHRpb25zLnByb2Nlc3NUeXBlID0gUHJvY2Vzc0xhdW5jaGVyOjpQcm9jZXNz VHlwZTo6UGx1Z2luMzI7DQotICAgIGVsc2UNCi0gICAgICAgIGxhdW5jaE9wdGlvbnMucHJvY2Vz c1R5cGUgPSBQcm9jZXNzTGF1bmNoZXI6OlByb2Nlc3NUeXBlOjpQbHVnaW42NDsNCisgICAgQVNT RVJUKHBsdWdpblByb2Nlc3NBdHRyaWJ1dGVzLm1vZHVsZUluZm8ucGx1Z2luQXJjaGl0ZWN0dXJl ID09IENQVV9UWVBFX1g4Nik7DQorICAgIGxhdW5jaE9wdGlvbnMucHJvY2Vzc1R5cGUgPSBQcm9j ZXNzTGF1bmNoZXI6OlByb2Nlc3NUeXBlOjpQbHVnaW42NDsNCiANCiAgICAgbGF1bmNoT3B0aW9u cy5leHRyYUluaXRpYWxpemF0aW9uRGF0YS5hZGQoInBsdWdpbi1wYXRoIiwgcGx1Z2luUHJvY2Vz c0F0dHJpYnV0ZXMubW9kdWxlSW5mby5wYXRoKTsNCiANCg==