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:"9362bf25c5ab5508aa70287a6c557ea5d0596beb/git" 9362bf2/git]:
 {{{
 #!CommitTicketReference repository="git"
 revision="9362bf25c5ab5508aa70287a6c557ea5d0596beb"
 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:"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

[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.3
Component:  Default |Version:  3.6.2
 Severity:  Unassigned  |   Keywords:  UndefinedBehavior
+---
 It is possible to get nBits to be a shift larger than `1 << 63`

 I don't have a cleaned up test case for this issue.  I will try to add one
 to this issue soon.

 This is probably not the right fix...
 {{{#!C++
 /*static public*/
 int64
 CommonBits::zeroLowerBits(int64 bits, int nBits)
 {
 if (nBits >= sizeof(nBits) * 8) return 0;
 int64 invMask = (1<< nBits)-1;
 int64 mask = ~ invMask;
 int64 zeroed = bits & mask;
 return zeroed;
 }
 }}}

 Found with autofuzz.

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