414755 - Missing SAVE_SP_AND_PC in STORE_(NUMBER|INT|UINT)

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

The macros STORE_(NUMBER|INT|UINT) from jsinterp.c do not call SAVE_SP_AND_PC before calling potentially GC-triggering js_NewDoubleValue. This leads to a GC hazard as demonstrated with the following example:

~/m/ff/mozilla/js/src $ cat ~/m/y.js

var expect = f();

gczeal(2);

var actual = f();

if (expect !== actual)
    throw "Hazard: actual="+actual+" expect="+expect;

function f()
{
    var a = 1e10;
    var b = 2e10;
    var c = 3e10;
    
    return (a*2) * ((b*2) * c); 
}

~/m/ff/mozilla/js/src $ ./Linux_All_DBG.OBJ/js ~/m/y.js
uncaught exception: Hazard: actual=1.44e+42 expect=2.4e+31

Note that so far I was not able to discover a way to get a crash with that. But it can be used to read a memory image of allocated structs.

Comment 1

[Igor Bukanov]

Attachment: v1

The patch just adds the missing calls to SAVE_SP_AND_PC.

Comment 2

[Igor Bukanov]

I checked in the patch from comment 1 to the trunk:

http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&branch=HEAD&cvsroot=%252Fcvsroot&date=explicit&mindate=1201671180&maxdate=1201671331&who=igor%25mir2.org

Comment 3

[Igor Bukanov]

Comment on attachment 300249 [details] [diff] [review]
v1

The patch applies to the 181 branch as is.

Comment 4

[Daniel Veditz [:dveditz]]

Comment on attachment 300249 [details] [diff] [review]
v1

approved for 1.8.1.13, a=dveditz for release-drivers

Comment 5

[Bob Clary [:bc] (inactive)]

Attachment: js1_5/extensions/regress-414755.js

Comment 6

[Bob Clary [:bc] (inactive)]

v

Comment 7

[Igor Bukanov]

I checked in the patch from comment 1 to MOZILLA_1_8_BRANCH:

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

Comment 8

[Bob Clary [:bc] (inactive)]

v 1.8.1 linux|mac

Comment 9

[Bob Clary [:bc] (inactive)]

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