1149495 - Assertion failure: isInt32(), at ../../dist/include/js/Value.h:1212 with runOffThreadScript

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 8af276ab8636 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --thread-count=2):

offThreadCompileScript('Error()', { lineNumber: (4294967295)});
runOffThreadScript().stack;



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x00000000006a259c in JS::Value::toInt32 (this=<optimized out>) at ../../dist/include/js/Value.h:1212
#0  0x00000000006a259c in JS::Value::toInt32 (this=<optimized out>) at ../../dist/include/js/Value.h:1212
#1  0x00000000006aeec8 in toInt32 (this=<optimized out>) at js/src/vm/SavedStacks.cpp:259
#2  js::SavedFrame::getLine (this=<optimized out>) at js/src/vm/SavedStacks.cpp:258
#3  0x00000000006c17e9 in JS::BuildStackString (cx=cx@entry=0x7ffff691b4e0, stack=..., stack@entry=..., stringp=..., stringp@entry=...) at js/src/vm/SavedStacks.cpp:669
#4  0x00000000006240c0 in js::ErrorObject::getStack (cx=0x7ffff691b4e0, argc=<optimized out>, vp=<optimized out>) at js/src/vm/ErrorObject.cpp:213
#5  0x0000000000674912 in js::CallJSNative (cx=0x7ffff691b4e0, native=0x623f10 <js::ErrorObject::getStack(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:235
#6  0x0000000000663943 in js::Invoke (cx=cx@entry=0x7ffff691b4e0, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:470
#7  0x0000000000665403 in js::Invoke (cx=cx@entry=0x7ffff691b4e0, thisv=..., fval=..., argc=argc@entry=0, argv=argv@entry=0x0, rval=..., rval@entry=...) at js/src/vm/Interpreter.cpp:526
#8  0x0000000000668eb0 in js::InvokeGetter (cx=cx@entry=0x7ffff691b4e0, obj=0x7ffff5262100, fval=..., rval=rval@entry=...) at js/src/vm/Interpreter.cpp:595
#9  0x00000000006d170e in CallGetter (vp=..., shape=..., receiver=..., cx=0x7ffff691b4e0) at js/src/vm/NativeObject.cpp:1635
#10 GetExistingProperty<(js::AllowGC)1> (cx=0x7ffff691b4e0, receiver=..., obj=..., shape=..., vp=...) at js/src/vm/NativeObject.cpp:1685
#11 0x00000000006d1bd2 in NativeGetPropertyInline<(js::AllowGC)1> (cx=0x7ffff691b4e0, obj=..., receiver=..., id=..., nameLookup=nameLookup@entry=NotNameLookup, vp=...) at js/src/vm/NativeObject.cpp:1899
#12 0x00000000006d1fb0 in js::NativeGetProperty (cx=<optimized out>, obj=..., receiver=..., id=..., vp=...) at js/src/vm/NativeObject.cpp:1933
#13 0x00000000004fd570 in js::GetProperty (cx=<optimized out>, obj=..., receiver=..., id=..., vp=...) at js/src/vm/NativeObject.h:1439
#14 0x00000000006573f9 in GetPropertyOperation (vp=..., lval=..., pc=<optimized out>, script=..., fp=<optimized out>, cx=0x7ffff691b4e0) at js/src/vm/Interpreter.cpp:260
#15 Interpret (cx=cx@entry=0x7ffff691b4e0, state=...) at js/src/vm/Interpreter.cpp:2406
#16 0x00000000006636a8 in js::RunScript (cx=cx@entry=0x7ffff691b4e0, state=...) at js/src/vm/Interpreter.cpp:420
#17 0x000000000066a756 in js::ExecuteKernel (cx=cx@entry=0x7ffff691b4e0, script=..., script@entry=..., scopeChainArg=..., thisv=..., type=type@entry=js::EXECUTE_GLOBAL, evalInFrame=..., evalInFrame@entry=..., result=result@entry=0x0) at js/src/vm/Interpreter.cpp:636
#18 0x000000000066c800 in js::Execute (cx=cx@entry=0x7ffff691b4e0, script=script@entry=..., scopeChainArg=..., rval=rval@entry=0x0) at js/src/vm/Interpreter.cpp:679
#19 0x0000000000a45b9e in ExecuteScript (cx=cx@entry=0x7ffff691b4e0, obj=..., scriptArg=..., rval=rval@entry=0x0) at js/src/jsapi.cpp:4089
#20 0x0000000000a45d1b in JS_ExecuteScript (cx=cx@entry=0x7ffff691b4e0, scriptArg=..., scriptArg@entry=...) at js/src/jsapi.cpp:4111
#21 0x00000000004253b3 in RunFile (compileOnly=false, file=0x7ffff699b800, filename=0x7fffffffdf75 "min.js", cx=0x7ffff691b4e0) at js/src/shell/js.cpp:466
#22 Process (cx=cx@entry=0x7ffff691b4e0, filename=0x7fffffffdf75 "min.js", forceTTY=forceTTY@entry=false) at js/src/shell/js.cpp:597
#23 0x0000000000472033 in ProcessArgs (op=0x7fffffffd9d0, cx=0x7ffff691b4e0) at js/src/shell/js.cpp:5777
#24 Shell (envp=<optimized out>, op=0x7fffffffd9d0, cx=0x7ffff691b4e0) at js/src/shell/js.cpp:6043
#25 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:6385
rax	0x0	0
rbx	0x7ffff691b4e0	140737330132192
rcx	0x7ffff6ca53cd	140737333842893
rdx	0x0	0
rsi	0x7ffff6f7a9d0	140737336814032
rdi	0x7ffff6f791c0	140737336807872
rbp	0x7fffffffbf30	140737488338736
rsp	0x7fffffffbf30	140737488338736
r8	0x7ffff7fe0780	140737354008448
r9	0x7ffff51e30a0	140737305784480
r10	0x7fffffffbcf0	140737488338160
r11	0x7ffff6c27960	140737333328224
r12	0x7fffffffc080	140737488339072
r13	0x0	0
r14	0x489af0	4758256
r15	0x7fffffffc020	140737488338976
rip	0x6a259c <JS::Value::toInt32() const+28>
=> 0x6a259c <JS::Value::toInt32() const+28>:	movl   $0x4bc,0x0
   0x6a25a7 <JS::Value::toInt32() const+39>:	callq  0x423140 <abort@plt>


