Last Comment Bug 458722 - gcc warning mainly 64 bit: src/xpcconvert.cpp:1931: warning: comparison is always false due to limited range of data type
: gcc warning mainly 64 bit: src/xpcconvert.cpp:1931: warning: comparison is al...
Status: RESOLVED FIXED
[build_warning]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla9
Assigned To: Atul Aggarwal
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on: 514107
Blocks: 458491
  Show dependency treegraph
 
Reported: 2008-10-06 07:38 PDT by georgi - hopefully not receiving bugspam
Modified: 2011-09-05 11:44 PDT (History)
8 users (show)
emorley: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 to fix warning (1.53 KB, patch)
2011-09-01 12:30 PDT, Atul Aggarwal
bzbarsky: review+
Details | Diff | Splinter Review

Description georgi - hopefully not receiving bugspam 2008-10-06 07:38:36 PDT
+++ This bug was initially created as a clone of Bug #458491 +++

not sure if this is sane:

1758:/opt/pub/firefox-central/src/js/src/xpconnect/src/xpcconvert.cpp:1931: warning: comparison is always false due to limited range of data type
1759:/opt/pub/firefox-central/src/js/src/xpconnect/src/xpcconvert.cpp:1932: warning: comparison is always false due to limited range of data type
1760:/opt/pub/firefox-central/src/js/src/xpconnect/src/xpcconvert.cpp:1933: warning: comparison is always false due to limited range of data type
1761:/opt/pub/firefox-central/src/js/src/xpconnect/src/xpcconvert.cpp:1934: warning: comparison is always false due to limited range of data type
1762:/opt/pub/firefox-central/src/js/src/xpconnect/src/xpcconvert.cpp:1935: warning: comparison is always false due to limited range of data type
1763:/opt/pub/firefox-central/src/js/src/xpconnect/src/xpcconvert.cpp:1936: warning: comparison is always false due to limited range of data type
1764:/opt/pub/firefox-central/src/js/src/xpconnect/src/xpcconvert.cpp:1937: warning: comparison is always false due to limited range of data type
1765:/opt/pub/firefox-central/src/js/src/xpconnect/src/xpcconvert.cpp:1938: warning: comparison is always false due to limited range of data type
1766:/opt/pub/firefox-central/src/js/src/xpconnect/src/xpcconvert.cpp:1939: warning: comparison is always false due to limited range of data type
1767:/opt/pub/firefox-central/src/js/src/xpconnect/src/xpcconvert.cpp:1940: warning: comparison is always false due to limited range of data type
1768:/opt/pub/firefox-central/src/js/src/xpconnect/src/xpcconvert.cpp:1941: warning: comparison is always false due to limited range of data type
1769:/opt/pub/firefox-central/src/js/src/xpconnect/src/xpcconvert.cpp:1942: warning: comparison is always false due to limited range of data type
1770:/opt/pub/firefox-central/src/js/src/xpconnect/src/xpcconvert.cpp:1943: warning: comparison is always false due to limited range of data type
1771:/opt/pub/firefox-central/src/js/src/xpconnect/src/xpcconvert.cpp:1945: warning: comparison is always false due to limited range of data type
1772:/opt/pub/firefox-central/src/js/src/xpconnect/src/xpcconvert.cpp:1947: warning: comparison is always false due to limited range of data type
1773:/opt/pub/firefox-central/src/js/src/xpconnect/src/xpcconvert.cpp:1948: warning: comparison is always false due to limited range of data type
1774:/opt/pub/firefox-central/src/js/src/xpconnect/src/xpcconvert.cpp:1949: warning: comparison is always false due to limited range of data type
1775:/opt/pub/firefox-central/src/js/src/xpconnect/src/xpcconvert.cpp:1950: warning: comparison is always false due to limited range of data type
Comment 1 georgi - hopefully not receiving bugspam 2008-10-08 05:39:27 PDT
the uint8 warning is real on 64 bit system (and probably at least in one case
on 32 bit systems).

the problem isn't the switch(), the problem is POPULATE macro:
http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/src/xpcconvert.cpp#1904


1904 #define POPULATE(_mode, _t)                                               
  \
1905     PR_BEGIN_MACRO                                                        
  \
1906         cleanupMode = _mode;                                              
  \
1907         if (capacity > ~(size_t)0 / sizeof(_t) ||


capacity is uint32 and ~(size_t)0 is uint64 and sizeof(_t) can't compensate for
32 bits.

this may be a bug of its own on 64 bit systems.
Comment 2 Karl Tomlinson (:karlt) 2009-09-01 19:13:26 PDT
http://hg.mozilla.org/mozilla-central/annotate/c07eb0b8b0f1/js/src/xpconnect/src/xpcconvert.cpp#l2090

There isn't really a bug here.  The code is doing what it is intended to do.
Perhaps it could be argued that capacity should be size_t, which would reduce
the number of warnings to just the cases where sizeof(_t) is 1, but I don't
think capacity should need to be size_t.

Really, the reason why this warning is showing up is that the macro is
intended to be for generic type _t, but, by the time the compiler sees the
macro expanded into each type, the code looks specific to that type (and
unnecessary).

The warning could be suppressed by using an intermediate variable, as
indicated in bug 514107 comment 1, but perhaps it would be better to consider
whether all the function calls in this macro really need to be expanded 18
times.

It looks like all the switch statement needs to do is set cleanupMode and
sizeof(_t), and then the rest can be done generically (after the switch,
without a MACRO).
Comment 3 Ed Morley [:emorley] 2011-07-08 05:32:43 PDT
Present in m-c tip, but now at http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/src/xpcconvert.cpp#2144 onwards.

Is the solution in comment 2 still the desired one?
Comment 4 Atul Aggarwal 2011-09-01 12:30:28 PDT
Created attachment 557607 [details] [diff] [review]
Patch v1 to fix warning

Implemented as suggested in c2.
Comment 5 Boris Zbarsky [:bz] (still a bit busy) 2011-09-01 12:38:22 PDT
Wasn't this warning already fixed in bug 551088?
Comment 6 Atul Aggarwal 2011-09-01 12:42:35 PDT
I could still see those warnings in my local build. Even changing the code (patch), those disappeared.
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2011-09-01 13:30:34 PDT
Comment on attachment 557607 [details] [diff] [review]
Patch v1 to fix warning

I guess this is ok...
Comment 8 Ed Morley [:emorley] 2011-09-04 17:38:29 PDT
In my queue :-)

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