62836 2011-06-16 18:10:59 -0700 REGRESSION (r89035): HTMLTableRowsCollection constructor crashes; depends on compiler order of evaluation 2011-06-17 09:25:49 -0700 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P1 Normal --- 1 darin darin ap oldest_to_newest 422368 0 darin 2011-06-16 18:10:59 -0700 This code is wrong: HTMLTableRowsCollection::HTMLTableRowsCollection(PassRefPtr<HTMLTableElement> table) : HTMLCollection(table, OtherCollection, table->collectionCache()) { } If the first argument is evaluated first, then the second may dereference null because of the semantics of PassRefPtr. The fix is simple. 422372 1 97533 darin 2011-06-16 18:15:16 -0700 Created attachment 97533 Patch 422373 2 darin 2011-06-16 18:17:04 -0700 Committed r89096: <http://trac.webkit.org/changeset/89096> 422421 3 ap 2011-06-16 22:40:31 -0700 Should the HTMLTableRowsCollection constructor just be changed to take a raw pointer? 422636 4 darin 2011-06-17 08:20:02 -0700 (In reply to comment #3) > Should the HTMLTableRowsCollection constructor just be changed to take a raw pointer? Since the function does take ownership of a reference, the PassRefPtr optimization works, so I think it's good to take a smart pointer, even though in this case we can’t take advantage of it. 422650 5 ap 2011-06-17 09:00:10 -0700 The only caller of HTMLTableRowsCollection::create() passes "this" to this function, and I don't see how any caller could possibly pass ownership of HTMLTableElement to HTMLTableRowsCollection. 422658 6 darin 2011-06-17 09:25:49 -0700 (In reply to comment #5) > The only caller of HTMLTableRowsCollection::create() passes "this" to this function, and I don't see how any caller could possibly pass ownership of HTMLTableElement to HTMLTableRowsCollection. RefPtr implements a shared ownership model. The function does take ownership. On the other hand, if nobody ever takes advantage of it, I think it’s fine to use raw pointers instead. 97533 2011-06-16 18:15:16 -0700 2011-06-16 18:16:32 -0700 Patch bug-62836-20110616181514.patch text/plain 1719 darin SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDg5MDk1KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTEtMDYtMTYgIERhcmluIEFk bGVyICA8ZGFyaW5AYXBwbGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09Q UyEpLgorCisgICAgICAgIFJFR1JFU1NJT04gKHI4OTAzNSk6IEhUTUxUYWJsZVJvd3NDb2xsZWN0 aW9uIGNvbnN0cnVjdG9yIGNyYXNoZXM7IGRlcGVuZHMgb24gY29tcGlsZXIgb3JkZXIgb2YgZXZh bHVhdGlvbgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 NjI4MzYKKworICAgICAgICBDb3ZlcmVkIGJ5IGV4aXN0aW5nIHRlc3RzIG9uIHBsYXRmb3JtcyB3 aGVyZSBpdCBvY2N1cnMuCisKKyAgICAgICAgKiBodG1sL0hUTUxUYWJsZVJvd3NDb2xsZWN0aW9u LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkhUTUxUYWJsZVJvd3NDb2xsZWN0aW9uOjpIVE1MVGFi bGVSb3dzQ29sbGVjdGlvbik6IEFkZCBhIGNhbGwgdG8gZ2V0KCkKKyAgICAgICAgdG8gcHJldmVu dCB0aGUgcGFzc2luZyBiZWhhdmlvciBvZiBQYXNzUmVmUHRyIGZyb20ga2lja2luZyBpbiBhbmQg Y2hhbmdpbmcgdGhlCisgICAgICAgIHRhYmxlIHBvaW50ZXIgdG8gMCwgcG9zc2libHkgYmVmb3Jl IGl0IGlzIGRlcmVmZXJlbmNlZC4KKwogMjAxMS0wNi0xNiAgU2hlcmlmZiBCb3QgIDx3ZWJraXQu cmV2aWV3LmJvdEBnbWFpbC5jb20+CiAKICAgICAgICAgVW5yZXZpZXdlZCwgcm9sbGluZyBvdXQg cjg5MDU1LgpJbmRleDogU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MVGFibGVSb3dzQ29sbGVjdGlv bi5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MVGFibGVSb3dzQ29s bGVjdGlvbi5jcHAJKHJldmlzaW9uIDg5MDk1KQorKysgU291cmNlL1dlYkNvcmUvaHRtbC9IVE1M VGFibGVSb3dzQ29sbGVjdGlvbi5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTE0OCw4ICsxNDgsMTEg QEAgSFRNTFRhYmxlUm93RWxlbWVudCogSFRNTFRhYmxlUm93c0NvbGxlYwogICAgIHJldHVybiAw OwogfQogCisvLyBNdXN0IGNhbGwgZ2V0KCkgb24gdGhlIHRhYmxlIGluIGNhc2UgdGhhdCBhcmd1 bWVudCBpcyBjb21waWxlZCBiZWZvcmUgZGVyZWZlcmVuY2luZyB0aGUKKy8vIHRhYmxlIHRvIGdl dCBhdCB0aGUgY29sbGVjdGlvbiBjYWNoZS4gT3JkZXIgb2YgYXJndW1lbnQgZXZhbHVhdGlvbiBp cyB1bmRlZmluZWQgYW5kIGNhbgorLy8gZGlmZmVyIGJldHdlZW4gY29tcGlsZXJzLgogSFRNTFRh YmxlUm93c0NvbGxlY3Rpb246OkhUTUxUYWJsZVJvd3NDb2xsZWN0aW9uKFBhc3NSZWZQdHI8SFRN TFRhYmxlRWxlbWVudD4gdGFibGUpCi0gICAgOiBIVE1MQ29sbGVjdGlvbih0YWJsZSwgT3RoZXJD b2xsZWN0aW9uLCB0YWJsZS0+Y29sbGVjdGlvbkNhY2hlKCkpCisgICAgOiBIVE1MQ29sbGVjdGlv bih0YWJsZS5nZXQoKSwgT3RoZXJDb2xsZWN0aW9uLCB0YWJsZS0+Y29sbGVjdGlvbkNhY2hlKCkp CiB7CiB9CiAK