Re: [geos-devel] [GEOS] #869: CommonBits::zeroLowerBits undefined behavior

[GEOS]

#869: CommonBits::zeroLowerBits undefined behavior
---+---
 Reporter:  goatbar|   Owner:  geos-devel@…
 Type:  defect |  Status:  closed
 Priority:  minor  |   Milestone:  3.6.4
Component:  Default| Version:  3.6.2
 Severity:  Unassigned |  Resolution:  fixed
 Keywords:  UndefinedBehavior  |
---+---

Comment (by goatbar):

 The test I was using when looking at this issue:

 {{{#!cpp
 // zeroLowerBits.
 template<>
 template<>
 void object::test<4>()
 {
 constexpr int64 val = static_cast(0xull);
 ensure_equals(sizeof(val), 8);

 ensure_equals(CommonBits::zeroLowerBits(val, -1), 0);
 ensure_equals(CommonBits::zeroLowerBits(val, 0), 0);
 ensure_equals(CommonBits::zeroLowerBits(val, 1), -2);
 ensure_equals(CommonBits::zeroLowerBits(val, 2), -4);
 ensure_equals(CommonBits::zeroLowerBits(val, 16), -65536);
 ensure_equals(CommonBits::zeroLowerBits(val, 31), -2147483648ll);
 ensure_equals(CommonBits::zeroLowerBits(val, 32), -4294967296ll);
 ensure_equals(CommonBits::zeroLowerBits(val, 62),
 -4611686018427387904ll);
 ensure_equals(static_cast(CommonBits::zeroLowerBits(val,
 62)), 0xc000ull);
 ensure_equals(static_cast(CommonBits::zeroLowerBits(val,
 63)), 0x8000ull);
 ensure_equals(CommonBits::zeroLowerBits(val, 64), 0);
 ensure_equals(CommonBits::zeroLowerBits(val, 1), 0);
 }
 }}}

-- 
Ticket URL: 
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

Re: [geos-devel] [GEOS] #869: CommonBits::zeroLowerBits undefined behavior

[GEOS]

#869: CommonBits::zeroLowerBits undefined behavior
---+---
 Reporter:  goatbar|   Owner:  geos-devel@…
 Type:  defect |  Status:  closed
 Priority:  minor  |   Milestone:  3.6.4
Component:  Default| Version:  3.6.2
 Severity:  Unassigned |  Resolution:  fixed
 Keywords:  UndefinedBehavior  |
---+---

Comment (by nila):

 @pramsey I noticed the initial commits didn't fall out well on some
 builds.

 I could not reproduce the failure, but perhaps using {{{std::uint64_t}}}
 would make any difference.

-- 
Ticket URL: 
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

Re: [geos-devel] [GEOS] #869: CommonBits::zeroLowerBits undefined behavior

[GEOS]

#869: CommonBits::zeroLowerBits undefined behavior
---+---
 Reporter:  goatbar|   Owner:  geos-devel@…
 Type:  defect |  Status:  closed
 Priority:  minor  |   Milestone:  3.6.4
Component:  Default| Version:  3.6.2
 Severity:  Unassigned |  Resolution:  fixed
 Keywords:  UndefinedBehavior  |
---+---
Changes (by pramsey):

 * status:  reopened => closed
 * resolution:   => fixed


-- 
Ticket URL: 
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

Re: [geos-devel] [GEOS] #869: CommonBits::zeroLowerBits undefined behavior

[GEOS]

#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  |
---+---

Comment (by Paul Ramsey ):

 In [changeset:"3e8be5b6f78961093ab7dbb5f404b20746618845/git" 3e8be5b/git]:
 {{{
 #!CommitTicketReference repository="git"
 revision="3e8be5b6f78961093ab7dbb5f404b20746618845"
 Improve the bit-shifting logic with good types and so on
 Even Roualt, Kurt Schwehr
 References #869
 }}}

-- 
Ticket URL: 
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

Re: [geos-devel] [GEOS] #869: CommonBits::zeroLowerBits undefined behavior

