379245 - Interpreter on 1.8.* does not clear obj register during inline calls

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

Currently the interpreter on 1.8. branches does not clear the object register during when returning from the inlined call. Thus for a case like f()() where f is inlined the interpreter passes as this to the second call the value of obj register left from f() invocation instead of unconditional null as demonstrated by the following example:

var fThis;

function f()
{
   fThis = this;
   return ({x: f}).x;
}

f()();

if (this !== fThis)
  throw "bad this";

Currently when invoked in js shell from 1.8.* branches the code throws the exception:

~/m/1.8.1/mozilla/js/src $ ./Linux_All_DBG.OBJ/js ~/s/y.js
uncaught exception: bad this
~/m/1.8.1/mozilla/js/src $ cd ~/m/1.8.0/mozilla/js/src
~/m/1.8.0/mozilla/js/src $ ./Linux_All_DBG.OBJ/js ~/s/y.js
uncaught exception: bad this

This is exploitable since a branch callback called when the inlined function returns can trigger GC and the value stored in obj register is not rooted. For example, when the example is run against 1.8.1 shell with branch callback enabled the code crashes during "this" calculations as on 1.8.1 the branch callback calls invokes th GC at the right moment:

~/m/1.8.1/mozilla/js/src $ ./Linux_All_DBG.OBJ/js -b 1000 ~/s/y.js
segmentation fault

The crash does not happen with 1.8.0 shell due to difference in MaybeGC implementation. 




 with enabled branch callback it crashes:

~/m/1.8.1/mozilla/js/src $ ./Linux_All_DBG.OBJ/js -b 1000 ~/s/y.js
segmentation fault

while on 1.8.0 the code gives:

~/m/1.8.0/mozilla/js/src $ ./Linux_All_DBG.OBJ/js -b 1000 ~/s/y.js
uncaught exception: bad this

Comment 1

[Igor Bukanov]

My quick attempt to trigger the hazard in a browser using the above example has not succeed in crashing the browser as it is non-trivial to trigger the branch callback that in turn triggers GC at the right moment. But I do not think it is impossible to write deterministic example triggering the crash. 

Comment 2

[Igor Bukanov]

Attachment: Fix v1

The fix clear the obj register both on return form the inlined function and on its invocation. The latter is not necessary to fix the bug but I prefer to play safe.

Comment 3

[Igor Bukanov]

The bug does not exist on trunk due to changes from bug 363530.

Comment 4

[Daniel Veditz [:dveditz]]

Comment on attachment 263246 [details] [diff] [review]
Fix v1

approved for 1.8.1.5 and 1.8.0.13, a=dveditz for release-drivers.

Comment 5

[Igor Bukanov]

I committed the patch from comment 2 tp MOZILLA_1_8_BRANCH:

Checking in jsinterp.c;
/cvsroot/mozilla/js/src/jsinterp.c,v  <--  jsinterp.c
new revision: 3.181.2.90; previous revision: 3.181.2.89
done

Comment 6

[Igor Bukanov]

Comment on attachment 263246 [details] [diff] [review]
Fix v1

The patch requires a merger for 1.8.0 branch.

Comment 7

[Igor Bukanov]

Attachment: Fix v1 for 1.8.0 branch

Comment 8

[Igor Bukanov]

Attachment: diff between 1.8.1 and 1.8.0 versions of fix v1

Comment 9

[Brendan Eich [:brendan]]

Comment on attachment 271662 [details] [diff] [review]
Fix v1 for 1.8.0 branch

Are we still maintaining 1.8.0?

/be

Comment 10

[Igor Bukanov]

(In reply to comment #9)
> (From update of attachment 271662 [details] [diff] [review])
> Are we still maintaining 1.8.0?

Well, the answer is not, but Linux distros still have to do it. So if the patch is trivial to port, I would like to continue to consider it for 1.8.0.

Comment 11

[Bob Clary [:bc] (inactive)]

Attachment: js1_5/Regress/regress-379245.js

Comment 12

[Bob Clary [:bc] (inactive)]

verified fixed 1.8.1 windows/linux/macppc opt/debug shell/browser 7/16

Comment 13

[Daniel Veditz [:dveditz]]

Comment on attachment 271662 [details] [diff] [review]
Fix v1 for 1.8.0 branch

approved for 1.8.0.13, a=dveditz

Comment 14

[Daniel Veditz [:dveditz]]

Fix landed for 1.8.0.13

Comment 15

[Bob Clary [:bc] (inactive)]

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