#869: CommonBits::zeroLowerBits undefined behavior
-------------------------------+---------------------------
Reporter: goatbar | Owner: geos-devel@…
Type: defect | Status: reopened
Priority: minor | Milestone: 3.6.4
Component: Default | Version: 3.6.2
Severity: Unassigned | Resolution:
Keywords: UndefinedBehavior |
-------------------------------+---------------------------
Changes (by rouault):
* status: closed => reopened
* resolution: fixed =>
Comment:
Actually, the fix is incorrect for several reasons:
- (1<< nBits)-1 has only int operands, so will be evaluated on 32 bit
only. Consequently nBits >= 32 will result in incorrect behaviour, unless
you cast the first 1 to int64 : ((int64)1 << nBits) - 1
- but, if let to 32 bits ( (1<< nBits)-1 ), then nBits == 31 is also an
invalid input (-fsanitize=undefined warns about "runtime error: left shift
of 1 by 31 places cannot be represented in type 'int'") even if "sane"
compilers / most CPUs will finally do the right thing. The right fix in
that case to avoid undefined/implementation defined behaviour is to use a
unsigned 1, so (1U << nBits) - 1
- for the same reason, is the mask is intended to be a 64 bit one, so
((int64)1 << nBits) - 1), then nBits == 63 also triggers undefined
behaviour. And thus the right fix would be ((unsigned int64)1 << nBits) -
1
--
Ticket URL: <https://trac.osgeo.org/geos/ticket/869#comment:5>
GEOS <http://trac.osgeo.org/geos>
GEOS (Geometry Engine - Open Source) is a C++ port of the Java Topology Suite
(JTS).
_______________________________________________
geos-devel mailing list
geos-devel@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/geos-devel