[GEOS]

#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  |
---+---

Comment (by Paul Ramsey ):

 In [changeset:"311b64a08768c3d4f2329ec4647070928058bcd2/git" 311b64a/git]:
 {{{
 #!CommitTicketReference repository="git"
 revision="311b64a08768c3d4f2329ec4647070928058bcd2"
 Improve the bit-shifting logic with good types and so on
 Even Roualt, Kurt Schwehr
 References #869
 }}}

-- 
Ticket URL: 
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

Re: [geos-devel] [GEOS] #869: CommonBits::zeroLowerBits undefined behavior

[GEOS]

#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: 
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

Re: [geos-devel] [GEOS] #869: CommonBits::zeroLowerBits undefined behavior

[GEOS]

#869: CommonBits::zeroLowerBits undefined behavior
---+---
 Reporter:  goatbar|   Owner:  geos-devel@…
 Type:  defect |  Status:  closed
 Priority:  minor  |   Milestone:  3.6.4
Component:  Default| Version:  3.6.2
 Severity:  Unassigned |  Resolution:  fixed
 Keywords:  UndefinedBehavior  |
---+---
Changes (by Paul Ramsey ):

 * status:  new => closed
 * resolution:   => fixed


Comment:

 In [changeset:"b8a51cb050b4af0ec6d0c2a9d3b533bbec17645f/git" b8a51cb/git]:
 {{{
 #!CommitTicketReference repository="git"
 revision="b8a51cb050b4af0ec6d0c2a9d3b533bbec17645f"
 Handle nbits > 63
 3.6 branch
 Closes #869
 }}}

-- 
Ticket URL: 
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

Re: [geos-devel] [GEOS] #869: CommonBits::zeroLowerBits undefined behavior

[GEOS]

#869: CommonBits::zeroLowerBits undefined behavior
---+---
 Reporter:  goatbar|   Owner:  geos-devel@…
 Type:  defect |  Status:  new
 Priority:  minor  |   Milestone:  3.6.4
Component:  Default| Version:  3.6.2
 Severity:  Unassigned |  Resolution:
 Keywords:  UndefinedBehavior  |
---+---

Comment (by Paul Ramsey ):

 In [changeset:"6c5b169568dc1e9556748e56bb83f3875018bccc/git" 6c5b169/git]:
 {{{
 #!CommitTicketReference repository="git"
 revision="6c5b169568dc1e9556748e56bb83f3875018bccc"
 Handle nbits > 37
 References #869
 }}}

-- 
Ticket URL: 
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

Re: [geos-devel] [GEOS] #869: CommonBits::zeroLowerBits undefined behavior

[GEOS]

#869: CommonBits::zeroLowerBits undefined behavior
---+---
 Reporter:  goatbar|   Owner:  geos-devel@…
 Type:  defect |  Status:  new
 Priority:  minor  |   Milestone:  3.6.4
Component:  Default| Version:  3.6.2
 Severity:  Unassigned |  Resolution:
 Keywords:  UndefinedBehavior  |
---+---

Comment (by Paul Ramsey ):

 In [changeset:"2c9bfe8a298e41c29046d286b35e8a69345a1807/git" 2c9bfe8/git]:
 {{{
 #!CommitTicketReference repository="git"
 revision="2c9bfe8a298e41c29046d286b35e8a69345a1807"
 Handle nbits > 63
 References #869
 }}}

-- 
Ticket URL: 
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

Re: [geos-devel] [GEOS] #869: CommonBits::zeroLowerBits undefined behavior

[GEOS]

#869: CommonBits::zeroLowerBits undefined behavior
---+---
 Reporter:  goatbar|   Owner:  geos-devel@…
 Type:  defect |  Status:  new
 Priority:  minor  |   Milestone:  3.6.4
Component:  Default| Version:  3.6.2
 Severity:  Unassigned |  Resolution:
 Keywords:  UndefinedBehavior  |
---+---
Changes (by robe):

 * milestone:  3.6.3 => 3.6.4


-- 
Ticket URL: 
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