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
339.02 KB, application/x-bzip
1.63 KB, patch
|Details | Diff | Splinter Review|
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.
5 years ago
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?
(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
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
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.
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.
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.)
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+
You need to log in before you can comment on or make changes to this bug.