211267 2020-04-30 17:38:23 -0700 REGRESSION (r154253): JSC::PropertySlot::m_attributes is uninitialized in constructor 2020-05-19 11:30:30 -0700 1 1 1 Unclassified WebKit JavaScriptCore Other Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 119972 212095 1 ddkilzer ddkilzer darin ews-watchlist keith_miller mark.lam msaboff saam tzagallo webkit-bug-importer oldest_to_newest 1647674 0 ddkilzer 2020-04-30 17:38:23 -0700 JSC::PropertySlot::m_attributes is uninitialized in constructor. Found by clang static analyzer with optin.cplusplus.UninitializedObject checker enabled. 1647675 1 398128 ddkilzer 2020-04-30 17:39:04 -0700 Created attachment 398128 Patch v1 1647676 2 webkit-bug-importer 2020-04-30 17:40:15 -0700 <rdar://problem/62687958> 1647677 3 398128 mark.lam 2020-04-30 17:42:20 -0700 Comment on attachment 398128 Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=398128&action=review r=me > Source/JavaScriptCore/runtime/PropertySlot.h:404 > + } m_additionalData { { 0, 0 } }; Interesting. I never knew that we can do this. 1647681 4 398128 ddkilzer 2020-04-30 17:51:22 -0700 Comment on attachment 398128 Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=398128&action=review >> Source/JavaScriptCore/runtime/PropertySlot.h:404 >> + } m_additionalData { { 0, 0 } }; > > Interesting. I never knew that we can do this. I think it works because both union types have two instance variables. I guess gcc and MSVC++ will tell us if it's portable, though. 1647683 5 ddkilzer 2020-04-30 17:53:53 -0700 Regressed in: Bug 119972: Add attributes field to PropertySlot <https://bugs.webkit.org/show_bug.cgi?id=119972> <https://trac.webkit.org/r154253> 1647806 6 ews-feeder 2020-05-01 03:26:15 -0700 Committed r260993: <https://trac.webkit.org/changeset/260993> All reviewed patches have been landed. Closing bug and clearing flags on attachment 398128. 1649456 7 398128 darin 2020-05-05 18:16:46 -0700 Comment on attachment 398128 Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=398128&action=review >>> Source/JavaScriptCore/runtime/PropertySlot.h:404 >>> + } m_additionalData { { 0, 0 } }; >> >> Interesting. I never knew that we can do this. > > I think it works because both union types have two instance variables. I guess gcc and MSVC++ will tell us if it's portable, though. No, it’s only domAttribute that is initialized. I looked it up: "When a union is initialized by aggregate initialization, only its first non-static data member is initialized." 1654096 8 ddkilzer 2020-05-19 11:30:30 -0700 (In reply to Darin Adler from comment #7) > Comment on attachment 398128 [details] > Patch v1 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=398128&action=review > > >>> Source/JavaScriptCore/runtime/PropertySlot.h:404 > >>> + } m_additionalData { { 0, 0 } }; > >> > >> Interesting. I never knew that we can do this. > > > > I think it works because both union types have two instance variables. I guess gcc and MSVC++ will tell us if it's portable, though. > > No, it’s only domAttribute that is initialized. I looked it up: > > "When a union is initialized by aggregate initialization, only its first > non-static data member is initialized." Bug 212095: Make union initializers for JSC::PropertySlot more explicit about which field is being initialized 398128 2020-04-30 17:39:04 -0700 2020-05-01 03:26:15 -0700 Patch v1 bug-211267-20200430174148.patch text/plain 2790 ddkilzer U3VidmVyc2lvbiBSZXZpc2lvbjogMjYwOTcxCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBj NmQ2ZTY0ZWEyZjNkNmRiOGE3YTY3YTY3NTZkZGJmMTE0NTk4NTFlLi41MWQ5NDlmNDRkMzc5Y2Q0 Yjg4MzI2ZTgwMDY2NDhkNjI0Y2E5ZGI5IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNSBAQAorMjAyMC0wNC0zMCAgRGF2aWQgS2lsemVyICA8ZGRraWx6ZXJAYXBwbGUuY29t PgorCisgICAgICAgIEpTQzo6UHJvcGVydHlTbG90OjptX2F0dHJpYnV0ZXMgaXMgdW5pbml0aWFs aXplZCBpbiBjb25zdHJ1Y3RvcgorICAgICAgICA8aHR0cHM6Ly93ZWJraXQub3JnL2IvMjExMjY3 PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogcnVu dGltZS9Qcm9wZXJ0eVNsb3QuaDoKKyAgICAgICAgKEpTQzo6UHJvcGVydHlTbG90OjpQcm9wZXJ0 eVNsb3QpOgorICAgICAgICAtIEluaXRpYWxpemUgbV9hdHRyaWJ1dGVzIGFuZCBtX2FkZGl0aW9u YWxEYXRhLCBhbmQgbWFrZSB1c2Ugb2YKKyAgICAgICAgICBkZWZhdWx0IGluaXRpYWxpemVycy4K KwogMjAyMC0wNC0zMCAgRGV2aW4gUm91c3NvICA8ZHJvdXNzb0BhcHBsZS5jb20+CiAKICAgICAg ICAgV2ViS2l0LldlYkNvbnRlbnQgcHJvY2VzcyBjcmFzaGVzIHdoZW4gd2ViIGRldmVsb3BlciB0 b29scyBhcmUgb3BlbmVkIGluIFNhZmFyaQpkaWZmIC0tZ2l0IGEvU291cmNlL0phdmFTY3JpcHRD b3JlL3J1bnRpbWUvUHJvcGVydHlTbG90LmggYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGlt ZS9Qcm9wZXJ0eVNsb3QuaAppbmRleCBmNzc2YzYyYTA1MmUzYTIxMmZhYzIyOGRiOTZlZWVjMTRj YzI5MDRmLi5mZTRjZDcwYWE1YTA4MjAxOTY0NjMwMzY3YmQ0M2Q4YWU1ZDZjYmVkIDEwMDY0NAot LS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9Qcm9wZXJ0eVNsb3QuaAorKysgYi9T b3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9Qcm9wZXJ0eVNsb3QuaApAQCAtMTEzLDE1ICsx MTMsOCBAQCBwdWJsaWM6CiAgICAgfTsKIAogICAgIGV4cGxpY2l0IFByb3BlcnR5U2xvdChjb25z dCBKU1ZhbHVlIHRoaXNWYWx1ZSwgSW50ZXJuYWxNZXRob2RUeXBlIGludGVybmFsTWV0aG9kVHlw ZSkKLSAgICAgICAgOiBtX29mZnNldChpbnZhbGlkT2Zmc2V0KQotICAgICAgICAsIG1fdGhpc1Zh bHVlKHRoaXNWYWx1ZSkKLSAgICAgICAgLCBtX3Nsb3RCYXNlKG51bGxwdHIpCi0gICAgICAgICwg bV93YXRjaHBvaW50U2V0KG51bGxwdHIpCi0gICAgICAgICwgbV9jYWNoZWFiaWxpdHkoQ2FjaGlu Z0Rpc2FsbG93ZWQpCi0gICAgICAgICwgbV9wcm9wZXJ0eVR5cGUoVHlwZVVuc2V0KQorICAgICAg ICA6IG1fdGhpc1ZhbHVlKHRoaXNWYWx1ZSkKICAgICAgICAgLCBtX2ludGVybmFsTWV0aG9kVHlw ZShpbnRlcm5hbE1ldGhvZFR5cGUpCi0gICAgICAgICwgbV9hZGRpdGlvbmFsRGF0YVR5cGUoQWRk aXRpb25hbERhdGFUeXBlOjpOb25lKQotICAgICAgICAsIG1faXNUYWludGVkQnlPcGFxdWVPYmpl Y3QoZmFsc2UpCiAgICAgewogICAgIH0KIApAQCAtMzk1LDIwICszODgsMjAgQEAgcHJpdmF0ZToK ICAgICAgICAgfSBjdXN0b21BY2Nlc3NvcjsKICAgICB9IG1fZGF0YTsKIAotICAgIHVuc2lnbmVk IG1fYXR0cmlidXRlczsKLSAgICBQcm9wZXJ0eU9mZnNldCBtX29mZnNldDsKKyAgICB1bnNpZ25l ZCBtX2F0dHJpYnV0ZXMgeyAwIH07CisgICAgUHJvcGVydHlPZmZzZXQgbV9vZmZzZXQgeyBpbnZh bGlkT2Zmc2V0IH07CiAgICAgSlNWYWx1ZSBtX3RoaXNWYWx1ZTsKLSAgICBKU09iamVjdCogbV9z bG90QmFzZTsKLSAgICBXYXRjaHBvaW50U2V0KiBtX3dhdGNocG9pbnRTZXQ7Ci0gICAgQ2FjaGVh YmlsaXR5VHlwZSBtX2NhY2hlYWJpbGl0eTsKLSAgICBQcm9wZXJ0eVR5cGUgbV9wcm9wZXJ0eVR5 cGU7CisgICAgSlNPYmplY3QqIG1fc2xvdEJhc2UgeyBudWxscHRyIH07CisgICAgV2F0Y2hwb2lu dFNldCogbV93YXRjaHBvaW50U2V0IHsgbnVsbHB0ciB9OworICAgIENhY2hlYWJpbGl0eVR5cGUg bV9jYWNoZWFiaWxpdHkgeyBDYWNoaW5nRGlzYWxsb3dlZCB9OworICAgIFByb3BlcnR5VHlwZSBt X3Byb3BlcnR5VHlwZSB7IFR5cGVVbnNldCB9OwogICAgIEludGVybmFsTWV0aG9kVHlwZSBtX2lu dGVybmFsTWV0aG9kVHlwZTsKLSAgICBBZGRpdGlvbmFsRGF0YVR5cGUgbV9hZGRpdGlvbmFsRGF0 YVR5cGU7Ci0gICAgYm9vbCBtX2lzVGFpbnRlZEJ5T3BhcXVlT2JqZWN0OworICAgIEFkZGl0aW9u YWxEYXRhVHlwZSBtX2FkZGl0aW9uYWxEYXRhVHlwZSB7IEFkZGl0aW9uYWxEYXRhVHlwZTo6Tm9u ZSB9OworICAgIGJvb2wgbV9pc1RhaW50ZWRCeU9wYXF1ZU9iamVjdCB7IGZhbHNlIH07CiAgICAg dW5pb24gewogICAgICAgICBET01BdHRyaWJ1dGVBbm5vdGF0aW9uIGRvbUF0dHJpYnV0ZTsKICAg ICAgICAgTW9kdWxlTmFtZXNwYWNlU2xvdCBtb2R1bGVOYW1lc3BhY2VTbG90OwotICAgIH0gbV9h ZGRpdGlvbmFsRGF0YTsKKyAgICB9IG1fYWRkaXRpb25hbERhdGEgeyB7IDAsIDAgfSB9OwogfTsK IAogQUxXQVlTX0lOTElORSBKU1ZhbHVlIFByb3BlcnR5U2xvdDo6Z2V0VmFsdWUoSlNHbG9iYWxP YmplY3QqIGdsb2JhbE9iamVjdCwgUHJvcGVydHlOYW1lIHByb3BlcnR5TmFtZSkgY29uc3QK