1745142 - Crash in [@ nsHtml5TreeBuilder::loadState]

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Calixte Denizet (:calixte)]

Maybe Fission related. (DOMFissionEnabled=1)

Crash report: https://crash-stats.mozilla.org/report/index/29446e3c-24d0-415d-a4b3-214790211209

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames of crashing thread:

0 xul.dll nsHtml5TreeBuilder::loadState parser/html/nsHtml5TreeBuilder.cpp:4625
1 xul.dll nsHtml5Parser::InitializeDocWriteParserState parser/html/nsHtml5Parser.cpp:688
2 xul.dll nsHtml5TreeOperation::Perform parser/html/nsHtml5TreeOperation.cpp:1209
3 xul.dll nsHtml5TreeOpExecutor::RunFlushLoop parser/html/nsHtml5TreeOpExecutor.cpp:658
4 xul.dll nsHtml5EncodingCommitter::Run parser/html/nsHtml5StreamParser.cpp:205
5 xul.dll mozilla::SchedulerGroup::Runnable::Run xpcom/threads/SchedulerGroup.cpp:144
6 xul.dll mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal xpcom/threads/TaskController.cpp:771
7 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1183
8 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:85
9 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:324

There are 2 crashes (from 2 installations) in nightly 97 with buildid 20211208214306. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1701828.

[1] https://hg.mozilla.org/mozilla-central/rev?node=9a8abd87cc79

Comment 1

[Henri Sivonen (:hsivonen)]

This is unlikely to have anything to do with Fission vs. non-Fission. This looks like a real regression from bug 1701828, but we shouldn't back that one out and should instead seek to understand what's wrong here.

It looks like we're trying to execute a script from the encoding commitment flush, and either this or the argument is bad. The argument is null-checked earlier, but since the access violation is on read rather than write, it still suggests that the bad address is the snapshot address rather than the nsHtml5Parser and its member nsHtml5TreeBuilder being bad.

I don't have access to the crash metadata. calixte, can you see a URL for repro?

Comment 2

[Henri Sivonen (:hsivonen)]

This might have the same root cause as bug 1745139, but there's enough inlining going on that I'm not sure. I'm going to try fixing the other bug first.

Comment 3

[Henri Sivonen (:hsivonen)]

It's very likely that the patch in bug 1745139 fixes this one as well.

Comment 4

[Henri Sivonen (:hsivonen)]

(In reply to Henri Sivonen (:hsivonen) from comment #3)

It's very likely that the patch in bug 1745139 fixes this one as well.

It didn't.

The crashes after the landing of that patch make this one a bit clearer:
The loop in https://searchfox.org/mozilla-central/rev/4646b826a25d3825cf209db890862b45fa09ffc3/parser/html/nsHtml5TreeOpExecutor.cpp#831 end up trying to run a script, and things go wrong from there.

Chances are that we ended up not processing the encoding commitment via the speculative load queue. We probably took this branch earlier:
https://searchfox.org/mozilla-central/rev/4646b826a25d3825cf209db890862b45fa09ffc3/parser/html/nsHtml5TreeBuilderCppSupplement.h#1261

But why?

Comment 5

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1745142 - Communicate encoding commitment via speculative load queue.

Comment 6

[Henri Sivonen (:hsivonen)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=54d0edcdb38a1b46b53fa15a470a5df6a0478c97

Comment 7

[Henri Sivonen (:hsivonen)]

Previous try run crashes in widget code, which look unrelated. Let's try a rebase:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=82899d5b89d3dd3b62796e7dd2972510e9c3c9d8

Comment 8

[Henri Sivonen (:hsivonen)]

How do I find an appropriate base rev to compare with? https://treeherder.mozilla.org/perfherder/compare?originalProject=mozilla-central&originalRevision=e135fe933c4e31482e91c77617a27c04b3a44eef&newProject=try&newRevision=0010e10179c44cec14f6c982e56b32a0c9b36e4a&framework=13&page=1 doesn't work?

Comment 9

[Calixte Denizet (:calixte)]

I've no idea, maybe :sparky will.

Comment 10

[Greg Mierzwinski [:sparky]]

:hsivonen, the try run you made needs to use the shippable build variants to get a comparison with m-c

Comment 11

[Henri Sivonen (:hsivonen)]

(In reply to Greg Mierzwinski [:sparky] from comment #10)

:hsivonen, the try run you made needs to use the shippable build variants to get a comparison with m-c

Thanks. https://treeherder.mozilla.org/perfherder/compare?originalProject=mozilla-central&originalRevision=e6f994394be4fd1eb00bd8db40e8a8dcb42f5f9a&newProject=try&newRevision=0010e10179c44cec14f6c982e56b32a0c9b36e4a&framework=13&page=2&showOnlyComparable=1

Comment 12

[Henri Sivonen (:hsivonen)]

Generating a new shippable build:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4b1b3e68d4a78df9236e9544401606f4d1879b9

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/77388b0795b4
Communicate encoding commitment via speculative load queue. r=smaug

Comment 14

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/77388b0795b4