add Clang sanitizer checks in CI

.github/workflows/test-linux.yml

@@ -7,15 +7,70 @@ on:
     branches: [master, stable-*]
 
 jobs:
-  tests:
-    name: Test Compile ${{ matrix.compiler }}
+  gcc-tests:
+    name: Test Compile gcc
     runs-on: ubuntu-latest
     env:
-      CC: ${{ matrix.compiler }}
+      CC: gcc
+
+    steps:
+      - uses: actions/[email protected]
+
+      - name: Install Doxygen
+        run: |
+          sudo apt update
+          sudo apt-get install doxygen graphviz clang-format-9
+
+      - name: Configure build
+        run: cmake -Bbuild -DWARNINGS_AS_ERRORS=ON .
+
+      - name: Formatting check
+        working-directory: build
+        run: |
+          clang-format-9 --version
+          make format
+          git diff --exit-code
+
+      - name: Build
+        working-directory: build
+        run: make
+
+      - name: binding-functions
+        working-directory: build
+        run: |
+          make binding-functions
+          test -s binding-functions
+
+      - name: Tests
+        working-directory: build
+        run: |
+          make test
+          sudo make install
+
+        # Note the packages aren't used to test the examples below
+      - name: Test packaging
+        working-directory: build
+        run: cpack -D CPACK_PACKAGE_CONTACT="Test build in CI"
+
+      - name: Examples
+        run: |
+          mkdir build/examples
+          cd build/examples
+          cmake ../../examples
+          make
+          make test
+
+  clang-tests:
+    name: Test Compile clang ${{ matrix.compile_opt }}
+    runs-on: ubuntu-latest
+    env:
+      CC: clang
 
     strategy:
       matrix:
-        compiler: [clang, gcc]
+        # See Clang docs for more information on the sanitizers:
+        # https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html
+        compile_opt: ["", "-fsanitize=undefined,float-divide-by-zero -fno-sanitize-recover=undefined,float-divide-by-zero", "-fsanitize=memory -fno-sanitize-recover=memory", "-fsanitize=address -fno-sanitize-recover=address"]
 
     steps:
       - uses: actions/[email protected]
@@ -26,7 +81,7 @@ jobs:
           sudo apt-get install doxygen graphviz clang-format-9
 
       - name: Configure build
-        run: cmake -Bbuild -DWARNINGS_AS_ERRORS=ON .
+        run: cmake -Bbuild -DWARNINGS_AS_ERRORS=ON -DCMAKE_C_FLAGS="${{ matrix.compile_opt }}" .
 
       - name: Formatting check
         working-directory: build
@@ -60,7 +115,7 @@ jobs:
         run: |
           mkdir build/examples
           cd build/examples
-          cmake ../../examples
+          cmake -DCMAKE_C_FLAGS="${{ matrix.compile_opt }}" ../../examples
           make
           make test
 

src/apps/testapps/testPolygonToCells.c

@@ -40,19 +40,27 @@ static GeoLoop holeGeoLoop = {.numVerts = 3, .verts = holeVerts};
 static GeoPolygon holeGeoPolygon;
 
 static LatLng emptyVerts[] = {{0.659966917655, -2.1364398519394},
-                              {0.659966917655, -2.1364398519395},
-                              {0.659966917655, -2.1364398519396}};
+                              {0.659966917656, -2.1364398519395},
+                              {0.659966917657, -2.1364398519396}};
 static GeoLoop emptyGeoLoop = {.numVerts = 3, .verts = emptyVerts};
 static GeoPolygon emptyGeoPolygon;
 
 static LatLng invalidVerts[] = {{INFINITY, INFINITY}, {-INFINITY, -INFINITY}};
 static GeoLoop invalidGeoLoop = {.numVerts = 2, .verts = invalidVerts};
 static GeoPolygon invalidGeoPolygon;
 
+static LatLng invalid2Verts[] = {{NAN, NAN}, {-NAN, -NAN}};
+static GeoLoop invalid2GeoLoop = {.numVerts = 2, .verts = invalid2Verts};
+static GeoPolygon invalid2GeoPolygon;
+
 static LatLng pointVerts[] = {{0, 0}};
 static GeoLoop pointGeoLoop = {.numVerts = 1, .verts = pointVerts};
 static GeoPolygon pointGeoPolygon;
 
