Closed Bug 530537 Opened 15 years ago Closed 15 years ago

Function.prototype.toString strips out parenthesis when displaying function

Categories

(Core :: JavaScript Engine, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
status1.9.2 --- beta5-fixed

People

(Reporter: morac, Assigned: mrbkap)

References

Details

(Keywords: regression, Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

When calling the toString() function for functions, parenthesis between strings are incorrectly stripped out of the resulting text.  This causes problems for add-ons or userChrome.js users who do use replace() and eval() on the text to change the functionality of a function or create new functions.

For example, type either of the following two lines into the error console:

a=function(b){dump("hi_"+(b?"yes":"no")+"_bye")};a.toString();
a=function(b){dump("hi_"+(b?"yes":"no")+"_bye")}


Under Firefox 3.5.5 the output is:

function (b) {
    dump("hi_" + (b ? "yes" : "no") + "_bye");
}


Under Namoroka Gecko/20091122 and Trunk Gecko/20091123 the output is:

function (b) {
    dump("hi_" + b ? "yes" : "no" + "_bye");
}


The output under Namoroka and the Trunk is wrong such that if the string is evaled back into a function the functionality changes.  In this example, the above code will result in either "yes" or "no" being dumped instead of "hi_yes_bye" or "hi_no_bye".



This problem only appears to occur if their is a string right before and after the parenthesis pair.  For example if I enter the following into the error console in Namoroka or Trunk, the results are correctly output:

a=function(b){dump("hi_"+(b?"yes":"no"))};

results in:

function (b) {
    dump("hi_" + (b ? "yes" : "no"));
}


a=function(b){dump((b?"yes":"no")+"_bye")};

results in:

function (b) {
    dump((b ? "yes" : "no") + "_bye");
}
Attached patch FixSplinter Review
This is fallout from the JSOP_CONCATN landing. We forgot to make the precedence in the opcode table equal to that of JSOP_ADD, so the decompiler didn't know that it has to add parentheses.

This is a really safe fix in that it only affects the number of parentheses the decompiler emits when dealing with chained addition. It should go on 1.9.2 if possible IMO, since we try pretty hard to make decompilation not change the meaning of a function.
Assignee: general → mrbkap
Status: NEW → ASSIGNED
Attachment #414056 - Flags: review?
Attachment #414056 - Flags: approval1.9.2?
Attachment #414056 - Flags: review? → review?(lw)
Comment on attachment 414056 [details] [diff] [review]
Fix

Thanks for findin' ma' bug!
Attachment #414056 - Flags: review?(lw) → review+
Flags: wanted1.9.2?
This needs to be fixed for 1.9.2.

/be
http://tinderbox.mozilla.org/showlog.cgi?log=TraceMonkey-Unittest/1259000375.1259003094.13212.gz

s: moz2-darwin9-slave18REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/tracemonkey-macosx-unittest-everythingelse/build/jsreftest/tests/jsreftest.html?test=js1_5/decompilation/regress-456964-01.js | Infinite loop decompling function Expected value ' function Test ( ) { var object = { abc : 1 , def : 2 } ; var o = ""; for ( var i in object ) { o += i + " = " + object [ i ] + "\ n "; } return o ; } ', Actual value ' function Test ( ) { var object = { abc : 1 , def : 2 } ; var o = ""; for ( var i in object ) { o += i + ( " = " + object [ i ] + "\ n " ) ; } return o ; } ' item 1

ditto linux. win everything else busted. we back out right?
Flags: wanted1.9.2? → blocking1.9.2+
http://hg.mozilla.org/mozilla-central/rev/5b26aa0bacee
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Attachment #414056 - Flags: approval1.9.2?
This fix was apparently wrong.  In particular, the right fix happened in bug 559438.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: