144474 2015-04-30 15:21:54 -0700 DOM bindings should not be using a reference type to point to a temporary object 2015-05-01 12:15:33 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 oliver oliver ap bdakin darin oldest_to_newest 1090597 0 oliver 2015-04-30 15:21:54 -0700 DOM bindings should not be using a reference type to point to a temporary object 1090601 1 252096 oliver 2015-04-30 15:27:16 -0700 Created attachment 252096 Patch 1090605 2 oliver 2015-04-30 15:41:44 -0700 Committed r183648: <http://trac.webkit.org/changeset/183648> 1090663 3 ap 2015-04-30 17:06:50 -0700 This broke bindings tests (need new results landed). I'm not sure if analysis in this patch is accurate, references sometimes do extend the lifetime of an object, even though that's counter-intuitive. 1090771 4 darin 2015-05-01 07:23:31 -0700 I don’t think this patch is correct; I would like more information about the problem here, and how the problem was solved. Alexey’s point is one possible source of misunderstanding that I am suspecting. 1090772 5 darin 2015-05-01 07:24:28 -0700 I’m particularly concerned about the change to %nativeType. I believe the change to the local variable pointing to existing_name is unnecessary but harmless. 1090779 6 oliver 2015-05-01 07:56:52 -0700 native type is used to define the type used for a local, the bug here occurs when we assign a temporary to a local reference. It does result in incorrect behavior and this trivially provable by making refptr clear the pointer reference in its destructor. The outcome is a huge number of tests failing courtesy of references to dead refptrs 1090847 7 ap 2015-05-01 12:15:33 -0700 Here is the change that this patch made in generated code: <https://build.webkit.org/builders/Apple%20Yosemite%20Release%20WK2%20%28Tests%29/builds/4924/steps/bindings-generation-tests/logs/stdio>. The reference used to be initialized from a ternary operator, which complicates things. Oliver told me that he is working on a minimal C++ test demonstrating that a reference doesn't extend object lifetime in this case (either correctly, or due to a clang bug). 252096 2015-04-30 15:27:16 -0700 2015-04-30 15:29:40 -0700 Patch bug-144474-20150430152605.patch text/plain 3008 oliver U3VidmVyc2lvbiBSZXZpc2lvbjogMTgzNjQ1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZTUxNTk1NDYyMTk0ODA3 ZmI0ZTA0ZjQyYmJmNzcxNDc5YmZlYTZiNy4uNTk5ZjkyYzY2NzQwZTY2NTkzYjdlMmM2YTdkNDlk OGYwZTQ5MjI0MCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDE1LTA0LTMwICBPbGl2 ZXIgSHVudCAgPG9saXZlckBhcHBsZS5jb20+CisKKyAgICAgICAgRE9NIGJpbmRpbmdzIHNob3Vs ZCBub3QgYmUgdXNpbmcgYSByZWZlcmVuY2UgdHlwZSB0byBwb2ludCB0byBhIHRlbXBvcmFyeSBv YmplY3QKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE0 NDQ3NAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRo ZSBET00gYmluZGluZ3Mgd2lsbCBjdXJyZW50bHkgdHJ5IGFuZCB1c2UgYSBsb2NhbCByZWZlcmVu Y2UgdG8gcG9pbnQKKyAgICAgICAgdG8gYSB0ZW1wb3Jhcnkgb2JqZWN0LiBUaGlzIGN1cnJlbnRs eSB3b3JrcyBhcyBhIGJ5IHByb2R1Y3Qgb2YgdGhlIGNvbXBpbGVyJ3MKKyAgICAgICAgc3RhY2sg bGF5b3V0LiBUaGlzIHBhdGNoIHJlbW92ZXMgdGhlIGRlcGVuZGVuY3kgb24gdW5kZWZpbmVkIGJl aGF2aW91cgorICAgICAgICBieSBlbnN1cmluZyB0aGF0IHdlIHVzZSBhIHZhbHVlIHJhdGhlciB0 aGFuIHJlZmVyZW5jZSB0eXBlLgorCisgICAgICAgICogYmluZGluZ3Mvc2NyaXB0cy9Db2RlR2Vu ZXJhdG9ySlMucG06CisgICAgICAgIChHZW5lcmF0ZVBhcmFtZXRlcnNDaGVjayk6CisgICAgICAg IChHZXROYXRpdmVUeXBlRm9yQ2FsbGJhY2tzKToKKwogMjAxNS0wNC0zMCAgQmV0aCBEYWtpbiAg PGJkYWtpbkBhcHBsZS5jb20+CiAKICAgICAgICAgU2hvdWxkIGNob29zZSBVSVNjcm9sbFZpZXcg aW5kaWNhdG9yU3R5bGUgYmFzZWQgb24gdGhlIGRvY3VtZW50IGJhY2tncm91bmQgY29sb3IKZGlm ZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL3NjcmlwdHMvQ29kZUdlbmVyYXRvckpT LnBtIGIvU291cmNlL1dlYkNvcmUvYmluZGluZ3Mvc2NyaXB0cy9Db2RlR2VuZXJhdG9ySlMucG0K aW5kZXggMTZhZjg4OTFlYjk1NjM5ZWQyZjk0M2ZhNzk2Yjk4Y2VhNmQ5NDk2Ny4uM2JkODRkMWVm ZDBhNGQyODBjNjE4N2YyNDg5NjVmMDZiM2RjN2VkNCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNv cmUvYmluZGluZ3Mvc2NyaXB0cy9Db2RlR2VuZXJhdG9ySlMucG0KKysrIGIvU291cmNlL1dlYkNv cmUvYmluZGluZ3Mvc2NyaXB0cy9Db2RlR2VuZXJhdG9ySlMucG0KQEAgLTMzMDMsNyArMzMwMyw3 IEBAIHN1YiBHZW5lcmF0ZVBhcmFtZXRlcnNDaGVjawogICAgICAgICAgICAgICAgIHB1c2goQCRv dXRwdXRBcnJheSwgIiAgICBBdG9taWNTdHJpbmdJbXBsKiBleGlzdGluZ18kbmFtZSA9IGV4ZWMt PmFyZ3VtZW50KCRhcmdzSW5kZXgpLmlzRW1wdHkoKSA/IG51bGxwdHIgOiBleGVjLT5hcmd1bWVu dCgkYXJnc0luZGV4KS50b1N0cmluZyhleGVjKS0+dG9FeGlzdGluZ0F0b21pY1N0cmluZyhleGVj KTtcbiIpOwogICAgICAgICAgICAgICAgIHB1c2goQCRvdXRwdXRBcnJheSwgIiAgICBpZiAoIWV4 aXN0aW5nXyRuYW1lKVxuIik7CiAgICAgICAgICAgICAgICAgcHVzaChAJG91dHB1dEFycmF5LCAi ICAgICAgICByZXR1cm4gSlNWYWx1ZTo6ZW5jb2RlKGpzTnVsbCgpKTtcbiIpOwotICAgICAgICAg ICAgICAgIHB1c2goQCRvdXRwdXRBcnJheSwgIiAgICBjb25zdCBBdG9taWNTdHJpbmcmICRuYW1l KGV4aXN0aW5nXyRuYW1lKTtcbiIpOworICAgICAgICAgICAgICAgIHB1c2goQCRvdXRwdXRBcnJh eSwgIiAgICBjb25zdCBBdG9taWNTdHJpbmcgJG5hbWUoZXhpc3RpbmdfJG5hbWUpO1xuIik7CiAg ICAgICAgICAgICB9IGVsc2UgewogICAgICAgICAgICAgICAgIHB1c2goQCRvdXRwdXRBcnJheSwg IiAgICAiIC4gR2V0TmF0aXZlVHlwZUZyb21TaWduYXR1cmUoJHBhcmFtZXRlcikgLiAiICRuYW1l KCIgLiBKU1ZhbHVlVG9OYXRpdmUoJHBhcmFtZXRlciwgJG9wdGlvbmFsICYmICRkZWZhdWx0QXR0 cmlidXRlICYmICRkZWZhdWx0QXR0cmlidXRlIGVxICJOdWxsU3RyaW5nIiA/ICJhcmd1bWVudE9y TnVsbChleGVjLCAkYXJnc0luZGV4KSIgOiAiZXhlYy0+YXJndW1lbnQoJGFyZ3NJbmRleCkiLCAk ZnVuY3Rpb24tPnNpZ25hdHVyZS0+ZXh0ZW5kZWRBdHRyaWJ1dGVzLT57IkNvbmRpdGlvbmFsIn0p IC4gIik7XG4iKTsKICAgICAgICAgICAgIH0KQEAgLTM2NDQsNyArMzY0NCw3IEBAIHN1YiBHZXRO YXRpdmVUeXBlRnJvbVNpZ25hdHVyZQogCiBteSAlbmF0aXZlVHlwZSA9ICgKICAgICAiQ29tcGFy ZUhvdyIgPT4gIlJhbmdlOjpDb21wYXJlSG93IiwKLSAgICAiRE9NU3RyaW5nIiA9PiAiY29uc3Qg U3RyaW5nJiIsCisgICAgIkRPTVN0cmluZyIgPT4gImNvbnN0IFN0cmluZyIsCiAgICAgIk5vZGVG aWx0ZXIiID0+ICJSZWZQdHI8Tm9kZUZpbHRlcj4iLAogICAgICJTZXJpYWxpemVkU2NyaXB0VmFs dWUiID0+ICJSZWZQdHI8U2VyaWFsaXplZFNjcmlwdFZhbHVlPiIsCiAgICAgIkRhdGUiID0+ICJk b3VibGUiLApAQCAtMzcwNCw2ICszNzA0LDcgQEAgc3ViIEdldE5hdGl2ZVR5cGVGb3JDYWxsYmFj a3MKICAgICBteSAkdHlwZSA9IHNoaWZ0OwogICAgIHJldHVybiAiUGFzc1JlZlB0cjxTZXJpYWxp emVkU2NyaXB0VmFsdWU+IiBpZiAkdHlwZSBlcSAiU2VyaWFsaXplZFNjcmlwdFZhbHVlIjsKICAg ICByZXR1cm4gIlBhc3NSZWZQdHI8RE9NU3RyaW5nTGlzdD4iIGlmICR0eXBlIGVxICJET01TdHJp bmdMaXN0IjsKKyAgICByZXR1cm4gImNvbnN0IFN0cmluZyYiIGlmICR0eXBlIGVxICJET01TdHJp bmciOwogCiAgICAgcmV0dXJuIEdldE5hdGl2ZVR5cGUoJHR5cGUpOwogfQo=