Bug 144474 – DOM bindings should not be using a reference type to point to a temporary object

• Component: New Bugs
• Status: RESOLVED FIXED

[Oliver Hunt]

DOM bindings should not be using a reference type to point to a temporary object

[Oliver Hunt]

Created attachment 252096 [details]
Patch

[Oliver Hunt]

Committed r183648: <http://trac.webkit.org/changeset/183648>

[Alexey Proskuryakov]

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.

[Darin Adler]

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.

[Darin Adler]

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.

[Oliver Hunt]

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

[Alexey Proskuryakov]

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).