164697 - RegExp object created by compiler may be wrong for execution contexts

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Brendan Eich [:brendan]]

The JS API lets you compile once, execute many times using different scope
chains ending in different global objects.  Untrusted JS authors can mess up one
another via /hi/.__proto__, which refers to the compile-time global object's
RegExp.prototype.  We need something akin to function object cloning to handle
this, without adding too much complexity to the trivial "evaluate" case where
"compile" and "execute" use the same global.

/be

Comment 1

[Brendan Eich [:brendan]]

Taking.

/be

Comment 2

[Brendan Eich [:brendan]]

Attachment: proposed fix

I'm still rebuilding my debug trunk mozilla build to test whether the new code
under JSOP_OBJECT's case in js_Interpret, to handle different compile- and
exec-time global objects, gets hit (it should, I think, due to Mozilla's XUL
brutal sharing).  This patch passes the js/tests suite.  Please tell me if it
worksforyou, or not.

/be

Comment 3

[Brendan Eich [:brendan]]

Attachment: oops, fp->objAtomMap is sparse, js_GC must null-test elements in it

Comment 4

[Brendan Eich [:brendan]]

Anyone test this yet?

rogerl, shaver: your review is craved.

/be

Comment 5

[rogerl (gone)]

Comment on attachment 96778 [details] [diff] [review]
oops, fp->objAtomMap is sparse, js_GC must null-test elements in it

r=rogerl

Comment 6

[Mike Shaver (:shaver emeritus)]

Comment on attachment 96778 [details] [diff] [review]
oops, fp->objAtomMap is sparse, js_GC must null-test elements in it

sr=shaver

Comment 7

[Phil Schwartau]

The patch passes the JS testsuite on WinNT, in both the debug
and optimized JS shell. No test regressions were introduced -

Comment 8

[Brendan Eich [:brendan]]

Fix is in the trunk.  This'll need to land on the branch along with a bunch of
other fixes for 1.5 final.

/be

Comment 9

[Phil Schwartau]

QA note: part of this fix has been backed out to fix bug 165201:
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/js/src/jsinterp.c

Bug 165201 is being held open until the best possible solution can be
found. In this situation, let me go ahead and mark this bug Verified - 

Comment 10

[Bob Clary [:bc] (inactive)]

I'm not sure how to fully test this however I added 

/cvsroot/mozilla/js/tests/js1_5/Regress/regress-164697.js,v  <--  regress-164697.js
initial revision: 1.1

to test Brendan's assertion that 

* This same coupling between instance parent and constructor
* parent turns up elsewhere (see jsobj.c's FindConstructor,
* js_ConstructObject, and js_NewObject).  It's fundamental.

for Objects, Functions, Arrays, Strings, Booleans, Numbers, Dates, RegExps and Errors. All pass except anonymous function expressions. I'll file new bug on that.