+static LatLng lineVerts[] = {{0, 0}, {1, 0}};
+static GeoLoop lineGeoLoop = {.numVerts = 2, .verts = lineVerts};
+static GeoPolygon lineGeoPolygon;
+
 /**
  * Return true if the cell crosses the meridian.
  */
@@ -141,9 +149,15 @@ SUITE(polygonToCells) {
     invalidGeoPolygon.geoloop = invalidGeoLoop;
     invalidGeoPolygon.numHoles = 0;
 
+    invalid2GeoPolygon.geoloop = invalid2GeoLoop;
+    invalid2GeoPolygon.numHoles = 0;
+
     pointGeoPolygon.geoloop = pointGeoLoop;
     pointGeoPolygon.numHoles = 0;
 
+    lineGeoPolygon.geoloop = lineGeoLoop;
+    lineGeoPolygon.numHoles = 0;
+
     TEST(maxPolygonToCellsSize) {
         int64_t numHexagons;
         t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(&sfGeoPolygon, 9, 0,
@@ -457,9 +471,13 @@ SUITE(polygonToCells) {
 
     TEST(polygonToCellsInvalid) {
         int64_t numHexagons;
-        t_assert(H3_EXPORT(maxPolygonToCellsSize)(&invalidGeoPolygon, 9, 0,
+        t_assert(
+            H3_EXPORT(maxPolygonToCellsSize)(&invalidGeoPolygon, 9, 0,
+                                             &numHexagons) == E_FAILED,
+            "Cannot determine cell size to invalid geo polygon with Infinity");
+        t_assert(H3_EXPORT(maxPolygonToCellsSize)(&invalid2GeoPolygon, 9, 0,
                                                   &numHexagons) == E_FAILED,
-                 "Cannot determine cell size to invalid geo polygon");
+                 "Cannot determine cell size to invalid geo polygon with NaNs");
 
         // Chosen arbitrarily, polygonToCells should error out before this is an
         // issue.
@@ -474,18 +492,15 @@ SUITE(polygonToCells) {
 
     TEST(polygonToCellsPoint) {
         int64_t numHexagons;
-        t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(&pointGeoPolygon, 9, 0,
-                                                         &numHexagons));
-        // 0 estimation, plus 1 vertex, plus 12 buffer
-        t_assert(numHexagons == 13, "Point has expected 13 hexagons");
+        t_assert(H3_EXPORT(maxPolygonToCellsSize)(&pointGeoPolygon, 9, 0,
+                                                  &numHexagons) == E_FAILED,
+                 "Cannot estimate for single point");
+    }
 
-        H3Index *hexagons = calloc(numHexagons, sizeof(H3Index));
-        t_assertSuccess(
-            H3_EXPORT(polygonToCells)(&pointGeoPolygon, 9, 0, hexagons));
-        for (int i = 0; i < numHexagons; i++) {
-            t_assert(hexagons[i] == H3_NULL,
-                     "no results for polygonToCells of a single point");
-        }
-        free(hexagons);
+    TEST(polygonToCellsLine) {
+        int64_t numHexagons;
+        t_assert(H3_EXPORT(maxPolygonToCellsSize)(&lineGeoPolygon, 9, 0,
+                                                  &numHexagons) == E_FAILED,
+                 "Cannot estimate for straight line");
     }
 }

src/h3lib/lib/bbox.c

@@ -123,7 +123,12 @@ H3Error bboxHexEstimate(const BBox *bbox, int res, int64_t *out) {
     double d = H3_EXPORT(greatCircleDistanceKm)(&p1, &p2);
     // Derived constant based on: https://math.stackexchange.com/a/1921940
     // Clamped to 3 as higher values tend to rapidly drag the estimate to zero.
-    double a = d * d / fmin(3.0, fabs((p1.lng - p2.lng) / (p1.lat - p2.lat)));
+    double lngDiff = p1.lng - p2.lng;
+    double latDiff = p1.lat - p2.lat;
+    if (lngDiff == 0 || latDiff == 0) {
+        return E_FAILED;
+    }
+    double a = d * d / fmin(3.0, fabs(lngDiff / latDiff));
 
     // Divide the two to get an estimate of the number of hexagons needed
     double estimateDouble = ceil(a / pentagonAreaKm2);