Assertion failure: !builder || !hasPendingIonBuilder(), at js/src/jit/BaselineJIT.h:492 with Proxy

RESOLVED FIXED in Firefox 50

Status

()

Core
JavaScript Engine
P1
critical
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: decoder, Assigned: h4writer)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla52
x86_64
Linux
assertion, jsbugmon, regression, testcase
Points:
---

Firefox Tracking Flags

(firefox50 fixed, firefox51 fixed, firefox52 fixed)

Details

(Whiteboard: [jsbugmon:update])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

a year ago
The following testcase crashes on mozilla-central revision 4f72b1d05267 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug --enable-optimize, run with --fuzzing-safe --thread-count=2 --ion-eager --baseline-eager):

var handler = {
  get(t, p) {"use strict"; return eval("f();");}
};
var f = new Proxy(function(){}, handler);
new f();
function newFunc(x) {
  new Function(x)();
};
newFunc(``);



Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x000000000069d622 in js::jit::BaselineScript::setPendingIonBuilder (builder=0x7ffff69bf1c0, script=0x7ffff367a1a8, maybeRuntime=0x7ffff695f208, this=0x7ffff33acd00) at js/src/jit/BaselineJIT.h:492
#0  0x000000000069d622 in js::jit::BaselineScript::setPendingIonBuilder (builder=0x7ffff69bf1c0, script=0x7ffff367a1a8, maybeRuntime=0x7ffff695f208, this=0x7ffff33acd00) at js/src/jit/BaselineJIT.h:492
#1  js::jit::AttachFinishedCompilations (cx=cx@entry=0x7ffff695f000) at js/src/jit/Ion.cpp:2072
#2  0x0000000000b14bf8 in InvokeInterruptCallback (cx=0x7ffff695f000) at js/src/vm/Runtime.cpp:528
#3  0x00007ffff7e406d4 in ?? ()
#4  0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x7ffff69bf1c0	140737330803136
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffb860	140737488336992
rsp	0x7fffffffb7b0	140737488336816
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4740	140737354024768
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x7ffff367a1a8	140737277043112
r13	0x7ffff694f800	140737330345984
r14	0x7ffff695f000	140737330409472
r15	0x7ffff33acd00	140737274105088
rip	0x69d622 <js::jit::AttachFinishedCompilations(JSContext*)+1554>
=> 0x69d622 <js::jit::AttachFinishedCompilations(JSContext*)+1554>:	movl   $0x0,0x0
   0x69d62d <js::jit::AttachFinishedCompilations(JSContext*)+1565>:	ud2    


I've been seeing this issue for quite a while, but only with highly intermittent testcases that would almost never reproduce. Now is a good time to strike this bug down I guess :)

Updated

a year ago
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]

Comment 1

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

The "good" changeset has the timestamp "20151008170051" and the hash "a71d7c5dc7bb1549fd0fb720ec8e6cdae1941b27".
The "bad" changeset has the timestamp "20151008170355" and the hash "b5a67fbf08051b7961cef6b9a8c8835fd3f87f50".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a71d7c5dc7bb1549fd0fb720ec8e6cdae1941b27&tochange=b5a67fbf08051b7961cef6b9a8c8835fd3f87f50
(Assignee)

Updated

a year ago
Flags: needinfo?(hv1989)
(Reporter)

Comment 2

a year ago
This is an automated crash issue comment:

Summary: Crash [@ JSRuntime::ionLazyLinkListRemove]
Build version: mozilla-central revision 401ea746b1a9
Build flags: --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-debug --enable-optimize
Runtime options: --fuzzing-safe --thread-count=2 --disable-oom-functions --ion-extra-checks --ion-eager

Testcase:

See attachment.

Backtrace:

 received signal SIGSEGV, Segmentation fault.
