Closed Bug 1752624 Opened 3 years ago Closed 3 years ago

nsCSSRenderingBorders.cpp:2182:20: runtime error: -287 is outside the range of representable values of type 'unsigned long'

Categories

(Core :: Web Painting, defect, P3)

Product:
Core
Component:
Web Painting
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
100 Branch
Tracking Flags:
Tracking Status
firefox98 --- wontfix
firefox99 --- wontfix
firefox100 --- fixed

People

(Reporter: tsmith, Assigned: arai)

References

(Blocks 2 open bugs)

Details

(Keywords: csectype-undefined, testcase)

Attachments

(3 files, 1 obsolete file)

testcase.html
279 bytes, text/html
Details
border-print.png
58.71 KB, image/png
Details
Bug 1752624 - Skip drawing border if the dirty rect and the border rect do not intersect. r?miko!
48 bytes, text/x-phabricator-request
Details | Review
Attached file testcase.html —

This was found by enabling the float-cast-overflow check in UBSan and running existing tests. This type of issue can create inconsistencies across platforms, architectures and optimization levels.

Found with m-c 20220127-4dfa6c06a936

To enable this check add the following to your mozconfig:

ac_add_options --enable-undefined-sanitizer="float-cast-overflow"

src/layout/painting/nsCSSRenderingBorders.cpp:2182:20: runtime error: -287 is outside the range of representable values of type 'unsigned long'
    #0 0x7fb7d78bb6a9 in nsCSSBorderRenderer::DrawDottedSideSlow(mozilla::Side) src/layout/painting/nsCSSRenderingBorders.cpp:2182:20
    #1 0x7fb7d78b824b in nsCSSBorderRenderer::DrawDashedOrDottedSide(mozilla::Side) src/layout/painting/nsCSSRenderingBorders.cpp:1770:5
    #2 0x7fb7d788ed84 in nsCSSBorderRenderer::DrawBorders() src/layout/painting/nsCSSRenderingBorders.cpp:3309:9
    #3 0x7fb7d7891331 in nsCSSRendering::PaintNonThemedOutline(nsPresContext*, gfxContext&, nsIFrame*, nsRect const&, nsRect const&, mozilla::ComputedStyle*) src/layout/painting/nsCSSRendering.cpp:1039:7
    #4 0x7fb7d78f5aa9 in mozilla::nsDisplayOutline::Paint(mozilla::nsDisplayListBuilder*, gfxContext*) src/layout/painting/nsDisplayList.cpp:4022:3
    #5 0x7fb7d7883d32 in mozilla::nsDisplayList::Paint(mozilla::nsDisplayListBuilder*, gfxContext*, int) src/layout/painting/nsDisplayList.cpp:2141:11
    #6 0x7fb7d7911a7d in mozilla::nsDisplayTransform::Paint(mozilla::nsDisplayListBuilder*, gfxContext*, mozilla::Maybe<mozilla::gfx::PolygonTyped<mozilla::gfx::UnknownUnits> > const&) src/layout/painting/nsDisplayList.cpp:6855:20
    #7 0x7fb7d79111d6 in mozilla::nsDisplayTransform::Paint(mozilla::nsDisplayListBuilder*, gfxContext*) src/layout/painting/nsDisplayList.cpp:6822:3
    #8 0x7fb7d7883d32 in mozilla::nsDisplayList::Paint(mozilla::nsDisplayListBuilder*, gfxContext*, int) src/layout/painting/nsDisplayList.cpp:2141:11
    #9 0x7fb7d7911a7d in mozilla::nsDisplayTransform::Paint(mozilla::nsDisplayListBuilder*, gfxContext*, mozilla::Maybe<mozilla::gfx::PolygonTyped<mozilla::gfx::UnknownUnits> > const&) src/layout/painting/nsDisplayList.cpp:6855:20
    #10 0x7fb7d79111d6 in mozilla::nsDisplayTransform::Paint(mozilla::nsDisplayListBuilder*, gfxContext*) src/layout/painting/nsDisplayList.cpp:6822:3
    #11 0x7fb7d7883d32 in mozilla::nsDisplayList::Paint(mozilla::nsDisplayListBuilder*, gfxContext*, int) src/layout/painting/nsDisplayList.cpp:2141:11
    #12 0x7fb7d78e109c in mozilla::nsDisplayList::PaintRoot(mozilla::nsDisplayListBuilder*, gfxContext*, unsigned int, mozilla::Maybe<double>) src/layout/painting/nsDisplayList.cpp:2206:5
    #13 0x7fb7d71ff8a5 in nsLayoutUtils::PaintFrame(gfxContext*, nsIFrame*, nsRegion const&, unsigned int, mozilla::nsDisplayListBuilderMode, nsLayoutUtils::PaintFrameFlags) src/layout/base/nsLayoutUtils.cpp:3427:9
    #14 0x7fb7d75386c8 in nsPageSequenceFrame::PrintNextSheet() src/layout/generic/nsPageSequenceFrame.cpp:693:3
    #15 0x7fb7d798c4a6 in nsPrintJob::PrintSheet(nsPrintObject*, bool&) src/layout/printing/nsPrintJob.cpp:2220:31
    #16 0x7fb7d798be41 in nsPagePrintTimer::Run() src/layout/printing/nsPagePrintTimer.cpp:74:43
    #17 0x7fb7ce8dde8c in mozilla::SchedulerGroup::Runnable::Run() src/xpcom/threads/SchedulerGroup.cpp:140:20
    #18 0x7fb7ce93407a in mozilla::RunnableTask::Run() src/xpcom/threads/TaskController.cpp:467:16
    #19 0x7fb7ce8f63df in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:770:26
    #20 0x7fb7ce8f3a2e in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:606:15
    #21 0x7fb7ce8f4184 in mozilla::TaskController::ProcessPendingMTTask(bool) src/xpcom/threads/TaskController.cpp:390:36
    #22 0x7fb7ce926264 in mozilla::TaskController::InitializeInternal()::$_1::operator()() const src/xpcom/threads/TaskController.cpp:127:37
    #23 0x7fb7ce926264 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_1>::Run() src/xpcom/threads/nsThreadUtils.h:531:5
    #24 0x7fb7ce911da3 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1195:16
    #25 0x7fb7ce91b014 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:467:10
    #26 0x7fb7d1fd7647 in bool mozilla::SpinEventLoopUntil<(mozilla::ProcessFailureBehavior)1, nsGlobalWindowOuter::Print(nsIPrintSettings*, nsIWebProgressListener*, nsIDocShell*, nsGlobalWindowOuter::IsPreview, nsGlobalWindowOuter::IsForWindowDotPrint, std::function<void (mozilla::dom::PrintPreviewResultInfo const&)>&&, mozilla::ErrorResult&)::$_3>(nsTSubstring<char> const&, nsGlobalWindowOuter::Print(nsIPrintSettings*, nsIWebProgressListener*, nsIDocShell*, nsGlobalWindowOuter::IsPreview, nsGlobalWindowOuter::IsForWindowDotPrint, std::function<void (mozilla::dom::PrintPreviewResultInfo const&)>&&, mozilla::ErrorResult&)::$_3&&, nsIThread*) src/objdir-ff-ubsan/dist/include/mozilla/SpinEventLoopUntil.h:176:25
    #27 0x7fb7d1fd32f9 in nsGlobalWindowOuter::Print(nsIPrintSettings*, nsIWebProgressListener*, nsIDocShell*, nsGlobalWindowOuter::IsPreview, nsGlobalWindowOuter::IsForWindowDotPrint, std::function<void (mozilla::dom::PrintPreviewResultInfo const&)>&&, mozilla::ErrorResult&) src/dom/base/nsGlobalWindowOuter.cpp:5441:5
    #28 0x7fb7d1fd12e1 in nsGlobalWindowOuter::PrintOuter(mozilla::ErrorResult&) src/dom/base/nsGlobalWindowOuter.cpp:5238:3
    #29 0x7fb7d1f6b6cf in nsGlobalWindowInner::Print(mozilla::ErrorResult&) src/dom/base/nsGlobalWindowInner.cpp:3771:3
    #30 0x7fb7d36f6c33 in mozilla::dom::Window_Binding::print(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) src/objdir-ff-ubsan/dom/bindings/WindowBinding.cpp:3118:24
    #31 0x7fb7d3f1ee06 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::MaybeCrossOriginObjectThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) src/dom/bindings/BindingUtils.cpp:3306:13
    #32 0x7fb7dbba15e0 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) src/js/src/vm/Interpreter.cpp:425:13
    #33 0x7fb7dbba15e0 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) src/js/src/vm/Interpreter.cpp:512:12
    #34 0x7fb7dbba2d04 in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) src/js/src/vm/Interpreter.cpp:572:10
    #35 0x7fb7dbb8e650 in js::CallFromStack(JSContext*, JS::CallArgs const&) src/js/src/vm/Interpreter.cpp:576:10
    #36 0x7fb7dbb8e650 in Interpret(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:3309:16
    #37 0x7fb7dbb737d3 in js::RunScript(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:394:13
    #38 0x7fb7dbba16dd in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) src/js/src/vm/Interpreter.cpp:544:13
    #39 0x7fb7dbba2d04 in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) src/js/src/vm/Interpreter.cpp:572:10
    #40 0x7fb7dbba2ed3 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) src/js/src/vm/Interpreter.cpp:589:8
    #41 0x7fb7dbe39064 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) src/js/src/vm/CallAndConstruct.cpp:117:10
    #42 0x7fb7d361e11c in mozilla::dom::IdleRequestCallback::Call(mozilla::dom::BindingCallContext&, JS::Handle<JS::Value>, mozilla::dom::IdleDeadline&, mozilla::ErrorResult&) src/objdir-ff-ubsan/dom/bindings/WindowBinding.cpp:852:8
    #43 0x7fb7d20d1670 in mozilla::dom::IdleRequestCallback::Call(mozilla::dom::IdleDeadline&, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*) src/objdir-ff-ubsan/dist/include/mozilla/dom/WindowBinding.h:695:12
    #44 0x7fb7d22fbde0 in mozilla::dom::IdleRequestCallback::Call(mozilla::dom::IdleDeadline&, char const*) src/objdir-ff-ubsan/dist/include/mozilla/dom/WindowBinding.h:708:12
    #45 0x7fb7d22fbde0 in mozilla::dom::IdleRequest::IdleRun(nsPIDOMWindowInner*, double, bool) src/dom/base/IdleRequest.cpp:61:13
    #46 0x7fb7d1f44dff in nsGlobalWindowInner::RunIdleRequest(mozilla::dom::IdleRequest*, double, bool) src/dom/base/nsGlobalWindowInner.cpp:740:12
    #47 0x7fb7d1f4398c in nsGlobalWindowInner::ExecuteIdleRequest(mozilla::TimeStamp) src/dom/base/nsGlobalWindowInner.cpp:768:3
    #48 0x7fb7d1f436a4 in IdleRequestExecutor::Run() src/dom/base/nsGlobalWindowInner.cpp:609:13
    #49 0x7fb7ce93407a in mozilla::RunnableTask::Run() src/xpcom/threads/TaskController.cpp:467:16
    #50 0x7fb7ce8f63df in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:770:26
    #51 0x7fb7ce8f3d84 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:648:15
    #52 0x7fb7ce8f4184 in mozilla::TaskController::ProcessPendingMTTask(bool) src/xpcom/threads/TaskController.cpp:390:36
    #53 0x7fb7ce926231 in mozilla::TaskController::InitializeInternal()::$_0::operator()() const src/xpcom/threads/TaskController.cpp:124:37
    #54 0x7fb7ce926231 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_0>::Run() src/xpcom/threads/nsThreadUtils.h:531:5
    #55 0x7fb7ce911da3 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1195:16
    #56 0x7fb7ce91b014 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:467:10
    #57 0x7fb7cfe000c2 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:85:21
    #58 0x7fb7cfe016a2 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:268:30
    #59 0x7fb7cfc74b01 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:331:10
    #60 0x7fb7cfc74b01 in MessageLoop::RunHandler() src/ipc/chromium/src/base/message_loop.cc:324:3
    #61 0x7fb7cfc74b01 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:306:3
    #62 0x7fb7d6b719f8 in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
    #63 0x7fb7db8a53e7 in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:870:20
    #64 0x7fb7cfe01681 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:235:9
    #65 0x7fb7cfc74b01 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:331:10
    #66 0x7fb7cfc74b01 in MessageLoop::RunHandler() src/ipc/chromium/src/base/message_loop.cc:324:3
    #67 0x7fb7cfc74b01 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:306:3
    #68 0x7fb7db8a454b in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:707:34
    #69 0x7fb7db8b9690 in mozilla::BootstrapImpl::XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/Bootstrap.cpp:67:12
    #70 0x55b7ff70ffe5 in content_process_main(mozilla::Bootstrap*, int, char**) src/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
    #71 0x55b7ff7103f5 in main src/browser/app/nsBrowserApp.cpp:327:18
    #72 0x7fb7f7855bf6 in __libc_start_main /build/glibc-S9d2JN/glibc-2.27/csu/../csu/libc-start.c:310
    #73 0x55b7ff65f0a8 in _start (src/objdir-ff-ubsan/dist/bin/firefox+0xf20a8)

