Closed Bug 1171528 Opened 5 years ago Closed 5 years ago

"WARNING: Overflowed nscoord_MAX in conversion to nscoord width" logged over 65,000 times in linux debug test logs

Categories

(Core :: Graphics, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: erahm, Assigned: erahm)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

A NS_WARNING in |SaturatingUnionEdges| is hit over 65,000 times in linux debug test logs generating the messsage:

> [NNNNN] WARNING: Overflowed nscoord_MAX in conversion to nscoord width: file ../../dist/include/nsRect.h, line 83

Either we're doing something horribly wrong (I somewhat doubt that due to the frequency of this message) and it needs to be fixed or we should consider removing the warning.
Assignee: nobody → lsalzman
(In reply to Eric Rahm [:erahm] from comment #0)
> A NS_WARNING in |SaturatingUnionEdges| is hit over 65,000 times in linux
> debug test logs generating the messsage:
> 
> > [NNNNN] WARNING: Overflowed nscoord_MAX in conversion to nscoord width: file ../../dist/include/nsRect.h, line 83
> 
> Either we're doing something horribly wrong (I somewhat doubt that due to
> the frequency of this message) and it needs to be fixed or we should
> consider removing the warning.

Any more information you could give as to some specific tests this is occurring on and/or a link to some set of logs where this can be found in volumes?
Flags: needinfo?(erahm)
(In reply to Lee Salzman [:eihrul] from comment #1)
> Any more information you could give as to some specific tests this is
> occurring on and/or a link to some set of logs where this can be found in
> volumes?

It seems to all be from crashtest. An example log: http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-debug/1433523286/mozilla-central_ubuntu64_vm-debug_test-crashtest-bm122-tests1-linux64-build19.txt.gz
Flags: needinfo?(erahm)
Crashtest log w/ pids and timestamps normalized out.

The command I used to accumulate all the instances:

> cat mozilla-central_ubuntu64_vm-debug_test-crashtest-bm122-tests1-linux64-build19.txt | grep WARNING | sort | uniq -c | sort -bnr | head -n 40
And instances per test:

49354 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/478185-1.html
11601 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/421671.html
2250 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/812893.html
1463 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/767765.html
188 - file:///builds/slave/test/build/tests/reftest/tests/layout/tables/crashtests/420654-1.xhtml
164 - file:///builds/slave/test/build/tests/reftest/tests/layout/base/crashtests/453894-1.xhtml
111 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/765621.html
85 - file:///builds/slave/test/build/tests/reftest/tests/layout/base/crashtests/576649-1.html
66 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/first-letter-638937-2.html
55 - file:///builds/slave/test/build/tests/reftest/tests/widget/cocoa/crashtests/449111-1.html
55 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/323386-1.html
54 - file:///builds/slave/test/build/tests/reftest/tests/layout/base/crashtests/439343.html
32 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/807565-1.html
32 - file:///builds/slave/test/build/tests/reftest/tests/gfx/tests/crashtests/839745-1.html
25 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/673770.html
24 - file:///builds/slave/test/build/tests/reftest/tests/layout/tables/crashtests/410426-1.html
22 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/479938-1.html
20 - file:///builds/slave/test/build/tests/reftest/tests/widget/cocoa/crashtests/435223-1.html
20 - file:///builds/slave/test/build/tests/reftest/tests/layout/base/crashtests/266360-1.html
19 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/545571-1.html
18 - file:///builds/slave/test/build/tests/reftest/tests/layout/base/crashtests/265986-1.html
16 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/459968.html
16 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/398181-2.html
16 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/398181-1.html
16 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/1169420-2.html
16 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/1169420-1.html
10 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/499885-1.xhtml
10 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/451315-1.html
10 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/444861-1.html
8 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/text-overflow-bug670564.xhtml
8 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/807565-2.html
8 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/798020-1.html
8 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/412201-1.xhtml
7 - file:///builds/slave/test/build/tests/reftest/tests/layout/tables/crashtests/450311-1.html
6 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/462968.xhtml
6 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/448903-1.html
6 - file:///builds/slave/test/build/tests/reftest/tests/layout/base/crashtests/541869-2.html
6 - file:///builds/slave/test/build/tests/reftest/tests/layout/base/crashtests/444967-1.html
5 - file:///builds/slave/test/build/tests/reftest/tests/layout/base/crashtests/763223-1.html
4 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/812879.html
4 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/553504-1.xhtml
4 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/508168-1.html
4 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/493118-1.html
4 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/411851-1.html
4 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/1146114.html
4 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/1146107.html
4 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/1015563-2.html
4 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/1015563-1.html
4 - file:///builds/slave/test/build/tests/reftest/tests/layout/base/crashtests/265973-1.html
3 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/541714-1.html
2 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/570289-1.html
2 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/489480-1.xhtml
2 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/265867-1.html
2 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/1146103.html
2 - file:///builds/slave/test/build/tests/reftest/tests/layout/base/crashtests/266445-2.html
2 - file:///builds/slave/test/build/tests/reftest/tests/layout/base/crashtests/266445-1.html
2 - file:///builds/slave/test/build/tests/reftest/tests/layout/base/crashtests/265999-1.html
2 - file:///builds/slave/test/build/tests/reftest/tests/layout/base/crashtests/265899-1.html
(In reply to Eric Rahm [:erahm] from comment #0)
> A NS_WARNING in |SaturatingUnionEdges| is hit over 65,000 times in linux
> debug test logs generating the messsage:
> 
> > [NNNNN] WARNING: Overflowed nscoord_MAX in conversion to nscoord width: file ../../dist/include/nsRect.h, line 83
> 
> Either we're doing something horribly wrong (I somewhat doubt that due to
> the frequency of this message) and it needs to be fixed or we should
> consider removing the warning.

It would appear this occurs on all platforms, not just Linux, and is a result of the crashtests using fairly large values on purpose to try and trigger various bugs. So, in this case, we're doing something horribly wrong, but it is on purpose... So we're down to either living with the warning or just removing it as options.
OS: Unspecified → All
Hardware: Unspecified → All
Assignee: lsalzman → nobody
Disable the extremely verbose warning for overflowed width in nsRect. It can
be reenabled at build time by adding the |--with-debug-label=gfx| flag.
Attachment #8617941 - Flags: review?(dholbert)
Assignee: nobody → erahm
Status: NEW → ASSIGNED
See also https://groups.google.com/d/msg/mozilla.dev.tech.layout/YXauN50HDhI/ybqAuH-ZtaUJ -- can we use MOZ_LOG here instead, so that we can configure this at runtime (or browser startup time) instead of at compile time?
(In reply to Daniel Holbert [:dholbert] from comment #8)
> See also
> https://groups.google.com/d/msg/mozilla.dev.tech.layout/YXauN50HDhI/ybqAuH-
> ZtaUJ -- can we use MOZ_LOG here instead, so that we can configure this at
> runtime (or browser startup time) instead of at compile time?

Sure, I'll need to add a logger though so it will be slightly more complicated.
Adds a gfx specific log module and uses that to warn about overflowed
nscoord_MAX.
Attachment #8620432 - Flags: review?(dholbert)
Attachment #8617941 - Attachment is obsolete: true
Attachment #8617941 - Flags: review?(dholbert)
Minor concern: we already have gfx/2d/Logging.h, and I slightly worry that "gfxLog.h" might be confusing alongside that.

BUT, after doing a bit more research, I think we probably should just drop this warning.

Some history:
 - This warning was added by mats in bug 665597 (and has since been moved around a bit)
 - He likely added it for consistency with our NSCoordSaturatingAdd() etc. functions, defined in nscoord.h.  (Those functions all warned about overflow, at that point in time.)
 - Later on, he removed the nscoord.h versions of these warnings, in bug 943448.
 - SO, I think these nsRect.h versions are kind of orphaned, and don't really have much reason to exist given that we're not warning about the same thing for nscoord overflow in general.

(Sorry for prompting you to do the extra work of spinning up a MOZ_LOG solution! I'd forgotten that we'd dropped the nscoord.h versions in bug 943448, until now.)
(on the other hand: maybe we should bring back the warnings that were removed in bug 943448, as opt-in warnings statements, now using something like GFX_LOG() from this patch. But given that we haven't missed these overflow warnings since bug 943448, I tend to think we're OK with them gone.)
Attachment #8620432 - Attachment is obsolete: true
Attachment #8620432 - Flags: review?(dholbert)
Comment on attachment 8620447 [details] [diff] [review]
Remove overflowed nscoord_MAX warnings from nsRect

Please also remove this line:
> 13 #include "nsDebug.h"                    // for NS_WARNING
...from the top of this file. (since you're removing the last NS_WARNING usages)

Thanks! r=me
Attachment #8620447 - Flags: review?(dholbert) → review+
https://hg.mozilla.org/mozilla-central/rev/b3f90ab360af
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.