Closed Bug 237461 Opened 20 years ago Closed 20 years ago

nested function with same name as declared var botches fun->nvars

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla1.7beta

People

(Reporter: brendan, Assigned: brendan)

References

()

Details

(Keywords: js1.5)

Attachments

(3 files, 2 obsolete files)

See the news article.  Aleksey's testcase:

function g()
    {
    var core = {};
    core.js = {};
    core.js.init = function()
        {
        var loader = null;
        
        function loader() {}
        };
    return core;
    }
var s = new Script(""+g.toString());
var frozen = s.freeze();              // crash.
print("len:"+frozen.length);

You'll need to hack JS_HAS_XDR_FREEZE_THAW to 1 in jsconfig.h to test.  Patch in
a moment.

/be
So this breaks XDR.  What else?  Patch now, trying for 1.7b.

/be
Status: NEW → ASSIGNED
Flags: blocking1.7b?
Priority: -- → P1
Target Milestone: --- → mozilla1.7beta
Attached patch proposed fix (for applying) (obsolete) — Splinter Review
Comment on attachment 143902 [details] [diff] [review]
diff -w version of last attachment (for reviewing)

This is as safe as anything else for 1.7b.

/be
Attachment #143902 - Flags: review?(shaver)
Attachment #143902 - Flags: approval1.7b?
Comment on attachment 143902 [details] [diff] [review]
diff -w version of last attachment (for reviewing)

Oops, that block-local pobj I added shadows a function-wide pobj that I can use
here.  Assume I've fixed that by removing the block-local when reviewing.

/be
Comment on attachment 143902 [details] [diff] [review]
diff -w version of last attachment (for reviewing)

Argh, hacking patches between two trees, one with extra changes.  Fixed patches
coming up.

/be
Attachment #143902 - Attachment is obsolete: true
Attachment #143902 - Flags: review?(shaver)
Attachment #143902 - Flags: approval1.7b?
Working patch, consolidate prop as well as pobj, elimating thereby-single-use
sprop and avoiding strict aliasing gcc warnings.

/be
Attachment #143901 - Attachment is obsolete: true
Comment on attachment 143907 [details] [diff] [review]
diff -w version of last attachment (for reviewing)

Let's try this one on for size.

/be
Attachment #143907 - Flags: review?(shaver)
Attachment #143907 - Flags: approval1.7b?
Comment on attachment 143907 [details] [diff] [review]
diff -w version of last attachment (for reviewing)

Add a test to the suite for this?
Attachment #143907 - Flags: review?(shaver) → review+
brendan: could the normal testsuite notice this some other way than Freeze? if 
not (as i suspect) the test will be fairly useless on average.
We can add a test area to the suite for XDR.  We'll need to build a special
version of the js shell.

/be
Comment on attachment 143907 [details] [diff] [review]
diff -w version of last attachment (for reviewing)

a=chofmann for 1.7b
Attachment #143907 - Flags: approval1.7b? → approval1.7b+
Fixed.

/be
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Flags: blocking1.7b?
I'll write to Aleksey.
js1_5/Regress/regress-237461.js checked in.
Flags: testcase+
verified fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: