Find while fuzzing mozilla-central asan debug rev: 404531:861067332bac ==6759==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f0db40ed21b bp 0x7ffcfb901ed0 sp 0x7ffcfb901dc0 T0) ==6759==The signal is caused by a WRITE memory access. ==6759==Hint: address points to the zero page. #0 0x7f0db40ed21a in nsStyleChangeList::AppendChange(nsIFrame*, nsIContent*, nsChangeHint) /builds/worker/workspace/build/src/layout/base/nsStyleChangeList.cpp:55:9 #1 0x7f0db3ffca07 in mozilla::UpdateOneAdditionalStyleContext(nsIFrame*, unsigned int, mozilla::ServoStyleContext&, mozilla::ServoRestyleState&) /builds/worker/workspace/build/src/layout/base/ServoRestyleManager.cpp:652:32 #2 0x7f0db3fe344c in mozilla::UpdateAdditionalStyleContexts(nsIFrame*, mozilla::ServoRestyleState&) /builds/worker/workspace/build/src/layout/base/ServoRestyleManager.cpp:670:5 #3 0x7f0db3fe24bb in mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /builds/worker/workspace/build/src/layout/base/ServoRestyleManager.cpp:911:7 #4 0x7f0db3fe273c in mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /builds/worker/workspace/build/src/layout/base/ServoRestyleManager.cpp:955:32 #5 0x7f0db3fe4f58 in mozilla::ServoRestyleManager::DoProcessPendingRestyles(mozilla::ServoTraversalFlags) /builds/worker/workspace/build/src/layout/base/ServoRestyleManager.cpp:1144:28 #6 0x7f0db3fb0b9a in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /builds/worker/workspace/build/src/layout/base/PresShell.cpp:4213:41 #7 0x7f0db3f41545 in nsRefreshDriver::Tick(long, mozilla::TimeStamp) /builds/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:1872:18 #8 0x7f0db3f4abde in mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) /builds/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:306:7 #9 0x7f0db3f4a98c in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) /builds/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:328:5 #10 0x7f0db3f4e00f in mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::TimeStamp) /builds/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:769:5 #11 0x7f0db3f4cf85 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) /builds/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:682:35 #12 0x7f0db3f4c6bb in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) /builds/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:583:9 #13 0x7f0db459d7a9 in mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) /builds/worker/workspace/build/src/layout/ipc/VsyncChild.cpp:68:16 #14 0x7f0daee9ce8b in mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:155:20 #15 0x7f0daed90c60 in mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:1812:28 #16 0x7f0daea4082d in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /builds/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:2110:25 #17 0x7f0daea3e466 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /builds/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:2040:17 #18 0x7f0daea3f04e in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) /builds/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1886:5 #19 0x7f0daea3f7af in mozilla::ipc::MessageChannel::MessageTask::Run() /builds/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1919:15 #20 0x7f0dade221bc in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1033:14 #21 0x7f0dade44938 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:508:10 #22 0x7f0daea478b3 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:97:21 #23 0x7f0dae98d7a8 in MessageLoop::RunInternal() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10 #24 0x7f0dae98d62c in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299:3 #25 0x7f0db39fcc8a in nsBaseAppShell::Run() /builds/worker/workspace/build/src/widget/nsBaseAppShell.cpp:157:27 #26 0x7f0db6ef1f80 in XRE_RunAppShell() /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:865:22 #27 0x7f0daea48515 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:269:9 #28 0x7f0dae98d7a8 in MessageLoop::RunInternal() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10 #29 0x7f0dae98d62c in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299:3 #30 0x7f0db6ef16de in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:691:34 #31 0x4ef336 in content_process_main(mozilla::Bootstrap*, int, char**) /builds/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:63:30 #32 0x4ef5be in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:280:18 #33 0x7f0dcd30d82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291 #34 0x41f114 in _start (/home/rforbes/fuzzing/builds/mc-asan-debug/firefox+0x41f114)

The crashing line seems to be https://searchfox.org/mozilla-central/rev/47cb352984bac15c476dcd75f8360f902673cb98/layout/base/nsStyleChangeList.cpp#55-59 But that doesn't really make much sense from a brief look... If "this" is null there, it should have crashed several lines before, when checking IsServo() or getting Length(). As far as it has got to the inner loop, it doesn't seem to me there is anything there nullable...

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #3) > But that doesn't really make much sense from a brief look... If "this" is > null there, it should have crashed several lines before, when checking > IsServo() or getting Length(). As far as it has got to the inner loop, it > doesn't seem to me there is anything there nullable... There's no null deref, right? This is just the assertion firing.

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #3) > The crashing line seems to be > https://searchfox.org/mozilla-central/rev/ > 47cb352984bac15c476dcd75f8360f902673cb98/layout/base/nsStyleChangeList. > cpp#55-59 btw, shouldn't that loop be #ifdef DEBUG? It iterates over nsStyleChangeList Length() but the loop body is just a debug MOZ_ASSERT.