Likely a shell-only issue.

Comment 1

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, result:
=== Treeherder Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20150327125918" and the hash "d56c2eb468f6".
The "bad" changeset has the timestamp "20150327131017" and the hash "b336dc0af92c".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d56c2eb468f6&tochange=b336dc0af92c

Comment 2

[Christian Holler (:decoder)]

Needinfo from fitzgen based on comment 1.

Comment 3

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: SavedFrame objects should handle all uint32 values.

Comment 4

[Shu-yu Guo [:shu]]

Comment on attachment 8588368 [details] [diff] [review]
SavedFrame objects should handle all uint32 values.

Review of attachment 8588368 [details] [diff] [review]:
-----------------------------------------------------------------

Looks simple enough, though I'd like the answers to the questions before I r+.

::: js/src/vm/SavedStacks.cpp
@@ +254,5 @@
>  uint32_t
>  SavedFrame::getLine()
>  {
>      const Value& v = getReservedSlot(JSSLOT_LINE);
> +    return v.toPrivateUint32();

Are SavedFrame exposed elsewhere? How do you guarantee that this doesn't get erroneously interpreted as an int32?

@@ +308,5 @@
>      MOZ_ASSERT(getReservedSlot(JSSLOT_SOURCE).isUndefined());
>      setReservedSlot(JSSLOT_SOURCE, StringValue(lookup->source));
>  
> +    setReservedSlot(JSSLOT_LINE, PrivateUint32Value(lookup->line));
> +    setReservedSlot(JSSLOT_COLUMN, PrivateUint32Value(lookup->column));

How are lines and columns guaranteed to be within uint32 range?

Comment 5

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to Shu-yu Guo [:shu] from comment #4)
> Are SavedFrame exposed elsewhere? How do you guarantee that this doesn't get
> erroneously interpreted as an int32?

Things should only be accessing the reserved slots through SavedFrame::getFoo helpers.

> How are lines and columns guaranteed to be within uint32 range?

For some reason, I was under the impression that PCToLineNumber returned uint32_t but that no longer seems to be the case and blame is telling me that it probably never was the case.

What would you suggest doing here?

Comment 6

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8588368 [details] [diff] [review]
SavedFrame objects should handle all uint32 values.

Re-requesting review as requested on IRC

Comment 7

[Shu-yu Guo [:shu]]

(In reply to Nick Fitzgerald [:fitzgen] from comment #5)
> (In reply to Shu-yu Guo [:shu] from comment #4)
> > Are SavedFrame exposed elsewhere? How do you guarantee that this doesn't get
> > erroneously interpreted as an int32?
> 
> Things should only be accessing the reserved slots through
> SavedFrame::getFoo helpers.
> 
> > How are lines and columns guaranteed to be within uint32 range?
> 
> For some reason, I was under the impression that PCToLineNumber returned
> uint32_t but that no longer seems to be the case and blame is telling me
> that it probably never was the case.
> 
> What would you suggest doing here?

We would have other problems if it overflowed uint32. In practice I wouldn't worry. Famous last words...

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bbe22e6e4e67

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bbe22e6e4e67