gcc warning mainly 64 bit: src/xpcconvert.cpp:1931: warning: comparison is always false due to limited range of data type

RESOLVED FIXED in mozilla9

Status

()

Core
JavaScript Engine
RESOLVED FIXED
9 years ago
6 years ago

People

(Reporter: georgi - hopefully not receiving bugspam, Assigned: Atul Aggarwal)

Tracking

Trunk
mozilla9
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [build_warning])

Attachments

(1 attachment)

+++ 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
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.
Summary: gcc warning (may be incorrect): 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 always false due to limited range of data type
Blocks: 514107
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).
Assignee: nobody → general
Component: Rewriting and Analysis → JavaScript Engine
QA Contact: rewriting-and-analysis → general
Blocks: 458491
No longer depends on: 458491
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?
No longer blocks: 514107
Depends on: 514107
Whiteboard: [build_warning]
(Assignee)

Comment 4

6 years ago
Created attachment 557607 [details] [diff] [review]
Patch v1 to fix warning

Implemented as suggested in c2.
Assignee: general → atulagrwl
Status: NEW → ASSIGNED
Attachment #557607 - Flags: review?(bzbarsky)
Wasn't this warning already fixed in bug 551088?
(Assignee)

Comment 6

6 years ago
I could still see those warnings in my local build. Even changing the code (patch), those disappeared.
Comment on attachment 557607 [details] [diff] [review]
Patch v1 to fix warning

I guess this is ok...
Attachment #557607 - Flags: review?(bzbarsky) → review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
In my queue :-)
Keywords: checkin-needed
Hardware: x86 → All
http://hg.mozilla.org/integration/mozilla-inbound/rev/1a9a2270cc47
Flags: in-testsuite-
Target Milestone: --- → mozilla9
http://hg.mozilla.org/mozilla-central/rev/1a9a2270cc47
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.