1134515 - Assertion failure: entry.isJs(), at vm/SPSProfiler.cpp

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

enableSPSProfiling()
y = new Uint8ClampedArray
x = y
y.valueOf = function() {
    try {
        x == 2
    } catch (e) {}
    evalcx("", this.z)
}
Number(y)

asserts js debug shell on m-c changeset 93ddd99ffd86 with --fuzzing-safe --no-threads --no-ion at Assertion failure: entry.isJs(), at vm/SPSProfiler.cpp.

Debug configure options:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/fuzzing/js/compileShell.py -b "--enable-debug --enable-more-deterministic --enable-nspr-build -R ~/trees/mozilla-central" -r 93ddd99ffd86

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/635e101ed2be
user:        Kannan Vijayan
date:        Wed Jan 14 16:19:13 2015 -0500
summary:     Bug 1057082 - 3/7 - Modify jits to use lastProfilingFrame and lastProfilingCallSite fields. r=jandem

S-s because this is related to profiling. Kannan, is bug 1057082 a likely regressor?

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: stack

(lldb) bt 5
* thread #1: tid = 0xe53c7, 0x00000001002bf063 js-dbg-64-dm-nsprBuild-darwin-93ddd99ffd86`js::SPSBaselineOSRMarker::SPSBaselineOSRMarker(this=<unavailable>, rt=<unavailable>, hasSPSFrame=<unavailable>, _notifier=<unavailable>) + 275 at SPSProfiler.cpp:365, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00000001002bf063 js-dbg-64-dm-nsprBuild-darwin-93ddd99ffd86`js::SPSBaselineOSRMarker::SPSBaselineOSRMarker(this=<unavailable>, rt=<unavailable>, hasSPSFrame=<unavailable>, _notifier=<unavailable>) + 275 at SPSProfiler.cpp:365
    frame #1: 0x0000000100216a2d js-dbg-64-dm-nsprBuild-darwin-93ddd99ffd86`Interpret(cx=0x0000000101f01f30, state=0x00007fff5fa03218) + 3949 at Interpreter.cpp:1732
    frame #2: 0x0000000100215a8d js-dbg-64-dm-nsprBuild-darwin-93ddd99ffd86`js::RunScript(cx=0x0000000101f01f30, state=0x00007fff5fa03218) + 301 at Interpreter.cpp:444
    frame #3: 0x0000000100204e3d js-dbg-64-dm-nsprBuild-darwin-93ddd99ffd86`js::Invoke(cx=0x0000000101f01f30, args=CallArgs at 0x00007fff5fa032a0, construct=<unavailable>) + 733 at Interpreter.cpp:513
    frame #4: 0x0000000100101b01 js-dbg-64-dm-nsprBuild-darwin-93ddd99ffd86`IntlInitialize(cx=0x0000000101f01f30, obj=<unavailable>, initializer=<unavailable>, locales=<unavailable>, options=<unavailable>) + 593 at Intl.cpp:417
(lldb)

Comment 2

[Christian Holler (:decoder)]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision 993eb76a8bd6).

Comment 3

[Christian Holler (:decoder)]

JSBugMon: Fix Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first good revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/cdc1a9758b94
user:        Nicolas B. Pierron
date:        Thu Feb 26 12:18:28 2015 +0100
summary:     Bug 1112164 part 14 - Add types to x86/x64 float registers. r=bbouvier,jandem

This iteration took 197.730 seconds to run.

Comment 4

[Christian Holler (:decoder)]

nbp, is comment 3 a likely fix for this issue?

Comment 5

[Nicolas B. Pierron [:nbp]]

(In reply to Christian Holler (:decoder) from comment #4)
> nbp, is comment 3 a likely fix for this issue?

I do not think so.

Comment 6

[Kannan Vijayan [:djvj]]

It turns out that the Invoke function can get called directly from a non-Interpreter context.  I think the only real fix is to make the assert into a conditional, and not do the markOSR in that situation.

Comment 7

[Kannan Vijayan [:djvj]]

Attachment: fix-bug-1134515.patch

Comment 8

[Shu-yu Guo [:shu]]

Comment on attachment 8575481 [details] [diff] [review]
fix-bug-1134515.patch

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

I don't think this is the right fix.

Here's what I see in gdb:

(gdb) p *profiler->size_
$35 = 1091
(gdb) p profiler->max_
$36 = 1000

Something seems wrong here.

Comment 9

[Shu-yu Guo [:shu]]

ISTM these markers need to all check if profiler->size() < profiler->max, else we'd be modifying beyond the end of profiler->stack().

Comment 10

[Shu-yu Guo [:shu]]

So this is actually s-s; don't open.

Comment 11

[Kannan Vijayan [:djvj]]

Ah crap.  You're right.  I banged this out a few minutes before my talk yesterday.  Should have taken a closer look.  Thanks for catching that.

Comment 12

[Kannan Vijayan [:djvj]]

Attachment: fix-bug-1134515.patch

Actual fix.

Comment 13

[Kannan Vijayan [:djvj]]

Try run before pushing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=09e38efc0924

Comment 14

[Kannan Vijayan [:djvj]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2d5eaa85e9da

Setting in-testsuite? for test push later.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2d5eaa85e9da

Please nominate this for Aurora approval when you get a chance :)

Comment 16

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 17

[Kannan Vijayan [:djvj]]

Comment on attachment 8576085 [details] [diff] [review]
fix-bug-1134515.patch

Approval Request Comment
[Feature/regressing bug #]:
bug 1057082 

[User impact if declined]:
sec-moderate bugs if profiler enabled.

[Describe test coverage new/current, TreeHerder]:
In tree for a month.

[Risks and why]: 
Low risk.

[String/UUID change made/needed]:
N/A

Comment 18

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8576085 [details] [diff] [review]
fix-bug-1134515.patch

Should be in 38 beta 4

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/10c3198eb453

Comment 20

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed on Fx38