The severity field is not set for this bug.
:miko, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(mikokm)
Severity: -- → S3
Flags: needinfo?(mikokm)
Priority: -- → P3

This issue is currently triggered fuzzing when the 'float-cast-overflow' UBSan check is enabled. This issue will need to be addressed before the check can be enabled by default.

If it requires too much effort to fix immediately please ni? me and let me know. If necessary it will be added to a suppression list. Thank you :)

Flags: needinfo?(mikokm)

:arai, would you mind taking a look at this? This code seems to have been added in bug 382721.

Flags: needinfo?(mikokm) → needinfo?(arai.unmht)
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED

so far, the issue is simply that negative region wasn't supported.
the patch should fix it.

Then, I have 2 questions:

  1. the code path is not used for display media (web render code path is used instead, that doesn't use the border renderer added by bug 382721). is it possible to disable web render for display media? (otherwise I'm not sure how to write testcase)
  2. for me, the testcase results in ~17000 pages in print. is it expected? or is it also some kind of bug?
Flags: needinfo?(arai.unmht) → needinfo?(mikokm)

(In reply to Tooru Fujisawa [:arai] from comment #5)

so far, the issue is simply that negative region wasn't supported.
the patch should fix it.

Then, I have 2 questions:

  1. the code path is not used for display media (web render code path is used instead, that doesn't use the border renderer added by bug 382721). is it possible to disable web render for display media? (otherwise I'm not sure how to write testcase)
  2. for me, the testcase results in ~17000 pages in print. is it expected? or is it also some kind of bug?

The testcase has some non-standard CSS (box-ordinal-group, marquee) and tries to print the page. I think we can leave the test out if it is difficult to add.

This code is used by "drawSnapshot" path and by SVG tests that use borders. This can be enabled with pref "reftest.use-draw-snapshot". If the test case requires printing, you can try using reftest-paged to test it in printing mode.

Flags: needinfo?(mikokm)

Thanks, reftest with that pref helps :)

