1670481 - Crash in [@ (anonymous namespace)::BytecodeParser::parse]

Status: NEW

(Core :: JavaScript Engine, defect, P3)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Crash report: https://crash-stats.mozilla.org/report/index/43f6df47-e514-4493-85a4-775770201011

Reason: EXC_BAD_ACCESS / KERN_INVALID_ADDRESS

Top 2 frames of crashing thread:

0 XUL  js/src/vm/BytecodeUtil.cpp:869
1 XUL js::DecompileValueGenerator js/src/vm/BytecodeUtil.cpp:2432

Comment 1

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

From bug 1665664 comment 7:
(In reply to Simon Bennetts from comment #7)

Looks like this isnt hapenning in my other profiles, so that one could have become corrupted?
I'll keep it for now in case you need more details but it looks like I can use a new one to get around this problem...

Comment 2

[Nicolas B. Pierron [:nbp]]

Crash-stat reports is highly strange, I would not have expected a bug which is mostly on Mac.
Additional point, while looking like a recent issue on crash-stat, it crashes on all channels, and only a few build-id are reported.

This definitely sounds like it could be a compiler issue …

Comment 3

[Gian-Carlo Pascutto [:gcp]]

Here's a report on Linux (there's a few, but it's really rare, and maybe a different problem):
https://crash-stats.mozilla.org/report/index/41ce5324-239d-457c-9933-af5470201021

Comment 4

[(Away)]

Crash-stat reports is highly strange, I would not have expected a bug which is mostly on Mac.

This was an inadvertent filtering due to signature differences. Mac spells (anonymous namespace) with parens. Windows spells it with a backtick.

Comment 5

[Nicolas B. Pierron [:nbp]]

Ok, back to JS world then.
Thanks for noticing this symbol issue.

Comment 6

[(Away)]

In case it helps, in the Windows minidumps I see values of stackDepth like -1 and -2 at https://searchfox.org/mozilla-central/rev/25d5a4443a7e13cfa58eff38f1faa5e69f0b170f/js/src/vm/BytecodeUtil.cpp#611.

Comment 7

[Jason Orendorff [:jorendorff]]

That's super helpful, dmajor. Thank you.

DecompileValueGenerator is not terribly hot code, so we can add a release assertion to crash before segfaulting, and perhaps get more information.

Comment 8

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1670481 - Strengthen assertions around stackDepth. r?jandem

dmajor observed that in some crash reports for this bug, stackDepth
has negative values. It seems unlikely we have a simple, deterministic
bug computing the stack depth; the fuzzers tend to find that sort of thing
very quickly. However, it is easy enough to strengthen these assertions,
as the code is very cold, to reduce security risk. And perhaps we will
learn something.

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:jorendorff, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8cde5fbfa29f
Strengthen assertions around stackDepth. r=nbp

Comment 11

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/8cde5fbfa29f

Comment 12

[Nicolas B. Pierron [:nbp]]

We should double check the logic of the Bytecode Parser, to ensure that this is correct.

Comment 13

[Nicolas B. Pierron [:nbp]]

Next step would be to add a debug-only phase which verify that this property of simulating each opcode works as soon as the bytecode is generated.

• If this fails, this would be reporting a bug which state that the BytecodeParser::parse function is not capable of simulating every opcode.

• If this does not fail, this would report that our bytecode gets mutated in the mean time, which is not supposed to ever happen. In which case we might consider mprotect-ing Stencil and immutable data to do a page fault on write.

Based on the number of crashes per day, this bug remains a low priority at the moment.

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

The leave-open keyword is there and there is no activity for 6 months.
:sdetar, maybe it's time to close this bug?
For more information, please visit auto_nag documentation.