237185 2022-02-24 23:19:02 -0800 Remove incorrect ASSERT. 2022-02-25 15:38:42 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 mark.lam mark.lam ews-watchlist keith_miller msaboff saam tzagallo webkit-bug-importer ysuzuki oldest_to_newest 1845915 0 mark.lam 2022-02-24 23:19:02 -0800 <rdar://problem/83902782> 1845919 1 453178 mark.lam 2022-02-24 23:40:10 -0800 Created attachment 453178 proposed patch. 1845964 2 453178 keith_miller 2022-02-25 04:08:18 -0800 Comment on attachment 453178 proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=453178&action=review > Source/JavaScriptCore/ChangeLog:10 > + weren't sure that the StringImpl pointer can be null or not. We're now certain We should explain why/how the StringImpl can be null. 1846202 3 mark.lam 2022-02-25 15:11:14 -0800 (In reply to Keith Miller from comment #2) > Comment on attachment 453178 [details] > proposed patch. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=453178&action=review > > > Source/JavaScriptCore/ChangeLog:10 > > + weren't sure that the StringImpl pointer can be null or not. We're now certain > > We should explain why/how the StringImpl can be null. The ASSERT is from here: ``` auto* impl = string->tryGetValueImpl(); ASSERT(impl); // FIXME: rdar://83902782 if (impl && impl->isAtom() && ... ``` ... where string is a JSString, which can also be a JSRopeString. JSString::tryGetValueImpl() is: ``` inline const StringImpl* JSString::tryGetValueImpl() const { uintptr_t pointer = fiberConcurrently(); if (pointer & isRopeInPointer) return nullptr; return bitwise_cast<StringImpl*>(pointer); } ``` Hence, if string is a JSRopeString, the implication can be null. 1846208 4 mark.lam 2022-02-25 15:17:40 -0800 > Hence, if string is a JSRopeString, the implication can be null. /implication/impl/ ... sigh auto-correct. Anyway, I'll add this info to the ChangeLog before landing. 1846209 5 mark.lam 2022-02-25 15:20:42 -0800 Landed in r290530: <http://trac.webkit.org/r290530>. 453178 2022-02-24 23:40:10 -0800 2022-02-25 15:38:42 -0800 proposed patch. bug-237185.patch text/plain 2485 mark.lam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjkwNDkzKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIxIEBA CisyMDIyLTAyLTI0ICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBS ZW1vdmUgaW5jb3JyZWN0IEFTU0VSVC4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcv c2hvd19idWcuY2dpP2lkPTIzNzE4NQorICAgICAgICA8cmRhcjovL3Byb2JsZW0vODM5MDI3ODI+ CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgVGhlc2Ug ZGVidWcgQVNTRVJUcyB3ZXJlIGFkZGVkIGluIGh0dHA6Ly90cmFjLndlYmtpdC5vcmcvcjI4MzYz MiBiYWNrIHdoZW4gd2UKKyAgICAgICAgd2VyZW4ndCBzdXJlIHRoYXQgdGhlIFN0cmluZ0ltcGwg cG9pbnRlciBjYW4gYmUgbnVsbCBvciBub3QuICBXZSdyZSBub3cgY2VydGFpbgorICAgICAgICB0 aGF0IHRoZSBTdHJpbmdJbXBsIHBvaW50ZXIgY2FuIGJlIG51bGwsIGFuZCB0aGF0IHRoZSBudWxs IGNoZWNrcyB0aGF0IHdlIGFkZGVkCisgICAgICAgIGluIHRoYXQgcGF0Y2ggaXMgdGhlIGNvcnJl Y3Qgd2F5IHRvIGhhbmRsZSBpdC4gIEhlbmNlLCB0aGUgQVNTRVJUcyBhcmUgaW52YWxpZCBhbmQK KyAgICAgICAgc2hvdWxkIGJlIHJlbW92ZWQuCisKKyAgICAgICAgKiBkZmcvREZHQnl0ZUNvZGVQ YXJzZXIuY3BwOgorICAgICAgICAoSlNDOjpERkc6OkJ5dGVDb2RlUGFyc2VyOjpwYXJzZUJsb2Nr KToKKyAgICAgICAgKEpTQzo6REZHOjpCeXRlQ29kZVBhcnNlcjo6aGFuZGxlUHV0QnlWYWwpOgor CiAyMDIyLTAyLTI0ICBDaHJpcyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CiAKICAgICAgICAg W0NvY29hXSBPbmx5IGNsZWFyIElDVSBjYWNoZSB3aGVuIHRpbWUgem9uZSBpcyBjaGFuZ2VkCklu ZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RGR0J5dGVDb2RlUGFyc2VyLmNwcAo9PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RGR0J5dGVDb2RlUGFyc2VyLmNw cAkocmV2aXNpb24gMjkwNDkzKQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdCeXRl Q29kZVBhcnNlci5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTY1MDIsNyArNjUwMiw2IEBAIHZvaWQg Qnl0ZUNvZGVQYXJzZXI6OnBhcnNlQmxvY2sodW5zaWduZWQKICAgICAgICAgICAgICAgICAgICAg ICAgIGFkZFRvR3JhcGgoQ2hlY2tJc0NvbnN0YW50LCBPcEluZm8oZnJvemVuKSwgcHJvcGVydHkp OwogICAgICAgICAgICAgICAgICAgICB9IGVsc2UgaWYgKGF1dG8qIHN0cmluZyA9IHByb3BlcnR5 LT5keW5hbWljQ2FzdENvbnN0YW50PEpTU3RyaW5nKj4oKm1fdm0pKSB7CiAgICAgICAgICAgICAg ICAgICAgICAgICBhdXRvKiBpbXBsID0gc3RyaW5nLT50cnlHZXRWYWx1ZUltcGwoKTsKLSAgICAg ICAgICAgICAgICAgICAgICAgIEFTU0VSVChpbXBsKTsgLy8gRklYTUU6IHJkYXI6Ly84MzkwMjc4 MgogICAgICAgICAgICAgICAgICAgICAgICAgaWYgKGltcGwgJiYgaW1wbC0+aXNBdG9tKCkgJiYg IXBhcnNlSW5kZXgoKmNvbnN0X2Nhc3Q8U3RyaW5nSW1wbCo+KGltcGwpKSkgewogICAgICAgICAg ICAgICAgICAgICAgICAgICAgIHVpZCA9IGJpdHdpc2VfY2FzdDxVbmlxdWVkU3RyaW5nSW1wbCo+ KGltcGwpOwogICAgICAgICAgICAgICAgICAgICAgICAgICAgIHByb3BlcnR5Q2VsbCA9IHN0cmlu ZzsKQEAgLTg4OTksNyArODg5OCw2IEBAIHZvaWQgQnl0ZUNvZGVQYXJzZXI6OmhhbmRsZVB1dEJ5 VmFsKEJ5dGUKICAgICAgICAgICAgICAgICBhZGRUb0dyYXBoKENoZWNrSXNDb25zdGFudCwgT3BJ bmZvKGZyb3plbiksIHByb3BlcnR5KTsKICAgICAgICAgICAgIH0gZWxzZSBpZiAoYXV0byogc3Ry aW5nID0gcHJvcGVydHktPmR5bmFtaWNDYXN0Q29uc3RhbnQ8SlNTdHJpbmcqPigqbV92bSkpIHsK ICAgICAgICAgICAgICAgICBhdXRvKiBpbXBsID0gc3RyaW5nLT50cnlHZXRWYWx1ZUltcGwoKTsK LSAgICAgICAgICAgICAgICBBU1NFUlQoaW1wbCk7IC8vIEZJWE1FOiByZGFyOi8vODM5MDI3ODIK ICAgICAgICAgICAgICAgICBpZiAoaW1wbCAmJiBpbXBsLT5pc0F0b20oKSAmJiAhcGFyc2VJbmRl eCgqY29uc3RfY2FzdDxTdHJpbmdJbXBsKj4oaW1wbCkpKSB7CiAgICAgICAgICAgICAgICAgICAg IHVpZCA9IGJpdHdpc2VfY2FzdDxVbmlxdWVkU3RyaW5nSW1wbCo+KGltcGwpOwogICAgICAgICAg ICAgICAgICAgICBwcm9wZXJ0eUNlbGwgPSBzdHJpbmc7Cg==