(In reply to Emilio Cobos Álvarez [:emilio] from comment #4) > There's no null deref, right? This is just the assertion firing. Comment 0 says "SEGV on unknown address 0x000000000000" which sounds like a null-deref, doesn't it?

Comment on attachment 8952632 [details] Bug 1439793: Simplify nsStyleChangeList::PopChangesForContent. https://reviewboard.mozilla.org/r/221878/#review227752

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #9) > (In reply to Emilio Cobos Álvarez [:emilio] from comment #4) > > There's no null deref, right? This is just the assertion firing. > > Comment 0 says "SEGV on unknown address 0x000000000000" which sounds like a > null-deref, doesn't it? That's only what our assertions do to crash I think...

Comment on attachment 8952633 [details] Bug 1439793: Handle correctly a reconstruct generated from additional style contexts. https://reviewboard.mozilla.org/r/221880/#review227756

Comment on attachment 8952634 [details] Bug 1439793: Crashtest. https://reviewboard.mozilla.org/r/221882/#review227758 ::: layout/style/crashtests/1439793.html:2 (Diff revision 1) > + try { o1 = document.createElementNS("http://www.w3.org/1998/Math/MathML", "mfenced") } catch(e) { } > + try { try { o2.appendChild(o1) } catch(e) { document.documentElement.appendChild(o1) } } catch(e) { } > + try { o3 = document.createElementNS("http://www.w3.org/1999/xhtml", "style") } catch(e) { } > + try { o3.appendChild(document.createTextNode("[s=\"\"]{")) } catch(e) { } > + try { (document.head || document.children[0]).appendChild(o3) } catch(e) { } > + try { o4 = o3.sheet } catch(e) { } > + try { window.scrollBy(0.41979783887602806, 16) } catch(e) { } > + try { o4.insertRule("::-moz-math-anonymous,g{scroll-behavior:smooth", 1) } catch(e) { } I don't really like those `try {} catch {}` stuff... It would be great if you can just unwrap all the lines. But it's not a big deal, so I'm fine as-is, too.

Comment on attachment 8952647 [details] Bug 1439793: Remove unneeded nullcheck, and ifdef a debug-only loop. https://reviewboard.mozilla.org/r/221894/#review227762 ::: layout/base/nsStyleChangeList.cpp:46 (Diff revision 1) > MOZ_ASSERT(!(aHint & nsChangeHint_AllReflowHints) || > (aHint & nsChangeHint_NeedReflow), > "Reflow hint bits set without actually asking for a reflow"); > > - if (aContent && (aHint & nsChangeHint_ReconstructFrame)) { > + if (aHint & nsChangeHint_ReconstructFrame) { > + MOZ_ASSERT(aContent, "What are we reconstructing?"); This assertion seems meaningless. Several lines above there is an assertion of `aContent || !(aHint & nsChangeHint_ReconstructFrame)` which is exactly what you are asserting here.

Comment on attachment 8952634 [details] Bug 1439793: Crashtest. https://reviewboard.mozilla.org/r/221882/#review227766 ::: layout/style/crashtests/1439793.html:9 (Diff revision 2) > + o3 = document.createElementNS("http://www.w3.org/1999/xhtml", "style"); > + o3.appendChild(document.createTextNode("[s=\"\"]{")); > + (document.head || document.children[0]).appendChild(o3); > + o4 = o3.sheet; > + window.scrollBy(0.41979783887602806, 16); > + o4.insertRule("::-moz-math-anonymous,g{scroll-behavior:smooth", 1) } catch(e) { } Redundant `} cache(e) { }` at the end of this line.

Comment on attachment 8952634 [details] Bug 1439793: Crashtest. https://reviewboard.mozilla.org/r/221882/#review227766 > Redundant `} cache(e) { }` at the end of this line. Whoops, thanks

Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/autoland/rev/2e6fed1f75ae Simplify nsStyleChangeList::PopChangesForContent. r=xidorn https://hg.mozilla.org/integration/autoland/rev/601daa76d780 Handle correctly a reconstruct generated from additional style contexts. r=xidorn https://hg.mozilla.org/integration/autoland/rev/9cffcaa22229 Crashtest. r=xidorn https://hg.mozilla.org/integration/autoland/rev/9c21c92c1b3d Remove unneeded nullcheck, and ifdef a debug-only loop. r=xidorn

https://hg.mozilla.org/mozilla-central/rev/2e6fed1f75ae https://hg.mozilla.org/mozilla-central/rev/601daa76d780 https://hg.mozilla.org/mozilla-central/rev/9cffcaa22229 https://hg.mozilla.org/mozilla-central/rev/9c21c92c1b3d