JSRuntime::ionLazyLinkListRemove (this=this@entry=0x7ffff695f200, builder=builder@entry=0x0) at js/src/vm/Runtime.cpp:890
#0  JSRuntime::ionLazyLinkListRemove (this=this@entry=0x7ffff695f200, builder=builder@entry=0x0) at js/src/vm/Runtime.cpp:890
#1  0x00000000005dd4e0 in js::jit::LinkIonScript (cx=cx@entry=0x7ffff695f000, calleeScript=calleeScript@entry=...) at js/src/jit/Ion.cpp:574
#2  0x00000000005dd9ef in js::jit::AttachFinishedCompilations (cx=cx@entry=0x7ffff695f000) at js/src/jit/Ion.cpp:2083
#3  0x00000000008efe29 in InvokeInterruptCallback (cx=0x7ffff695f000) at js/src/vm/Runtime.cpp:528
#4  0x00007ffff7e3fd39 in ?? ()
#5  0x0000000000000000 in ?? ()
rax	0x7ffff6b17e19	140737332215321
rbx	0x7ffff695f000	140737330409472
rcx	0x7ffff36e0f10	140737277464336
rdx	0x7ffff6920bc0	140737330154432
rsi	0x0	0
rdi	0x7ffff695f200	140737330409984
rbp	0x0	0
rsp	0x7fffffffc3b8	140737488339896
r8	0x7ffff6914dc0	140737330105792
r9	0x8000	32768
r10	0x7ffff6a00121	140737331069217
r11	0x206	518
r12	0x7ffff695f200	140737330409984
r13	0x7ffff694f588	140737330345352
r14	0x7fffffffc510	140737488340240
r15	0x7fffffffc430	140737488340016
rip	0x8f4240 <JSRuntime::ionLazyLinkListRemove(js::jit::IonBuilder*)>
=> 0x8f4240 <JSRuntime::ionLazyLinkListRemove(js::jit::IonBuilder*)>:	mov    0x80(%rsi),%rcx
   0x8f4247 <JSRuntime::ionLazyLinkListRemove(js::jit::IonBuilder*)+7>:	mov    0x88(%rsi),%rdx



In the attached testcase, there is a // TODO comment indicating which lines can be removed that turn this well reproducing testcase into a highly intermittent one. It would be good to figure out why this happens and if we can improve the shell somehow to avoid this.
(Reporter)

Comment 3

a year ago
Created attachment 8788414 [details]
Testcase for comment 11688023
(Assignee)

Comment 4

a year ago
Created attachment 8793265 [details] [diff] [review]
Patch

Thanks for the testcase. Made it quite easy to spot the issue.
Assignee: nobody → hv1989
Flags: needinfo?(hv1989)
Attachment #8793265 - Flags: review?(jdemooij)
Comment on attachment 8793265 [details] [diff] [review]
Patch

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

::: js/src/jit/Ion.cpp
@@ +2566,5 @@
>              return status;
>      }
>  
> +    // Skip if the script is being compiled off thread (again).
> +    // The above lines (invoke) could have started an ion compilation.

I wondered which lines, maybeCreateThisForConstructor? I would use that instead of 'invoke' as it's more specific.
Attachment #8793265 - Flags: review?(jdemooij) → review+
Is this ready to land, Hannes?
Flags: needinfo?(hv1989)
(Assignee)

Updated

a year ago
Flags: needinfo?(hv1989)
Priority: -- → P1

Comment 7

a year ago
Pushed by hv1989@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d7df8e59c649
IonMonkey - Make sure we don't compile the same script twice at the same time, r=jandem

Comment 8

a year ago
Pushed by hv1989@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b874833c927
IonMonkey - Fix hazard, r=bustage

Comment 9

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d7df8e59c649
https://hg.mozilla.org/mozilla-central/rev/2b874833c927
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Hi :h4writer,
Since this is a regression and also affects 51, do you think the patch is worth uplifting to 51?
Flags: needinfo?(hv1989)
(Assignee)

Comment 11

a year ago
Created attachment 8798785 [details] [diff] [review]
patch
Attachment #8793265 - Attachment is obsolete: true
Flags: needinfo?(hv1989)
Attachment #8798785 - Flags: review+
(Assignee)

Comment 12

a year ago
Comment on attachment 8798785 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]
bug 1178834

[User impact if declined]:
I think this could cause memory leaks. The occurrence should be very low though.

[Describe test coverage new/current, TreeHerder]:
jit-tests are fine and landed on mozilla-inbound yesterday

[Risks and why]: 
Low risk. It adds a small check that would let this go through a fairly often used data-flow.


[String/UUID change made/needed]:
/
Attachment #8798785 - Flags: approval-mozilla-beta?
Attachment #8798785 - Flags: approval-mozilla-aurora?
status-firefox50: --- → affected
Comment on attachment 8798785 [details] [diff] [review]
patch

Crash fix, Aurora51+, Beta50+
Attachment #8798785 - Flags: approval-mozilla-beta?
Attachment #8798785 - Flags: approval-mozilla-beta+
Attachment #8798785 - Flags: approval-mozilla-aurora?
Attachment #8798785 - Flags: approval-mozilla-aurora+

Comment 14

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/0d808f4fa330
status-firefox51: affected → fixed

Comment 15

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/29d25f5224ad
status-firefox50: affected → fixed
You need to log in before you can comment on or make changes to this bug.