47055 2010-10-03 01:03:50 -0700 All the WebSocket tests crash 2010-10-03 11:47:42 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Other OS X 10.5 RESOLVED FIXED P2 Normal --- 1 abarth abarth abarth andersca ap darin ukai zecke oldest_to_newest 288746 0 abarth 2010-10-03 01:03:50 -0700 All the WebSocket tests crash 288747 1 69583 abarth 2010-10-03 01:05:48 -0700 Created attachment 69583 Patch 288748 2 69583 zecke 2010-10-03 01:13:32 -0700 Comment on attachment 69583 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=69583&action=review You could keep the 'const'. The patch looks sane. > WebCore/ChangeLog:9 > + That doesn't work in C++. Instead, we need to actually store the You could remove the extra space. 288749 3 abarth 2010-10-03 01:19:50 -0700 Committed r68984: <http://trac.webkit.org/changeset/68984> 288770 4 darin 2010-10-03 10:37:36 -0700 The C++ standard, section 12.5, paragraph 5, says: "The temporary to which the reference is bound [...] persists for the lifetime of the reference except as specified below.” And the text below does not list any exception which applies here. So the temporary objects here will last the lifetime of the reference. So this code change should not have had any effect at all. 288774 5 ap 2010-10-03 11:41:43 -0700 This is a fairly common idiom in WebCore code. People have submitted patches to "fix" such code several times in the past, but reviewers stopped that before, and the problem always turned out to be something different. When did this start? Did this happen on buildbots? 288777 6 zecke 2010-10-03 11:47:42 -0700 (In reply to comment #5) > This is a fairly common idiom in WebCore code. People have submitted patches to "fix" such code several times in the past, but reviewers stopped that before, and the problem always turned out to be something different. I was not aware that it is valid until the end of the block. I was doing this example http://paste.lisp.org/display/115156 for adam to show that it is at least valid for function calls. The other thing is, I think it is to be preferred to not use const reference as it can have suprising effects and as long as the function being called is inline, gcc will be able to eliminate the copy (since early gcc 3.x), as can be seen when compiling/executing the example. Anyway, I will be more careful. > > When did this start? Did this happen on buildbots? Leaving that to Adam. 69583 2010-10-03 01:05:48 -0700 2010-10-03 01:13:32 -0700 Patch bug-47055-20101003010546.patch text/plain 1681 abarth SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA2ODk4MykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTcgQEAKKzIwMTAtMTAtMDMgIEFkYW0gQmFydGggIDxhYmFydGhAd2Via2l0Lm9y Zz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBBbGwg dGhlIFdlYlNvY2tldCB0ZXN0cyBjcmFzaAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9y Zy9zaG93X2J1Zy5jZ2k/aWQ9NDcwNTUKKworICAgICAgICBUaGlzIGNvZGUgYXR0ZW1wdHMgdG8g aG9sZCBvbnRvIHRlbXBvcmFyeSBvYmplY3RzIHVzaW5nIHJlZmVyZW5jZXMuCisgICAgICAgIFRo YXQgZG9lc24ndCB3b3JrIGluIEMrKy4gIEluc3RlYWQsIHdlIG5lZWQgdG8gYWN0dWFsbHkgc3Rv cmUgdGhlCisgICAgICAgIG9iamVjdHMgc29tZXdoZXJlLgorCisgICAgICAgICogYmluZGluZ3Mv anMvSlNXZWJTb2NrZXRDdXN0b20uY3BwOgorICAgICAgICAoV2ViQ29yZTo6SlNXZWJTb2NrZXRD b25zdHJ1Y3Rvcjo6Y29uc3RydWN0SlNXZWJTb2NrZXQpOgorCiAyMDEwLTEwLTAyICBEaWVnbyBH b256YWxleiAgPGRpZWdvaGNnQHdlYmtpdC5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgS2Vu bmV0aCBSb2hkZSBDaHJpc3RpYW5zZW4uCkluZGV4OiBXZWJDb3JlL2JpbmRpbmdzL2pzL0pTV2Vi U29ja2V0Q3VzdG9tLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL2JpbmRpbmdzL2pzL0pTV2Vi U29ja2V0Q3VzdG9tLmNwcAkocmV2aXNpb24gNjg5ODMpCisrKyBXZWJDb3JlL2JpbmRpbmdzL2pz L0pTV2ViU29ja2V0Q3VzdG9tLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNTQsMTAgKzU0LDEwIEBA IEVuY29kZWRKU1ZhbHVlIEpTQ19IT1NUX0NBTEwgSlNXZWJTb2NrZXQKICAgICBpZiAoIWV4ZWMt PmFyZ3VtZW50Q291bnQoKSkKICAgICAgICAgcmV0dXJuIHRocm93Vk1FcnJvcihleGVjLCBjcmVh dGVTeW50YXhFcnJvcihleGVjLCAiTm90IGVub3VnaCBhcmd1bWVudHMiKSk7CiAKLSAgICBjb25z dCBTdHJpbmcmIHVybFN0cmluZyA9IHVzdHJpbmdUb1N0cmluZyhleGVjLT5hcmd1bWVudCgwKS50 b1N0cmluZyhleGVjKSk7CisgICAgU3RyaW5nIHVybFN0cmluZyA9IHVzdHJpbmdUb1N0cmluZyhl eGVjLT5hcmd1bWVudCgwKS50b1N0cmluZyhleGVjKSk7CiAgICAgaWYgKGV4ZWMtPmhhZEV4Y2Vw dGlvbigpKQogICAgICAgICByZXR1cm4gdGhyb3dWTUVycm9yKGV4ZWMsIGNyZWF0ZVN5bnRheEVy cm9yKGV4ZWMsICJ3cm9uZyBVUkwiKSk7Ci0gICAgY29uc3QgS1VSTCYgdXJsID0gY29udGV4dC0+ Y29tcGxldGVVUkwodXJsU3RyaW5nKTsKKyAgICBLVVJMIHVybCA9IGNvbnRleHQtPmNvbXBsZXRl VVJMKHVybFN0cmluZyk7CiAgICAgUmVmUHRyPFdlYlNvY2tldD4gd2ViU29ja2V0ID0gV2ViU29j a2V0OjpjcmVhdGUoY29udGV4dCk7CiAgICAgRXhjZXB0aW9uQ29kZSBlYyA9IDA7CiAgICAgaWYg KGV4ZWMtPmFyZ3VtZW50Q291bnQoKSA8IDIpCg==