Bug 70403

Summary:

bytecompiler sometimes generates incorrect bytecode for put_by_id

Product:

WebKit

Reporter:

Zheng Liu <zheng.z.liu>

Component:

JavaScriptCore

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, dbates, fpizlo, ggaren, oliver, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

Linux

Attachments:

Description	Flags
A simple patch to copy the rvalue before put.	none
Fix	none
Correct format.	none
Fix ChangeLog	ggaren: review-
Added test at LayoutTests/fast/js.	none

Zheng Liu

Reported 2011-10-19 01:34:37 PDT

Bytecompiler sometimes generates incorrect bytecode for put_by_id. case:(a same case of dot) function f(a,i,j) { a.__defineSetter__('x', function(v) { this['z']=v; i='CLOBBERED'; }); j['y']=(a['x']=i); print(j.y); }; f({},'RVAL',{}); expected: 'RVAL', got: 'CLOBBERED' Reason: [ 34] mov r4, "y"(@k1) [ 37] put_by_val r-9, "x"(@k0), r-8 [ 41] put_by_val r3, r4, r-8 r-8 is clobbered in this case. Expression (a['x']=i) should not forward r-8.

Attachments
A simple patch to copy the rvalue before put. (1.47 KB, patch) 2011-10-19 01:46 PDT, Zheng Liu	no flags	Details Formatted Diff Diff
Fix (1.57 KB, patch) 2011-10-19 05:16 PDT, Zheng Liu	no flags	Details Formatted Diff Diff
Correct format. (2.69 KB, patch) 2011-10-20 19:03 PDT, Zheng Liu	no flags	Details Formatted Diff Diff
Fix ChangeLog (2.41 KB, patch) 2011-10-20 19:52 PDT, Zheng Liu	ggaren: review-	Details Formatted Diff Diff
Added test at LayoutTests/fast/js. (4.66 KB, patch) 2011-10-21 00:58 PDT, Zheng Liu	no flags	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Zheng Liu

Comment 1 2011-10-19 01:46:32 PDT

Created attachment 111577 [details] A simple patch to copy the rvalue before put.

Zheng Liu

Comment 2 2011-10-19 05:16:44 PDT

Created attachment 111596 [details] Fix Don't copy when the result is to be ignored.

Zheng Liu

Comment 3 2011-10-20 19:03:04 PDT

Created attachment 111888 [details] Correct format.

Zheng Liu

Comment 4 2011-10-20 19:52:12 PDT

Created attachment 111891 [details] Fix ChangeLog

Filip Pizlo

Comment 5 2011-10-20 21:46:00 PDT

Great catch! Took me a while to see what was going on. Can you add a LayoutTest and include it in this patch?

Geoffrey Garen

Comment 6 2011-10-20 23:00:24 PDT

Comment on attachment 111891 [details] Fix ChangeLog r-, but this patch looks ready to go once it has a layout test. See http://www.webkit.org/quality/testwriting.html.

Zheng Liu

Comment 7 2011-10-21 00:58:58 PDT

Created attachment 111919 [details] Added test at LayoutTests/fast/js.

Filip Pizlo

Comment 8 2011-10-21 00:59:47 PDT

Comment on attachment 111919 [details] Added test at LayoutTests/fast/js. Looks great, r=me.

WebKit Review Bot

Comment 9 2011-10-21 02:10:34 PDT

Comment on attachment 111919 [details] Added test at LayoutTests/fast/js. Clearing flags on attachment: 111919 Committed r98091: <http://trac.webkit.org/changeset/98091>

WebKit Review Bot

Comment 10 2011-10-21 02:10:40 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.