Bug 18274 – ResolveNode::emitCode() doesn't make a new temporary when dst is 0, leading to incorrect codegen

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Cameron Zwarich (cpst)]

This JavaScript

var o = { a: function () { return 1; } };
o.a();
o;

generates the following code:

[   0] load		lr1, undefined(@k0)		
[   3] new_object	lr1
[   5] new_func_exp		tr0, f0
[   8] put_prop_id	lr1, a(@id0), tr0
[  12] get_prop_id	tr0, lr1, a(@id0)
[  16] call		lr1, tr0, lr1, 8, 1
[  22] end		lr1

The end call ends up overwriting the local variable o with 1, which is clearly incorrect. Obviously, we don't always want ResolveNode::emitCode() to return a new temporary register if dst is unspecified. However, there are two situations where this causes a bug:

1) The returned register is reused. This occurs in FunctionCallDotNode and FunctionCallBracketNode. I have a patch that fixes this particular problem, and I will post it for review. DoWhileNode and ForNode increment the reference count of the returned register, but only to return it, not to reuse it.

2) The returned register is ignored, requiring dst to be the actual target. This occurs in codegen for IfNode, IfElseNode, WhileNode, and ForInNode. It doesn't cause any realistic problems in any of them, because they are statements. The first three return 0 as it is, and ForInNode returns dst, which still isn't a problem if it doesn't contain the correct value, because nothing will use it once we have completions working.

Should we just accept this behaviour of emitNode and remember to code around it?

[Cameron Zwarich (cpst)]

Created attachment 20289 [details]
Proposed patch

[Oliver Hunt]

Landed r31555