Closed Bug 763313 Opened 12 years ago Closed 12 years ago

Braceless functions have their destructuring-args bytecode duplicated

Categories

(Core :: JavaScript Engine, defect)

x86_64
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla16

People

(Reporter: jruderman, Assigned: Benjamin)

References

Details

(Keywords: regression, testcase)

Attachments

(1 file)

js> (function ([]) 1)
(function ([]) let [] = arguments[0];1)

jsfunfuzz noticed this bug because it makes decompilation introduce a syntax error in some contexts:

js> (function () {for (var x in (function ([]) 1)) {}})
(function () {for (var x in (function ([]) let [] = arguments[0];1)) {}})
js> dis(function ([]) 1)   
flags: LAMBDA EXPR_CLOSURE NULL_CLOSURE
loc     op
-----   --
main:
00000:  getarg 0
00003:  dup
00004:  pop
00005:  pop
00006:  getarg 0
00009:  dup
00010:  pop
00011:  pop
00012:  one
00013:  return
00014:  stop

Source notes:
 ofs  line    pc  delta desc     args
---- ---- ----- ------ -------- ------
  0:   14     3 [   3] decl     offset 2
  2:   14     9 [   6] decl     offset 2
Regression from bug 759498.
Attached patch fixSplinter Review
Assignee: general → bpeterson
Attachment #631769 - Flags: review?(jorendorff)
Comment on attachment 631769 [details] [diff] [review]
fix

Nice work. I dig the test.

Feel free to rename PNK_SEQ to something else. (If you want to clean this up more I'd be happy to review, but that's totally optional.)

Is PNX_DESTRUCT always set in this case? Can you assert it instead of checking?
Attachment #631769 - Flags: review?(jorendorff) → review+
(In reply to Jason Orendorff [:jorendorff] from comment #4)
> Comment on attachment 631769 [details] [diff] [review]
> fix
> 
> Nice work. I dig the test.
> 
> Feel free to rename PNK_SEQ to something else. (If you want to clean this up
> more I'd be happy to review, but that's totally optional.)

What would I rename it to? I think PNK_SEQ is supposed to be a generic sequence of statements. It's also used somewhere in for loops, too.

> 
> Is PNX_DESTRUCT always set in this case? Can you assert it instead of
> checking?

No, PNK_SEQ appears in for loops, too.
https://hg.mozilla.org/integration/mozilla-inbound/rev/2bcd13a8efb3
Flags: in-testsuite+
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/2bcd13a8efb3
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: