Found with m-c: BuildID=20180507140941 SourceStamp=93443d36d4bd53dba004f7b73430879f96daa681 ==25250==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x00000056d5d8 bp 0x7ffe1fbede30 sp 0x7ffe1fbedcc0 T0) ==25250==The signal is caused by a WRITE memory access. ==25250==Hint: address points to the zero page. #0 0x56d5d7 in MOZ_CrashPrintf src/mfbt/Assertions.cpp:63:3 #1 0x7f3dbab4cc6b in InvalidArrayIndex_CRASH(unsigned long, unsigned long) src/xpcom/ds/nsTArray.cpp:26:3 #2 0x7f3dc2dc4b6a in ElementAt src/obj-firefox/dist/include/nsTArray.h:1041:7 #3 0x7f3dc2dc4b6a in operator[] src/obj-firefox/dist/include/nsTArray.h:1070 #4 0x7f3dc2dc4b6a in nsFloatManager::EllipseShapeInfo::LineEdge(int, int, bool) const src/layout/generic/nsFloatManager.cpp:1007 #5 0x7f3dc2dc2104 in LineLeft src/layout/generic/nsFloatManager.cpp:2348:43 #6 0x7f3dc2dc2104 in nsFloatManager::GetFlowArea(mozilla::WritingMode, int, int, nsFloatManager::BandInfoType, nsFloatManager::ShapeType, mozilla::LogicalRect, nsFloatManager::SavedState*, nsSize const&) const src/layout/generic/nsFloatManager.cpp:225 #7 0x7f3dc2cbef1e in GetFloatAvailableSpaceWithState src/layout/generic/BlockReflowInput.cpp:345:21 #8 0x7f3dc2cbef1e in GetFloatAvailableSpace src/layout/generic/BlockReflowInput.h:134 #9 0x7f3dc2cbef1e in mozilla::BlockReflowInput::AddFloat(nsLineLayout*, nsIFrame*, int) src/layout/generic/BlockReflowInput.cpp:636 #10 0x7f3dc2f1f061 in AddFloat src/layout/generic/nsLineLayout.h:182:22 #11 0x7f3dc2f1f061 in nsLineLayout::ReflowFrame(nsIFrame*, nsReflowStatus&, mozilla::ReflowOutput*, bool&) src/layout/generic/nsLineLayout.cpp:966 #12 0x7f3dc2d5073d in nsBlockFrame::ReflowInlineFrame(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) src/layout/generic/nsBlockFrame.cpp:4158:15 #13 0x7f3dc2d4f0e7 in nsBlockFrame::DoReflowInlineFrames(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) src/layout/generic/nsBlockFrame.cpp:3958:5 #14 0x7f3dc2d45e09 in nsBlockFrame::ReflowInlineFrames(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3832:9 #15 0x7f3dc2d3e360 in nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2816:5 #16 0x7f3dc2d33be0 in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2352:7 #17 0x7f3dc2d2b3f4 in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1225:3 #18 0x7f3dc2d8b8b6 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:951:14 #19 0x7f3dc303f55b in nsHTMLButtonControlFrame::ReflowButtonContents(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsIFrame*) src/layout/forms/nsHTMLButtonControlFrame.cpp:252:3 #20 0x7f3dc303ecb0 in nsHTMLButtonControlFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/forms/nsHTMLButtonControlFrame.cpp:204:3 #21 0x7f3dc2f1ec6f in nsLineLayout::ReflowFrame(nsIFrame*, nsReflowStatus&, mozilla::ReflowOutput*, bool&) src/layout/generic/nsLineLayout.cpp:924:13 #22 0x7f3dc2d5073d in nsBlockFrame::ReflowInlineFrame(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) src/layout/generic/nsBlockFrame.cpp:4158:15 #23 0x7f3dc2d4f0e7 in nsBlockFrame::DoReflowInlineFrames(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) src/layout/generic/nsBlockFrame.cpp:3958:5 #24 0x7f3dc2d45e09 in nsBlockFrame::ReflowInlineFrames(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3832:9 #25 0x7f3dc2d3e360 in nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2816:5 #26 0x7f3dc2d33be0 in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2352:7 #27 0x7f3dc2d2b3f4 in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1225:3 #28 0x7f3dc2d4c667 in nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) src/layout/generic/nsBlockReflowContext.cpp:306:11 #29 0x7f3dc2d406e3 in nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3463:11 #30 0x7f3dc2d3e4b5 in nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2813:5 #31 0x7f3dc2d33be0 in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2352:7 #32 0x7f3dc2d2b3f4 in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1225:3 #33 0x7f3dc2d8b8b6 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:951:14 #34 0x7f3dc2d8a102 in nsCanvasFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsCanvasFrame.cpp:713:5 #35 0x7f3dc2d8b8b6 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:951:14 #36 0x7f3dc2e76658 in nsHTMLScrollFrame::ReflowScrolledFrame(mozilla::ScrollReflowInput*, bool, bool, mozilla::ReflowOutput*, bool) src/layout/generic/nsGfxScrollFrame.cpp:555:3 #37 0x7f3dc2e77a79 in nsHTMLScrollFrame::ReflowContents(mozilla::ScrollReflowInput*, mozilla::ReflowOutput const&) src/layout/generic/nsGfxScrollFrame.cpp:678:3 #38 0x7f3dc2e7ba58 in nsHTMLScrollFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsGfxScrollFrame.cpp:1055:3 #39 0x7f3dc2d0f74e in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, int, int, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:995:14 #40 0x7f3dc2d0e2ce in mozilla::ViewportFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/ViewportFrame.cpp:335:7 #41 0x7f3dc2af3c10 in mozilla::PresShell::DoReflow(nsIFrame*, bool) src/layout/base/PresShell.cpp:8983:11 #42 0x7f3dc2b09b20 in mozilla::PresShell::ProcessReflowCommands(bool) src/layout/base/PresShell.cpp:9156:24 #43 0x7f3dc2b07f29 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) src/layout/base/PresShell.cpp:4366:11 #44 0x7f3dc2be5030 in FlushPendingNotifications src/obj-firefox/dist/include/nsIPresShell.h:583:5 #45 0x7f3dc2be5030 in nsDocumentViewer::LoadComplete(nsresult) src/layout/base/nsDocumentViewer.cpp:982 #46 0x7f3dc5d3f8a2 in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) src/docshell/base/nsDocShell.cpp:7236:21 #47 0x7f3dc5d3bcc9 in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) src/docshell/base/nsDocShell.cpp:7029:7 #48 0x7f3dc5d434cf in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) src/docshell/base/nsDocShell.cpp #49 0x7f3dbc9ecef7 in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) src/uriloader/base/nsDocLoader.cpp:1315:3 #50 0x7f3dbc9ebf7a in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) src/uriloader/base/nsDocLoader.cpp:858:14 #51 0x7f3dbc9e8b55 in nsDocLoader::DocLoaderIsEmpty(bool) src/uriloader/base/nsDocLoader.cpp:747:9 #52 0x7f3dbc9eab1c in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) src/uriloader/base/nsDocLoader.cpp:632:5 #53 0x7f3dbc9ebb3c in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) src/uriloader/base/nsDocLoader.cpp #54 0x7f3dbae1fd1a in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) src/netwerk/base/nsLoadGroup.cpp:629:28 #55 0x7f3dbdda526a in DoUnblockOnload src/dom/base/nsDocument.cpp:8419:18 #56 0x7f3dbdda526a in nsDocument::UnblockOnload(bool) src/dom/base/nsDocument.cpp:8341 #57 0x7f3dbdd85bd4 in nsIDocument::DispatchContentLoadedEvents() src/dom/base/nsDocument.cpp:5321:3 #58 0x7f3dbde9cde4 in applyImpl<nsIDocument, void (nsIDocument::*)()> src/obj-firefox/dist/include/nsThreadUtils.h:1165:12 #59 0x7f3dbde9cde4 in apply<nsIDocument, void (nsIDocument::*)()> src/obj-firefox/dist/include/nsThreadUtils.h:1171 #60 0x7f3dbde9cde4 in mozilla::detail::RunnableMethodImpl<nsIDocument*, void (nsIDocument::*)(), true, (mozilla::RunnableKind)0>::Run() src/obj-firefox/dist/include/nsThreadUtils.h:1216 #61 0x7f3dbac180d1 in mozilla::SchedulerGroup::Runnable::Run() src/xpcom/threads/SchedulerGroup.cpp:337:32 #62 0x7f3dbac36ec9 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1090:14 #63 0x7f3dbac52900 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:519:10 #64 0x7f3dbbb306da in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21 #65 0x7f3dbba84f89 in RunInternal src/ipc/chromium/src/base/message_loop.cc:326:10 #66 0x7f3dbba84f89 in RunHandler src/ipc/chromium/src/base/message_loop.cc:319 #67 0x7f3dbba84f89 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299 #68 0x7f3dc253e57a in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:157:27 #69 0x7f3dc67b2f7b in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:893:22 #70 0x7f3dbba84f89 in RunInternal src/ipc/chromium/src/base/message_loop.cc:326:10 #71 0x7f3dbba84f89 in RunHandler src/ipc/chromium/src/base/message_loop.cc:319 #72 0x7f3dbba84f89 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299 #73 0x7f3dc67b2940 in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:719:34 #74 0x4f1875 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:50:30 #75 0x4f1875 in main src/browser/app/nsBrowserApp.cpp:280 #76 0x7f3dda3bc82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291 #77 0x420f48 in _start (firefox+0x420f48)

Comment on attachment 8973827 [details] Bug 1459697 Part 2: Account for the possibility that EllipseShapeInfo may not generate an interval for the entire BStart() to BEnd() range, due to rounding error in the distance field calculation. The testcase doesn't crash in my testing, but this patch will prevent future crashes.

Could you explain how this could fix a null-dereference, given that mIntervals.IsEmpty() has already been tested earlier in the function?

Oh, never mind, the null-deref is a side-effects of the way MOZ_CRASH works. Seeing the actual ASAN output would have been useful here, though.

The testcase hits the fatal assertion in a debug build for me, though.

(In reply to David Baron :dbaron: ⌚️UTC-7 from comment #5) > The testcase hits the fatal assertion in a debug build for me, though. Hmm... must be dependent on device pixel ratio is my guess. I'll try to artificially manipulate my dpr on my macOS build (zoom levels?) and see if I can get this to trigger.

With manipulation of the zoom levels, I was able to reproduce. I've uploaded a patch that solves the problem in a reasonable way. I'll add a test based on the testcase shortly.

Comment on attachment 8974117 [details] Bug 1459697 Part 1: In EllipseShapeInfo distance field calculation, remove a warning that might trigger due to rounding error, and shorten the iteration of each block pixel row, when possible. https://reviewboard.mozilla.org/r/242408/#review248802 r=dbaron, although it really seems like there should be a better way to do this whole thing than a distance field. ::: layout/generic/nsFloatManager.cpp:826 (Diff revision 1) > // comparison which builds the intervals. > dfType usedMargin5X = CalcUsedShapeMargin5X(aShapeMargin, > aAppUnitsPerDevPixel); > > + // Calculate the bounds of one quadrant of the ellipse, in integer device > + // pixels. This bounds is equal to the rectangle defined by the radii, plus "This bounds is" -> "These bounds are"

Comment on attachment 8973827 [details] Bug 1459697 Part 2: Account for the possibility that EllipseShapeInfo may not generate an interval for the entire BStart() to BEnd() range, due to rounding error in the distance field calculation. https://reviewboard.mozilla.org/r/242186/#review248806 ::: layout/generic/nsFloatManager.cpp:1014 (Diff revision 4) > + DebugOnly<nscoord> onePixelPastLastInterval = > + mIntervals[mIntervals.Length() - 1].YMost() + > + mIntervals[mIntervals.Length() - 1].Height(); > + NS_WARNING_ASSERTION(bSmallestWithinIntervals < onePixelPastLastInterval, > + "We should have found a matching interval for this " > + "block value."); Put this whole thing inside an #ifdef DEBUG instead of using DebugOnly, since I don't think the compiler will optimize away everything on the right side of the =.

Comment on attachment 8974082 [details] Bug 1459697 Part 3: Add a crashtest. https://reviewboard.mozilla.org/r/242382/#review248808 I presume you tested that this asserts prior to the patches?

Comment on attachment 8974118 [details] Bug 1459697 Part 4: Change a WPT reftest to make failures visible in red. https://reviewboard.mozilla.org/r/242410/#review248810

Pushed by bwerth@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a6e7205db0c0 Part 1: In EllipseShapeInfo distance field calculation, remove a warning that might trigger due to rounding error, and shorten the iteration of each block pixel row, when possible. r=dbaron https://hg.mozilla.org/integration/autoland/rev/bf95429788de Part 2: Account for the possibility that EllipseShapeInfo may not generate an interval for the entire BStart() to BEnd() range, due to rounding error in the distance field calculation. r=dbaron https://hg.mozilla.org/integration/autoland/rev/ffff86f5db2e Part 3: Add a crashtest. r=dbaron https://hg.mozilla.org/integration/autoland/rev/731dfa211b38 Part 4: Change a WPT reftest to make failures visible in red. r=dbaron

Created web-platform-tests PR https://github.com/w3c/web-platform-tests/pull/10952 for changes under testing/web-platform/tests

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

https://hg.mozilla.org/mozilla-central/rev/a6e7205db0c0 https://hg.mozilla.org/mozilla-central/rev/bf95429788de https://hg.mozilla.org/mozilla-central/rev/ffff86f5db2e https://hg.mozilla.org/mozilla-central/rev/731dfa211b38

Please request Beta approval on this when you get a chance.

Upstream PR merged

Comment on attachment 8974117 [details] Bug 1459697 Part 1: In EllipseShapeInfo distance field calculation, remove a warning that might trigger due to rounding error, and shorten the iteration of each block pixel row, when possible. Approval Request Comment [Feature/Bug causing the regression]: Bug 1265342 [User impact if declined]: Out-of-bounds memory read on sites that use shape-outside: ellipse with shape-margin. [Is this code covered by automated tests?]: Yes. [Has the fix been verified in Nightly?]: Yes. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: shape-margin is not in wide usage yet; this is our first release that enables it, and the feature is still behind a pref (except in Nightly). [String changes made/needed]: None.

(In reply to Brad Werth [:bradwerth] from comment #32) > Comment on attachment 8974117 [details] > [List of other uplifts needed for the feature/fix]: None. To be clear, all 4 parts of the patch should be uplifted. I only filled out the uplift request on part 1.

Comment on attachment 8974117 [details] Bug 1459697 Part 1: In EllipseShapeInfo distance field calculation, remove a warning that might trigger due to rounding error, and shorten the iteration of each block pixel row, when possible. Avoids shipping a new crash caused by bug 1265342. Approved for 61.0b5.

https://hg.mozilla.org/releases/mozilla-beta/rev/c2cc19eb1159 https://hg.mozilla.org/releases/mozilla-beta/rev/b6cde02169dc https://hg.mozilla.org/releases/mozilla-beta/rev/9bbd3253d3a5 https://hg.mozilla.org/releases/mozilla-beta/rev/5fcefe47c777