Makoto Kato [:m_kato] Assignee
	Description • 14 years ago

link.exe sets default stack size is 1MB. (http://msdn.microsoft.com/en-us/library/8cxs58a6.aspx)

But http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/src/xpcjsruntime.cpp#1227

1227     JS_SetNativeStackQuota(cx, 128 * sizeof(size_t) * 1024);

stack limit of js engine sets 128 * 8 * 1024 = 1MB (32bit version is 512KB)

So e4x/Regress/regress-347155.js causes stack overflow.

	David Baron :dbaron:
	Comment 2 • 14 years ago

Do you also expect this to fix this crashtest failure, or should a separate bug be filed on it:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1280581006.1280581431.31618.gz
Rev3 WINNT 6.1 x64 mozilla-central opt test crashtest on 2010/07/31 05:56:46
TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/mozilla-central-w764-opt-u-crashtest/build/reftest/tests/content/events/crashtests/recursive-DOMNodeInserted.html | Exited with code -1073741571 during test run

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1280502954.1280503374.5513.gz
Rev3 WINNT 6.1 x64 mozilla-central opt test crashtest on 2010/07/30 08:15:54
TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/mozilla-central-w764-opt-u-crashtest/build/reftest/tests/content/events/crashtests/recursive-DOMNodeInserted.html | Exited with code -1073741571 during test run

	David Baron :dbaron:
	Comment 3 • 14 years ago

(Actually, I'm reasonably confident it will also fix it.  And probably also the editor test crash in mochitest-plain-3/5, which also looks like it could recurse deeply.)

	David Baron :dbaron:
	Comment 4 • 14 years ago

Was this a regression from http://hg.mozilla.org/mozilla-central/rev/b9f320e24e04 , or has it been a problem since we started running the Win64 tests?

	Makoto Kato [:m_kato] Assignee
	Comment 5 • 14 years ago

(In reply to comment #4)
> Was this a regression from
> http://hg.mozilla.org/mozilla-central/rev/b9f320e24e04 , or has it been a
> problem since we started running the Win64 tests?

I believe that this is from changing compiler to VC++9. But, also, this problem has on VC++10 since stack size is unchanged.

(In reply to comment #2)
> Do you also expect this to fix this crashtest failure, or should a separate bug
> be filed on it:
> 
> http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1280581006.1280581431.31618.gz
> Rev3 WINNT 6.1 x64 mozilla-central opt test crashtest on 2010/07/31 05:56:46
> TEST-UNEXPECTED-FAIL |
> file:///C:/talos-slave/mozilla-central-w764-opt-u-crashtest/build/reftest/tests/content/events/crashtests/recursive-DOMNodeInserted.html
> | Exited with code -1073741571 during test run
> 
> http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1280502954.1280503374.5513.gz
> Rev3 WINNT 6.1 x64 mozilla-central opt test crashtest on 2010/07/30 08:15:54
> TEST-UNEXPECTED-FAIL |
> file:///C:/talos-slave/mozilla-central-w764-opt-u-crashtest/build/reftest/tests/content/events/crashtests/recursive-DOMNodeInserted.html
> | Exited with code -1073741571 during test run

I think that this is same.

	Justin Dolske [:Dolske]
	Comment 6 • 14 years ago

Comment on attachment 461446 [details] [diff] [review]
fix

Someone more familiar with the particularities of x64 and this problem should review this. Maybe ted?

	Justin Dolske [:Dolske]
	Comment 7 • 14 years ago

(Also, do Linux or OS X need a stacksize adjustment too on x86?)

	(not currently active) Ted Mielczarek
	Comment 8 • 14 years ago

Comment on attachment 461446 [details] [diff] [review]
fix

Sorry, but this really isn't my purview either. dbaron: care to review?

As an aside, I don't think you need the "see bug 582910" comment. We have blame for people to find bug numbers.

	David Baron :dbaron:
	Comment 9 • 14 years ago

Comment on attachment 461446 [details] [diff] [review]
fix

r=dbaron

Though perhaps it's better to use "/STACK" rather tan "-STACK" to be consistent with the code right above it?

(I even found the documentation for the option at
http://msdn.microsoft.com/en-us/library/8cxs58a6%28v=VS.90%29.aspx .)

	Makoto Kato [:m_kato] Assignee
	Comment 10 • 14 years ago

(In reply to comment #9)
> Comment on attachment 461446 [details] [diff] [review]
> fix
> 
> r=dbaron
> 
> Though perhaps it's better to use "/STACK" rather tan "-STACK" to be consistent
> with the code right above it?
> 
> (I even found the documentation for the option at
> http://msdn.microsoft.com/en-us/library/8cxs58a6%28v=VS.90%29.aspx .)

It works either '-' or '/'.  Most linker parameters that we uses are '-'

link -NOLOGO -DLL -OUT:xul.dll -PDB:xul.pdb -SUBSYSTEM:WINDOWS  dlldeps-xul.obj
nsStaticXULComponents.obj nsDllMain.obj dlldeps.obj nsGFXDeps.obj dlldeps-zlib.o
bj nsUnicharUtils.obj nsBidiUtils.obj nsRDFResource.obj    ./module.res -NXCOMPA
T -DYNAMICBASE -SAFESEH  -DEBUG -DEBUGTYPE:CV -MANIFEST:NO -LIBPATH:"c:/Workspac
e/hg.mozilla.org/objdir-win32-1/memory/jemalloc/crtsrc/build/intel" -NODEFAULTLI
B:msvcrt -NODEFAULTLIB:msvcrtd -NODEFAULTLIB:msvcprt -NODEFAULTLIB:msvcprtd ...

	David Baron :dbaron:
	Comment 11 • 14 years ago

Actually, though, maybe this patch should be changing all the locations where we currently have this code:
http://mxr.mozilla.org/mozilla-central/search?string=HEAP%3A0
(r=dbaron and a=dbaron stand with that change)

	Makoto Kato [:m_kato] Assignee
	Comment 13 • 14 years ago

http://hg.mozilla.org/mozilla-central/rev/4e8a688ab358

	Makoto Kato [:m_kato] Assignee
	Comment 14 • 14 years ago

We may need this for xpcshell.  xpcshell test hang (timeout) is same reason (stack overflow).

	Makoto Kato [:m_kato] Assignee
	Comment 15 • 14 years ago

fix xpcshell test hang (I forget xpcshell for test!)

	Makoto Kato [:m_kato] Assignee
	Comment 16 • 14 years ago

land for xpcshell
http://hg.mozilla.org/mozilla-central/rev/dc804317e500

	David Baron :dbaron:
	Comment 17 • 14 years ago

And xpcshell tests are now green.