Closed Bug 1276082 Opened 4 years ago Closed 4 years ago

Assertion failure: isString(), at dist/include/js/Value.h:1330

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla49
Tracking Status
firefox48 --- unaffected
firefox49 --- verified

People

(Reporter: gkw, Assigned: arai)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:update])

Attachments

(2 files)

The following testcase crashes on mozilla-central revision b0096c5c7277 (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --ion-eager):

function f() {
    (function() {}).bind()(/x/);
}
f();
f();


Backtrace:

0   js-dbg-64-dm-clang-darwin-b0096c5c7277	0x000000010c9b0b3d js::GetSelfHostedFunctionName(JSFunction*) + 109 (Value.h:1330)
1   js-dbg-64-dm-clang-darwin-b0096c5c7277	0x000000010c3f0d0c js::jit::MakeMRegExpHoistable(js::jit::MIRGraph&) + 1532 (IonAnalysis.cpp:1873)
2   js-dbg-64-dm-clang-darwin-b0096c5c7277	0x000000010c3ee318 js::jit::OptimizeMIR(js::jit::MIRGenerator*) + 120 (Ion.cpp:1471)
3   js-dbg-64-dm-clang-darwin-b0096c5c7277	0x000000010c3fad73 js::jit::CompileBackEnd(js::jit::MIRGenerator*) + 67 (Ion.cpp:1971)
4   js-dbg-64-dm-clang-darwin-b0096c5c7277	0x000000010c3fcb34 js::jit::Compile(JSContext*, JS::Handle<JSScript*>, js::jit::BaselineFrame*, unsigned char*, bool, bool) + 4260 (Ion.cpp:2246)
/snip

For detailed crash information, see attachment.

MIR is on the stack, setting s-s as a start.
Summary: Assertion failure: isString(), at /Users/skywalker/shell-cache/js-dbg-64-dm-clang-darwin-b0096c5c7277/objdir-js/dist/include/js/Value.h:1330 → Assertion failure: isString(), at dist/include/js/Value.h:1330
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/dca0aaaa1f59
user:        Hannes Verschore
date:        Thu May 26 01:26:35 2016 +0900
summary:     Bug 1260435 - Make RegExp hoistable when it's used only in RegExpBuiltinExec. r=h4writer

Hannes, is bug 1260435 a likely regressor?
Blocks: 1260435
Flags: needinfo?(hv1989)
Oh this is actually a commit of Arai. Somehow the system got confused.

Seems that it fails because:
> fun->getExtendedSlot(LAZY_FUNCTION_NAME_SLOT)
is undefined.

fun->dump() gives "bound"

I assume we need to add an extra check before trying to use "fun->getExtendedSlot(LAZY_FUNCTION_NAME_SLOT)". The functions we are testing are not InterpretedLazy. So I assume "LAZY_FUNCTION_NAME_SLOT" won't be set to the name...
We need something like:
fun->isInterpretedLazy()

right?
Flags: needinfo?(hv1989) → needinfo?(arai.unmht)
Thanks :)

(In reply to Hannes Verschore [:h4writer] from comment #3)
> Oh this is actually a commit of Arai. Somehow the system got confused.
wow, I imported your patch and worked on it, and the author was kept same.


> Seems that it fails because:
> > fun->getExtendedSlot(LAZY_FUNCTION_NAME_SLOT)
> is undefined.
> 
> fun->dump() gives "bound"
> 
> I assume we need to add an extra check before trying to use
> "fun->getExtendedSlot(LAZY_FUNCTION_NAME_SLOT)". The functions we are
> testing are not InterpretedLazy. So I assume "LAZY_FUNCTION_NAME_SLOT" won't
> be set to the name...
> We need something like:
> fun->isInterpretedLazy()
> 
> right?

LAZY_FUNCTION_NAME_SLOT is set only to global functions, regardless of the existence of the script,
and inner functions doesn't have any value in the slot (there is exceptional case (bug 1185106).
So, the slot can be string or undefined, and I think we should just check whether the slot is string or not.
Flags: needinfo?(arai.unmht)
Added isString check before toString.
Assignee: nobody → arai.unmht
Attachment #8757886 - Flags: review?(hv1989)
Comment on attachment 8757886 [details] [diff] [review]
Get self-hosted function name only from global functions.

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

::: js/src/vm/SelfHosting.cpp
@@ +3119,5 @@
>  
>  JSAtom*
>  js::GetSelfHostedFunctionName(JSFunction* fun)
>  {
> +    Value maybeName = fun->getExtendedSlot(LAZY_FUNCTION_NAME_SLOT);

I think you can call it name instead of maybeName.
Attachment #8757886 - Flags: review?(hv1989) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/f2e6fa9e0a42936f6b46d5ef927663fd4abe3316
Bug 1276082 - Get self-hosted function name only from global functions. r=h4writer
https://hg.mozilla.org/mozilla-central/rev/f2e6fa9e0a42
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Group: javascript-core-security → core-security-release
Keywords: regression
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.