UBSan fix for getBit in precision/CommonBits.cpp

[goatbar]

This is small enough that it doesn't seem to merit a pull request.

Found by and patch by Google engineer Han Shen:

The origin implementation never works for i >= 32, because literal "1" is 32-bit, thus "1<<i" is always 32-bit, "mask"'s higher 32-bit is always 0. Worse, "1 << i" causes undefined exception ("i" ranges from [1, 52]). Fixed by using "1ull" (64-bit) in place of "1".

-fsanitize=shift-exponent

src/precision/CommonBits.cpp

@@ -53 +53 @@
 int
 CommonBits::getBit(int64 bits, int i)
 {
-        int64 mask = (1 << i);
+        int64 mask = (1ull << i);
         return (bits & mask) != 0 ? 1 : 0;
 }

UBSan: ​https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html

[strk]

Any chance you could also provide a testcase showing how the current implementation fails and the patch succeeds ?

[goatbar]

My env currently has geos 3.5.0, but this should be the same with 3.6.1 or head.

It was found with ​https://github.com/paulsmith/gogeos

Build with:

-fsanitize=shift-exponent -fsanitize-trap=shift-exponent

==================== Test output for //third_party/golang/geos/geos:geos_test:
=== RUN   TestCoordSeq
--- PASS: TestCoordSeq (0.00s)
=== RUN   TestGeometryTypeMethod
--- PASS: TestGeometryTypeMethod (0.00s)
=== RUN   TestGeometryTypeString
--- PASS: TestGeometryTypeString (0.00s)
=== RUN   TestGeometryType
--- PASS: TestGeometryType (0.00s)
=== RUN   TestGeometryProject
--- PASS: TestGeometryProject (0.00s)
=== RUN   TestGeometryInterpolate
--- PASS: TestGeometryInterpolate (0.00s)
=== RUN   TestGeometryBuffer
--- PASS: TestGeometryBuffer (0.00s)
=== RUN   TestGeometryBufferWithOpts
--- PASS: TestGeometryBufferWithOpts (0.00s)
=== RUN   TestOffsetCurve
SIGILL: illegal instruction
PC=0xa3388a m=4 sigcode=2
signal arrived during cgo execution

goroutine 40 [syscall, locked to thread]:
runtime.cgocall(0x7b7e00, 0xc42003c660, 0x7f1aefa3b300)
	cgocall.go:131 +0xe2 fp=0xc42003c610 sp=0xc42003c5d0 pc=0x63f2d2
geos/geos._Cfunc_GEOSOffsetCurve_r(0x7f1aefa02420, 0x7f1aefa3b300, 0x3ff0000000000000, 0x100000008, 0x4014000000000000, 0x0)
	geos/_objs/geos_test_testlib/_cgo_gotypes.go:1370 +0x4e fp=0xc42003c660 sp=0xc42003c610 pc=0x73a8ce
geos/geos.cGEOSOffsetCurve.func1(0x7f1aefa02420, 0x7f1aefa3b300, 0x3ff0000000000000, 0x100000008, 0x4014000000000000, 0x7f1aefa92cf0)
	geos/cwrappers.go:259 +0xcb fp=0xc42003c6a0 sp=0xc42003c660 pc=0x746c9b
geos/geos.cGEOSOffsetCurve(0x7f1aefa3b300, 0x3ff0000000000000, 0x100000008, 0x4014000000000000, 0x0)
	geos/cwrappers.go:259 +0xa4 fp=0xc42003c6e0 sp=0xc42003c6a0 pc=0x73d7c4
geos/geos.(*Geometry).OffsetCurve(0xc42011e0d8, 0x3ff0000000000000, 0x8, 0x0, 0x1, 0x4014000000000000, 0x3b, 0xc42003c778, 0x698673)
	geos/geom.go:327 +0x5c fp=0xc42003c720 sp=0xc42003c6e0 pc=0x741f5c
geos/geos.TestOffsetCurve(0xc4201464e0)
	geos/geom_test.go:110 +0xa4 fp=0xc42003c7a8 sp=0xc42003c720 pc=0x72f314

​https://github.com/paulsmith/gogeos/blob/master/geos/geom_test.go#L107

From github:

Error: Failed to load processor go

No macro or processor named 'go' found

With that patch applied, all tests pass.

[goatbar]

​https://git.osgeo.org/gitea/geos/geos/pulls/35

[dbaston]

Resolved in d390b76/git