Closed
Bug 1257722
Opened 8 years ago
Closed 8 years ago
bug908915.js fails with -1073740791 on 64-bit Visual Studio 2015 builds
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: gps, Assigned: sfink)
References
Details
Attachments
(1 file)
2.63 KB,
patch
|
jorendorff
:
review+
gps
:
feedback+
|
Details | Diff | Splinter Review |
This is related to bug 908915 - a previous sec-high bug. Purposefully not marking as depends on so as to not tip our hand. When building with Visual Studio 2015 Update 1, JIT tests pass across the board except for a failure in bug908915.js in 64-bit builds. Here is output from the Try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d73b8a3fe6b: 17:52:06 WARNING - TEST-UNEXPECTED-FAIL | tests\jit-test\jit-test\tests\basic\bug908915.js | (code -1073740791, args "--baseline-eager --no-fpu") 17:52:06 INFO - INFO exit-status : -1073740791 17:52:06 INFO - INFO timed-out : False 17:52:06 INFO - INFO stderr 2> 17:52:06 INFO - Exit code: -1073740791 17:52:06 INFO - FAIL - basic\bug908915.js 17:52:06 WARNING - TEST-UNEXPECTED-FAIL | tests\jit-test\jit-test\tests\basic\bug908915.js | (code -1073740791, args "--ion-eager --ion-offthread-compile=off --non-writable-jitcode --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads") 17:52:06 INFO - INFO exit-status : -1073740791 17:52:06 INFO - INFO timed-out : False 17:52:06 INFO - INFO stderr 2> 17:52:06 INFO - Exit code: -1073740791 17:52:06 INFO - FAIL - basic\bug908915.js 17:52:06 WARNING - TEST-UNEXPECTED-FAIL | tests\jit-test\jit-test\tests\basic\bug908915.js | (code -1073740791, args "") 17:52:06 INFO - INFO exit-status : -1073740791 17:52:06 INFO - INFO timed-out : False 17:52:06 INFO - INFO stderr 2> 17:52:06 INFO - Exit code: -1073740791 17:52:06 INFO - FAIL - basic\bug908915.js 17:52:06 WARNING - TEST-UNEXPECTED-FAIL | tests\jit-test\jit-test\tests\basic\bug908915.js | (code -1073740791, args "--ion-eager --ion-offthread-compile=off") 17:52:06 INFO - INFO exit-status : -1073740791 17:52:06 INFO - INFO timed-out : False 17:52:06 INFO - INFO stderr 2> 17:52:06 INFO - Exit code: -1073740791 17:52:06 INFO - FAIL - basic\bug908915.js 17:52:06 WARNING - TEST-UNEXPECTED-FAIL | tests\jit-test\jit-test\tests\basic\bug908915.js | (code -1073740791, args "--no-baseline --no-ion") 17:52:06 INFO - INFO exit-status : -1073740791 17:52:06 INFO - INFO timed-out : False 17:52:06 INFO - INFO stderr 2> 17:52:06 INFO - TEST-PASS | tests\jit-test\jit-test\tests\basic\bug920484.js | Success (code 0, args "--baseline-eager") 17:52:06 INFO - Exit code: -1073740791 17:52:06 INFO - FAIL - basic\bug908915.js 17:52:06 WARNING - TEST-UNEXPECTED-FAIL | tests\jit-test\jit-test\tests\basic\bug908915.js | (code -1073740791, args "--baseline-eager") 17:52:06 INFO - INFO exit-status : -1073740791 17:52:06 INFO - INFO timed-out : False 17:52:06 INFO - INFO stderr 2> Error code -1073740791 is a stack overflow. I haven't yet reproduced this locally, so I don't have crash data. If you'd like to reproduce in automation, see bug 1124033 comment 21 for instructions to get VS2015 running in automation. Building with VS2015u1 locally should be sufficient. While we don't yet ship VS2015 builds to users, I'm marking this bug as security sensitive because it could point to a security issue in existing code that our existing tests don't trigger. Better be safe than sorry.
Reporter | ||
Comment 1•8 years ago
|
||
I can reproduce on my Windows 10 machine with a 64-bit build: Unhandled exception at 0x00007FFDADD18528 (ucrtbase.dll) in js.exe: An invalid parameter was passed to a function that considers invalid parameters fatal. #if defined _NO_CRT_STDIO_INLINE ; #else { int _Result; va_list _ArgList; __crt_va_start(_ArgList, _Format); _Result = _vfprintf_l(_Stream, _Format, NULL, _ArgList); __crt_va_end(_ArgList); return _Result; -> } #endif _Format 0x00007ff693ab5f80 "# Roots.\n" const char * const _Result Error reading register value. _Stream ucrtbase.dll!0x00007ffdadd94168 (Type information missing from symbol file) {_Placeholder=0x0000000000000000 } _iobuf * const ucrtbase.dll!_invoke_watson() Unknown ucrtbase.dll!_invalid_parameter() Unknown ucrtbase.dll!_invalid_parameter_noinfo() Unknown ucrtbase.dll!_isatty() Unknown ucrtbase.dll!__acrt_stdio_begin_temporary_buffering_nolock() Unknown ucrtbase.dll!__crt_stdio_output::output_processor<char,class __crt_stdio_output::string_output_adapter<char>,class __crt_stdio_output::positional_parameter_base<char,class __crt_stdio_output::string_output_adapter<char> > >::output_processor<char,class __crt_stdio_output::string_output_adapter<char>,class __crt_stdio_output::positional_parameter_base<char,class __crt_stdio_output::string_output_adapter<char> > >(class __crt_stdio_output::string_output_adapter<char> const &,unsigned __int64,char const * const,struct __crt_locale_pointers * const,char * const) Unknown ucrtbase.dll!__crt_seh_guarded_call<int>::operator()<class <lambda_aa279a7bcb127014629d5b1d62c7325d>,class <lambda_0a64b6a4deda75c01876f29c44fc5eb3> &,class <lambda_3323e69fbefe9d1a11713e670d47cdfa> >(class <lambda_aa279a7bcb127014629d5b1d62c7325d> &&,class <lambda_0a64b6a4deda75c01876f29c44fc5eb3> &,class <lambda_3323e69fbefe9d1a11713e670d47cdfa> &&) Unknown ucrtbase.dll!__stdio_common_vfprintf() Unknown js.exe!fprintf(_iobuf * const _Stream, const char * const _Format, ...) Line 838 C++ js.exe!js::DumpHeap(JSRuntime * rt, _iobuf * fp, js::DumpHeapNurseryBehaviour nurseryBehaviour) Line 1044 C++ js.exe!DumpHeap(JSContext * cx, unsigned int argc, JS::Value * vp) Line 1461 C++ js.exe!js::Invoke(JSContext * cx, const JS::CallArgs & args, js::MaybeConstruct construct) Line 478 C++ js.exe!js::Invoke(JSContext * cx, const JS::Value & thisv, const JS::Value & fval, unsigned int argc, const JS::Value * argv, JS::MutableHandle<JS::Value> rval) Line 530 C++ js.exe!js::CrossCompartmentWrapper::call(JSContext * cx, JS::Handle<JSObject *> wrapper, const JS::CallArgs & args) Line 291 C++ js.exe!js::Proxy::call(JSContext * cx, JS::Handle<JSObject *> proxy, const JS::CallArgs & args) Line 391 C++ js.exe!js::proxy_Call(JSContext * cx, unsigned int argc, JS::Value * vp) Line 682 C++ js.exe!js::Invoke(JSContext * cx, const JS::CallArgs & args, js::MaybeConstruct construct) Line 478 C++ js.exe!js::Invoke(JSContext * cx, const JS::Value & thisv, const JS::Value & fval, unsigned int argc, const JS::Value * argv, JS::MutableHandle<JS::Value> rval) Line 530 C++ js.exe!js::jit::DoCallFallback(JSContext * cx, js::jit::BaselineFrame * frame, js::jit::ICCall_Fallback * stub_, unsigned int argc, JS::Value * vp, JS::MutableHandle<JS::Value> res) Line 6140 C++ [External Code]
Updated•8 years ago
|
Group: core-security → javascript-core-security
Reporter | ||
Comment 2•8 years ago
|
||
This bug is blocking the switch to VS2015. jorendorff: could we please get an assessment of this bug so we know how if/how it impacts the switch to VS2015?
Flags: needinfo?(jorendorff)
Comment 4•8 years ago
|
||
I can reproduce this with the following script: os.file.redirect(null); gc(); backtrace(); The GC likely ends up releasing the last reference to the stdout RCFile and we fclose() stdout. Then we call backtrace(), where we do fprintf(stdout, "%s", sprinter.string()) and we crash because the CRT notices stdout is invalid. sfink, how is this supposed to work?
Flags: needinfo?(jdemooij) → needinfo?(sphink)
Updated•8 years ago
|
Summary: bug908915.js fails with -1073740791 (stack overflow) on 64-bit Visual Studio 2015 builds → bug908915.js fails with -1073740791 on 64-bit Visual Studio 2015 builds
Assignee | ||
Comment 6•8 years ago
|
||
(In reply to Jan de Mooij [:jandem] from comment #4) > I can reproduce this with the following script: > > os.file.redirect(null); > gc(); > backtrace(); > > The GC likely ends up releasing the last reference to the stdout RCFile and > we fclose() stdout. > > Then we call backtrace(), where we do fprintf(stdout, "%s", > sprinter.string()) and we crash because the CRT notices stdout is invalid. > > sfink, how is this supposed to work? Generally, the shell should now be doing a little logic to figure out what the right filehandle is to be writing to (check gOutFile->isOpen(), then use gOutFile->fp). But for cases like this, the RCFiles for stdout and stderr should be initialized with an extra reference. I'll bet I lost that in all the rebasing I ended up doing for this stuff. I'll take a look.
Assignee: nobody → sphink
Flags: needinfo?(sphink)
Assignee | ||
Comment 7•8 years ago
|
||
This should prevent stdout/stderr from getting closed at all. It's a little arguable whether it'd be ok to close them at the end of main(), but it seems safest to just not close them at all.
Attachment #8733951 -
Flags: review?(terrence)
Comment 8•8 years ago
|
||
Comment on attachment 8733951 [details] [diff] [review] Prevent stdout/stderr from getting closed Review of attachment 8733951 [details] [diff] [review]: ----------------------------------------------------------------- Stealing. r=me.
Attachment #8733951 -
Flags: review?(terrence) → review+
Reporter | ||
Comment 9•8 years ago
|
||
Comment on attachment 8733951 [details] [diff] [review] Prevent stdout/stderr from getting closed Review of attachment 8733951 [details] [diff] [review]: ----------------------------------------------------------------- My last VS2015 Try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=b2f956c69f12 was happy with this patch. Please land it ASAP!
Attachment #8733951 -
Flags: feedback+
Comment 11•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/50b237b0fbaa
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment 12•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c679a879092a
You need to log in
before you can comment on or make changes to this bug.
Description
•