485178 - js shell: scatter sets wrong thread stack limit

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

Currently SetContextOptions from shell/js.cpp sets the stack limit for the context based on the address of the gStackBase global variable. But this is valid only for the main thread, for threads created through the scatter function this generates the wrong answer meaning that code either miss the proper stack limit or would generate the spurious too-much-recursion errors. The following example demonstrates the first case on Linux:

@watson-32~> cat ~/s/x1.js
function f()
{
    var obj = {};
    obj.toString = function() { return ""+obj; }
    return ""+obj;
}

scatter([Math.sin, f]);
@watson-32~> ~/build/js32.tm.dbg/js ~/s/x1.js
Segmentation fault

I nominate the bug for 1.9.1 as those incorrect recursion errors makes debugging multi-threading problems harder.

Comment 1

[Igor Bukanov]

Attachment: v1

The patch uses thread-private to store the address of a variable in the top native stack frame and use it to calculate the limit.

Comment 2

[Igor Bukanov]

landed to TM - http://hg.mozilla.org/tracemonkey/rev/0b36bddcefe4

Comment 3

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Looks like this broke Windows:

e:/builds/moz2_slave/tracemonkey-win32-nightly/build/js/src/shell/js.cpp(4600) : error C2664: 'PR_NewThreadPrivateIndex' : cannot convert parameter 1 from 'uint32 *' to 'PRUintn *'
        Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast

http://tinderbox.mozilla.org/showlog.cgi?log=TraceMonkey/1238061720.1238063080.2204.gz#err1

Can someone back it out? I won't get a chance for a couple of hours.

Comment 4

[Igor Bukanov]

I backed out the landed patch. I assumed that uint32 is the same as PRUintn. But this is only so under Linux.

Comment 5

[Igor Bukanov]

Attachment: v2

The new patch uses the proper type, PRUintn, not uint32, for thread-private index type.

Comment 6

[Igor Bukanov]

re-landed to TM - http://hg.mozilla.org/tracemonkey/rev/72ec438d1934

Comment 7

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/72ec438d1934

Comment 8

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9a0957485ff7