Then, I realized that it's strange that the negative region reaches the code.
It means that, the border is completely out of the dirty rect, and it won't be rendered.
to my understanding, in that case the entire border rendering should be skipped.

maybe there's something different for printing?
the buggy case happens during printing the second page, while the border appears only in the first page in the preview.

Attached image border-print.png —

This is what I observe with the testcase.
during printing the second page, the border is rendered in the negative region, where the dirty rect is in the positive region (of course).

The algorithm there assumes that the border rect and the dirty rect overlaps (otherwise entering the code path doesn't make sense), and it narrows the target area from the entire border rect down to the overlap rect.

But in this case, those rects don't overlap, and the code results in calculating meaning-less value, that's negative value.
So, I think the patch isn't correct, but there should be some guard that it doesn't enter the border rendering for such case.
or, maybe there's already such guard, but it gets tripped because of table+frameset ?

Apparently, the filtering is done based on nsIFrame::InkOverflowRect, that reflects the inner table+frameset size, where the table is too long that overlaps with the dirty rect,
but the outline rendered there (the "border" in the above attachment) doesn't reflect those table+frameset
that results in not-overlapping case.

Simple solution would be add yet another filtering immediately before the border renderer.

https://searchfox.org/mozilla-central/rev/d4d7611ee4dd0003b492b865bc5988a4e6afc985/layout/generic/nsIFrame.cpp#3989

/**
 * Check if a frame should be visited for building display list.
 */
static bool DescendIntoChild(nsDisplayListBuilder* aBuilder,
                             const nsIFrame* aChild, const nsRect& aVisible,
                             const nsRect& aDirty) {
...
  nsRect overflow = aChild->InkOverflowRect();
Attachment #9266915 - Attachment is obsolete: true
Pushed by arai_a@mac.com: https://hg.mozilla.org/integration/autoland/rev/bf5828257d6e Skip drawing border if the dirty rect and the border rect do not intersect. r=miko

Backed out changeset bf5828257d6e (Bug 1752624) for causing reftest failures on caret_on_presshell_reinit-2.html.
Backout link
Push with failures
Failure Log

Flags: needinfo?(arai.unmht)
Pushed by nbeleuzu@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/57cf4394d8bf Skip drawing border if the dirty rect and the border rect do not intersect. r=miko

Re-land it because the failure wasn`t caused by this changes

Flags: needinfo?(arai.unmht)

https://hg.mozilla.org/mozilla-central/rev/57cf4394d8bf

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
status-firefox100: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 100 Branch

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

status-firefox99: --- → ?

The mis-behavior of the code won't affect the actual rendering result, given it's out of the visible area anyway.
So I think it's fine to make beta wontfix.

Regressions: 1761756
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: