Bug 97036 - A lot of ERROR messages in FractionalLayoutUnits when opening fast/overflow/overflow-height-float-not-removed-crash3.html regression test
Summary: A lot of ERROR messages in FractionalLayoutUnits when opening fast/overflow/o...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-18 12:54 PDT by Alexey Proskuryakov
Modified: 2024-03-30 18:48 PDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2012-09-18 12:54:41 PDT
18.09.12 12:47:02,850 WebProcess[15556]: ERROR: !(m_value >= 0)
18.09.12 12:47:02,850 WebProcess[15556]: /Users/ap/Safari/OpenSource/Source/WebCore/platform/FractionalLayoutUnit.h(166) : unsigned int WebCore::FractionalLayoutUnit::toUnsigned() const
18.09.12 12:47:02,850 WebProcess[15556]: ERROR: !(isInBounds(value))
18.09.12 12:47:02,850 WebProcess[15556]: /Users/ap/Safari/OpenSource/Source/WebCore/platform/FractionalLayoutUnit.h(101) : WebCore::FractionalLayoutUnit::FractionalLayoutUnit(unsigned int)
18.09.12 12:47:02,850 WebProcess[15556]: ERROR: !(m_value >= 0)
18.09.12 12:47:02,851 WebProcess[15556]: /Users/ap/Safari/OpenSource/Source/WebCore/platform/FractionalLayoutUnit.h(166) : unsigned int WebCore::FractionalLayoutUnit::toUnsigned() const
18.09.12 12:47:02,851 WebProcess[15556]: ERROR: !(isInBounds(value))
18.09.12 12:47:02,851 WebProcess[15556]: /Users/ap/Safari/OpenSource/Source/WebCore/platform/FractionalLayoutUnit.h(101) : WebCore::FractionalLayoutUnit::FractionalLayoutUnit(unsigned int)
18.09.12 12:47:02,851 WebProcess[15556]: ERROR: !(m_value >= 0)
18.09.12 12:47:02,851 WebProcess[15556]: /Users/ap/Safari/OpenSource/Source/WebCore/platform/FractionalLayoutUnit.h(166) : unsigned int WebCore::FractionalLayoutUnit::toUnsigned() const
18.09.12 12:47:02,851 WebProcess[15556]: ERROR: !(isInBounds(value))
18.09.12 12:47:02,851 WebProcess[15556]: /Users/ap/Safari/OpenSource/Source/WebCore/platform/FractionalLayoutUnit.h(101) : WebCore::FractionalLayoutUnit::FractionalLayoutUnit(unsigned int)
18.09.12 12:47:02,852 WebProcess[15556]: ERROR: !(m_value >= 0)
18.09.12 12:47:02,852 WebProcess[15556]: /Users/ap/Safari/OpenSource/Source/WebCore/platform/FractionalLayoutUnit.h(166) : unsigned int WebCore::FractionalLayoutUnit::toUnsigned() const
18.09.12 12:47:02,852 WebProcess[15556]: ERROR: !(isInBounds(value))
18.09.12 12:47:02,852 WebProcess[15556]: /Users/ap/Safari/OpenSource/Source/WebCore/platform/FractionalLayoutUnit.h(101) : WebCore::FractionalLayoutUnit::FractionalLayoutUnit(unsigned int)
Comment 1 Levi Weintraub 2012-09-18 14:40:56 PDT
We display this information on Debug builds as a tool to notify developers that we're overflowing a LayoutUnit, as this is often a source of bugs and improperly handled conditions. This particular warning shows that a negative number is being cast into an unsigned, which is rarely what one wants.

Are you suggesting we remove these warnings (/suppress them by default) or do you have a suggestion of a way to present this data in a more helpful, less distracting way?
Comment 2 Alexey Proskuryakov 2012-09-18 15:27:58 PDT
I think that this bug needs to be fixed by whoever uses LayoutUnits incorrectly.

Not sure if there is complete consensus on what LOG_ERROR should be used for. I think that it's really usually about things largely outside WebKit control, like corrupt sqlite databases. Broken HTML input should never cause any sort of console logging.

Using LOG_ERROR as a temporary solution before turning it into an ASSERT seems very reasonable to me, as long as we agree that making it an assertion is the goal.
Comment 3 Levi Weintraub 2012-09-18 15:39:57 PDT
(In reply to comment #2)
> I think that this bug needs to be fixed by whoever uses LayoutUnits incorrectly.

Here's a possible culprit: https://bugs.webkit.org/show_bug.cgi?id=68744

I'll have to dig in when I have some time to positively identify this particular issue.

> Not sure if there is complete consensus on what LOG_ERROR should be used for. I think that it's really usually about things largely outside WebKit control, like corrupt sqlite databases. Broken HTML input should never cause any sort of console logging.
> 
> Using LOG_ERROR as a temporary solution before turning it into an ASSERT seems very reasonable to me, as long as we agree that making it an assertion is the goal.

To be clear, the logging only happens on Debug builds.

I agree in theory with what you're saying. It's currently possible to overflow rendering code any number of ways, and usually we handle it in some sort of graceful way, such as maxing with zero when we expect positive numbers. That and outstanding issues such as the one I mentioned prevented us from going with ASSERTS from the beginning (it was our initial implementation). Casting negative numbers to unsigned is almost never the right thing to do, so this particular check would likely be a great candidate for an ASSERT.

Longterm, Emil is experimenting with saturated math that would enable us to ASSERT for any actual overflows.
Comment 4 Ahmad Saleem 2024-03-30 18:48:27 PDT
FractionalLayoutUnit.h got changed into 'LayoutUnit.h': https://github.com/WebKit/WebKit/commit/6acc3f9e4fbe34c3519d021ee6836f0dbabffc36

The following test is skipped on Mac [Debug]:

https://searchfox.org/wubkat/rev/da53e5336a9c4f2e056e219a184c4f908cda7f02/LayoutTests/platform/mac/TestExpectations#530

> # Overflowing LayoutUnits cause RenderGeometryMap assertions

With: webkit.org/b/67434

I think this is duplicate of bug 67434 or to fix in it or update the bug URL in Test Expectation.