Braceless functions have their destructuring-args bytecode duplicated

RESOLVED FIXED in mozilla16

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Jesse Ruderman, Assigned: Benjamin)

Tracking

(Blocks: 1 bug, {regression, testcase})

Trunk
mozilla16
x86_64
Mac OS X
regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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)) {}})
(Reporter)

Comment 1

5 years ago
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
(Reporter)

Comment 2

5 years ago
Regression from bug 759498.
(Assignee)

Comment 3

5 years ago
Created attachment 631769 [details] [diff] [review]
fix
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+
(Assignee)

Comment 5

5 years ago
(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
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.