From df07a40893697856c9aa22967da461f517e27676 Mon Sep 17 00:00:00 2001 From: Fabrice Aneche Date: Wed, 22 Mar 2023 15:37:22 -0400 Subject: [PATCH] applying auto update --- H3_VERSION | 2 +- h3_algos.c | 45 ++++++----- h3_alloc.h | 10 +-- h3_bbox.c | 10 ++- h3_coordijk.c | 152 +++++++++++++++++++++++++++++++++++- h3_coordijk.h | 7 +- h3_directedEdge.c | 3 +- h3_h3Assert.c | 32 ++++++++ h3_h3Assert.h | 125 ++++++++++++++++++++++++++++++ h3_h3Index.c | 185 ++++++++++++++++++++++++++++++++++++++++---- h3_h3api.h | 20 +++++ h3_latLng.c | 6 +- h3_localij.c | 49 ++++++------ h3_mathExtensions.h | 10 ++- h3_vertex.c | 11 ++- 15 files changed, 592 insertions(+), 75 deletions(-) create mode 100644 h3_h3Assert.c create mode 100644 h3_h3Assert.h diff --git a/H3_VERSION b/H3_VERSION index 82f24fd..b913b7c 100644 --- a/H3_VERSION +++ b/H3_VERSION @@ -1 +1 @@ -v4.0.1 +v4.1.0 diff --git a/h3_algos.c b/h3_algos.c index aae6914..90e0e0c 100644 --- a/h3_algos.c +++ b/h3_algos.c @@ -30,6 +30,7 @@ #include "h3_baseCells.h" #include "h3_bbox.h" #include "h3_faceijk.h" +#include "h3_h3Assert.h" #include "h3_h3Index.h" #include "h3_h3api.h" #include "h3_latLng.h" @@ -355,14 +356,16 @@ H3Error h3NeighborRotations(H3Index origin, Direction dir, int *rotations, if (dir < CENTER_DIGIT || dir >= INVALID_DIGIT) { return E_FAILED; } + // Ensure that rotations is modulo'd by 6 before any possible addition, + // to protect against signed integer overflow. + *rotations = *rotations % 6; for (int i = 0; i < *rotations; i++) { dir = _rotate60ccw(dir); } int newRotations = 0; int oldBaseCell = H3_GET_BASE_CELL(current); - if (oldBaseCell < 0 || // LCOV_EXCL_BR_LINE - oldBaseCell >= NUM_BASE_CELLS) { + if (NEVER(oldBaseCell < 0) || oldBaseCell >= NUM_BASE_CELLS) { // Base cells less than zero can not be represented in an index return E_CELL_INVALID; } @@ -428,13 +431,14 @@ H3Error h3NeighborRotations(H3Index origin, Direction dir, int *rotations, // on how we got here. // check for a cw/ccw offset face; default is ccw - if (_baseCellIsCwOffset( - newBaseCell, baseCellData[oldBaseCell].homeFijk.face)) { + if (ALWAYS(_baseCellIsCwOffset( + newBaseCell, + baseCellData[oldBaseCell].homeFijk.face))) { current = _h3Rotate60cw(current); } else { // See cwOffsetPent in testGridDisk.c for why this is // unreachable. - current = _h3Rotate60ccw(current); // LCOV_EXCL_LINE + current = _h3Rotate60ccw(current); } alreadyAdjustedKSubsequence = 1; } else { @@ -457,8 +461,8 @@ H3Error h3NeighborRotations(H3Index origin, Direction dir, int *rotations, current = _h3Rotate60cw(current); *rotations = *rotations + 5; } else { - // Should never occur - return E_FAILED; // LCOV_EXCL_LINE + // TODO: Should never occur, but is reachable by fuzzer + return E_FAILED; } } } @@ -599,10 +603,11 @@ H3Error H3_EXPORT(gridDiskDistancesUnsafe)(H3Index origin, int k, H3Index *out, // the end of this ring. H3Error neighborResult = h3NeighborRotations( origin, NEXT_RING_DIRECTION, &rotations, &origin); - if (neighborResult) { // LCOV_EXCL_BR_LINE + if (neighborResult) { // Should not be possible because `origin` would have to be a // pentagon - return neighborResult; // LCOV_EXCL_LINE + // TODO: Reachable via fuzzer + return neighborResult; } if (H3_EXPORT(isPentagon)(origin)) { @@ -703,10 +708,11 @@ H3Error H3_EXPORT(gridRingUnsafe)(H3Index origin, int k, H3Index *out) { for (int ring = 0; ring < k; ring++) { H3Error neighborResult = h3NeighborRotations( origin, NEXT_RING_DIRECTION, &rotations, &origin); - if (neighborResult) { // LCOV_EXCL_BR_LINE + if (neighborResult) { // Should not be possible because `origin` would have to be a // pentagon - return neighborResult; // LCOV_EXCL_LINE + // TODO: Reachable via fuzzer + return neighborResult; } if (H3_EXPORT(isPentagon)(origin)) { @@ -723,10 +729,11 @@ H3Error H3_EXPORT(gridRingUnsafe)(H3Index origin, int k, H3Index *out) { for (int pos = 0; pos < k; pos++) { H3Error neighborResult = h3NeighborRotations( origin, DIRECTIONS[direction], &rotations, &origin); - if (neighborResult) { // LCOV_EXCL_BR_LINE + if (neighborResult) { // Should not be possible because `origin` would have to be a // pentagon - return neighborResult; // LCOV_EXCL_LINE + // TODO: Reachable via fuzzer + return neighborResult; } // Skip the very last index, it was already added. We do @@ -847,7 +854,8 @@ H3Error _getEdgeHexagons(const GeoLoop *geoloop, int64_t numHexagons, int res, while (found[loc] != 0) { // If this conditional is reached, the `found` memory block is // too small for the given polygon. This should not happen. - if (loopCount > numHexagons) return E_FAILED; // LCOV_EXCL_LINE + // TODO: Reachable via fuzzer + if (loopCount > numHexagons) return E_FAILED; if (found[loc] == pointHex) break; // At least two points of the geoloop index to the // same cell @@ -944,14 +952,13 @@ H3Error H3_EXPORT(polygonToCells)(const GeoPolygon *geoPolygon, int res, &numSearchHexes, search, found); // If this branch is reached, we have exceeded the maximum number of // hexagons possible and need to clean up the allocated memory. - // LCOV_EXCL_START + // TODO: Reachable via fuzzer if (edgeHexError) { H3_MEMORY(free)(search); H3_MEMORY(free)(found); H3_MEMORY(free)(bboxes); return edgeHexError; } - // LCOV_EXCL_STOP // 2. Iterate over all holes, trace the polygons defining the holes with // hexagons and add to only the search hash. We're going to temporarily use @@ -964,14 +971,13 @@ H3Error H3_EXPORT(polygonToCells)(const GeoPolygon *geoPolygon, int res, search, found); // If this branch is reached, we have exceeded the maximum number of // hexagons possible and need to clean up the allocated memory. - // LCOV_EXCL_START + // TODO: Reachable via fuzzer if (edgeHexError) { H3_MEMORY(free)(search); H3_MEMORY(free)(found); H3_MEMORY(free)(bboxes); return edgeHexError; } - // LCOV_EXCL_STOP } // 3. Re-zero the found hash so it can be used in the main loop below @@ -1005,14 +1011,13 @@ H3Error H3_EXPORT(polygonToCells)(const GeoPolygon *geoPolygon, int res, // If this branch is reached, we have exceeded the maximum // number of hexagons possible and need to clean up the // allocated memory. - // LCOV_EXCL_START + // TODO: Reachable via fuzzer if (loopCount > numHexagons) { H3_MEMORY(free)(search); H3_MEMORY(free)(found); H3_MEMORY(free)(bboxes); return E_FAILED; } - // LCOV_EXCL_STOP if (out[loc] == hex) break; // Skip duplicates found loc = (loc + 1) % numHexagons; loopCount++; diff --git a/h3_alloc.h b/h3_alloc.h index ac0f24a..5a25b4d 100644 --- a/h3_alloc.h +++ b/h3_alloc.h @@ -24,7 +24,7 @@ #ifndef ALLOC_H #define ALLOC_H -#include "h3_h3api.h" // for TJOIN +#include "h3_h3api.h" // for TJOIN #ifdef H3_ALLOC_PREFIX #define H3_MEMORY(name) TJOIN(H3_ALLOC_PREFIX, name) @@ -33,10 +33,10 @@ extern "C" { #endif -void* H3_MEMORY(malloc)(size_t size); -void* H3_MEMORY(calloc)(size_t num, size_t size); -void* H3_MEMORY(realloc)(void* ptr, size_t size); -void H3_MEMORY(free)(void* ptr); +void *H3_MEMORY(malloc)(size_t size); +void *H3_MEMORY(calloc)(size_t num, size_t size); +void *H3_MEMORY(realloc)(void *ptr, size_t size); +void H3_MEMORY(free)(void *ptr); #ifdef __cplusplus } diff --git a/h3_bbox.c b/h3_bbox.c index 60ea01c..9dacd30 100644 --- a/h3_bbox.c +++ b/h3_bbox.c @@ -121,9 +121,17 @@ H3Error bboxHexEstimate(const BBox *bbox, int res, int64_t *out) { p2.lat = bbox->south; p2.lng = bbox->west; double d = H3_EXPORT(greatCircleDistanceKm)(&p1, &p2); + double lngDiff = fabs(p1.lng - p2.lng); + double latDiff = fabs(p1.lat - p2.lat); + if (lngDiff == 0 || latDiff == 0) { + return E_FAILED; + } + double length = fmax(lngDiff, latDiff); + double width = fmin(lngDiff, latDiff); + double ratio = length / width; // 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 a = d * d / fmin(3.0, ratio); // Divide the two to get an estimate of the number of hexagons needed double estimateDouble = ceil(a / pentagonAreaKm2); diff --git a/h3_coordijk.c b/h3_coordijk.c index 77e9640..e6e597b 100644 --- a/h3_coordijk.c +++ b/h3_coordijk.c @@ -1,5 +1,5 @@ /* - * Copyright 2016-2018, 2020-2021 Uber Technologies, Inc. + * Copyright 2016-2018, 2020-2022 Uber Technologies, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,9 +26,12 @@ #include #include "h3_constants.h" +#include "h3_h3Assert.h" #include "h3_latLng.h" #include "h3_mathExtensions.h" +#define INT32_MAX_3 (INT32_MAX / 3) + /** * Sets an IJK coordinate to the specified component values. * @@ -207,10 +210,50 @@ void _ijkScale(CoordIJK *c, int factor) { c->k *= factor; } +/** + * Returns true if _ijkNormalize with the given input could have a signed + * integer overflow. Assumes k is set to 0. + */ +bool _ijkNormalizeCouldOverflow(const CoordIJK *ijk) { + // Check for the possibility of overflow + int max, min; + if (ijk->i > ijk->j) { + max = ijk->i; + min = ijk->j; + } else { + max = ijk->j; + min = ijk->i; + } + if (min < 0) { + // Only if the min is less than 0 will the resulting number be larger + // than max. If min is positive, then max is also positive, and a + // positive signed integer minus another positive signed integer will + // not overflow. + if (ADD_INT32S_OVERFLOWS(max, min)) { + // max + min would overflow + return true; + } + if (SUB_INT32S_OVERFLOWS(0, min)) { + // 0 - INT32_MIN would overflow + return true; + } + if (SUB_INT32S_OVERFLOWS(max, min)) { + // max - min would overflow + return true; + } + } + return false; +} + /** * Normalizes ijk coordinates by setting the components to the smallest possible * values. Works in place. * + * This function does not protect against signed integer overflow. The caller + * must ensure that none of (i - j), (i - k), (j - i), (j - k), (k - i), (k - j) + * will overflow. This function may be changed in the future to make that check + * itself and return an error code. + * * @param c The ijk coordinates to normalize. */ void _ijkNormalize(CoordIJK *c) { @@ -267,6 +310,104 @@ Direction _unitIjkToDigit(const CoordIJK *ijk) { return digit; } +/** + * Returns non-zero if _upAp7 with the given input could have a signed integer + * overflow. + * + * Assumes ijk is IJK+ coordinates (no negative numbers). + */ +H3Error _upAp7Checked(CoordIJK *ijk) { + // Doesn't need to be checked because i, j, and k must all be non-negative + int i = ijk->i - ijk->k; + int j = ijk->j - ijk->k; + + // <0 is checked because the input must all be non-negative, but some + // negative inputs are used in unit tests to exercise the below. + if (i >= INT32_MAX_3 || j >= INT32_MAX_3 || i < 0 || j < 0) { + if (ADD_INT32S_OVERFLOWS(i, i)) { + return E_FAILED; + } + int i2 = i + i; + if (ADD_INT32S_OVERFLOWS(i2, i)) { + return E_FAILED; + } + int i3 = i2 + i; + if (ADD_INT32S_OVERFLOWS(j, j)) { + return E_FAILED; + } + int j2 = j + j; + + if (SUB_INT32S_OVERFLOWS(i3, j)) { + return E_FAILED; + } + if (ADD_INT32S_OVERFLOWS(i, j2)) { + return E_FAILED; + } + } + + // TODO: Do the int math parts here in long double? + ijk->i = (int)lroundl(((i * 3) - j) / 7.0L); + ijk->j = (int)lroundl((i + (j * 2)) / 7.0L); + ijk->k = 0; + + // Expected not to be reachable, because max + min or max - min would need + // to overflow. + if (NEVER(_ijkNormalizeCouldOverflow(ijk))) { + return E_FAILED; + } + _ijkNormalize(ijk); + return E_SUCCESS; +} + +/** + * Returns non-zero if _upAp7r with the given input could have a signed integer + * overflow. + * + * Assumes ijk is IJK+ coordinates (no negative numbers). + */ +H3Error _upAp7rChecked(CoordIJK *ijk) { + // Doesn't need to be checked because i, j, and k must all be non-negative + int i = ijk->i - ijk->k; + int j = ijk->j - ijk->k; + + // <0 is checked because the input must all be non-negative, but some + // negative inputs are used in unit tests to exercise the below. + if (i >= INT32_MAX_3 || j >= INT32_MAX_3 || i < 0 || j < 0) { + if (ADD_INT32S_OVERFLOWS(i, i)) { + return E_FAILED; + } + int i2 = i + i; + if (ADD_INT32S_OVERFLOWS(j, j)) { + return E_FAILED; + } + int j2 = j + j; + if (ADD_INT32S_OVERFLOWS(j2, j)) { + return E_FAILED; + } + int j3 = j2 + j; + + if (ADD_INT32S_OVERFLOWS(i2, j)) { + return E_FAILED; + } + if (SUB_INT32S_OVERFLOWS(j3, i)) { + return E_FAILED; + } + } + + // TODO: Do the int math parts here in long double? + ijk->i = (int)lroundl(((i * 2) + j) / 7.0L); + ijk->j = (int)lroundl(((j * 3) - i) / 7.0L); + ijk->k = 0; + + // Expected not to be reachable, because max + min or max - min would need + // to overflow. + if (NEVER(_ijkNormalizeCouldOverflow(ijk))) { + return E_FAILED; + } + _ijkNormalize(ijk); + return E_SUCCESS; +} + /** * Find the normalized ijk coordinates of the indexing parent of a cell in a * counter-clockwise aperture 7 grid. Works in place. @@ -527,13 +668,20 @@ void ijkToIj(const CoordIJK *ijk, CoordIJ *ij) { * * @param ij The input IJ coordinates * @param ijk The output IJK+ coordinates + * @returns E_SUCCESS on success, E_FAILED if signed integer overflow would have + * occurred. */ -void ijToIjk(const CoordIJ *ij, CoordIJK *ijk) { +H3Error ijToIjk(const CoordIJ *ij, CoordIJK *ijk) { ijk->i = ij->i; ijk->j = ij->j; ijk->k = 0; + if (_ijkNormalizeCouldOverflow(ijk)) { + return E_FAILED; + } + _ijkNormalize(ijk); + return E_SUCCESS; } /** diff --git a/h3_coordijk.h b/h3_coordijk.h index 38fd8a2..65f3206 100644 --- a/h3_coordijk.h +++ b/h3_coordijk.h @@ -1,5 +1,5 @@ /* - * Copyright 2016-2018, 2020-2021 Uber Technologies, Inc. + * Copyright 2016-2018, 2020-2022 Uber Technologies, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -93,8 +93,11 @@ int _ijkMatches(const CoordIJK *c1, const CoordIJK *c2); void _ijkAdd(const CoordIJK *h1, const CoordIJK *h2, CoordIJK *sum); void _ijkSub(const CoordIJK *h1, const CoordIJK *h2, CoordIJK *diff); void _ijkScale(CoordIJK *c, int factor); +bool _ijkNormalizeCouldOverflow(const CoordIJK *ijk); void _ijkNormalize(CoordIJK *c); Direction _unitIjkToDigit(const CoordIJK *ijk); +H3Error _upAp7Checked(CoordIJK *ijk); +H3Error _upAp7rChecked(CoordIJK *ijk); void _upAp7(CoordIJK *ijk); void _upAp7r(CoordIJK *ijk); void _downAp7(CoordIJK *ijk); @@ -108,7 +111,7 @@ Direction _rotate60ccw(Direction digit); Direction _rotate60cw(Direction digit); int ijkDistance(const CoordIJK *a, const CoordIJK *b); void ijkToIj(const CoordIJK *ijk, CoordIJ *ij); -void ijToIjk(const CoordIJ *ij, CoordIJK *ijk); +H3Error ijToIjk(const CoordIJ *ij, CoordIJK *ijk); void ijkToCube(CoordIJK *ijk); void cubeToIjk(CoordIJK *ijk); diff --git a/h3_directedEdge.c b/h3_directedEdge.c index 2acff27..b1b85a3 100644 --- a/h3_directedEdge.c +++ b/h3_directedEdge.c @@ -23,6 +23,7 @@ #include "h3_algos.h" #include "h3_constants.h" #include "h3_coordijk.h" +#include "h3_h3Assert.h" #include "h3_h3Index.h" #include "h3_latLng.h" #include "h3_vertex.h" @@ -278,7 +279,7 @@ H3Error H3_EXPORT(directedEdgeToBoundary)(H3Index edge, CellBoundary *cb) { // crosses an edge of the icosahedron. FaceIJK fijk; H3Error fijkResult = _h3ToFaceIjk(origin, &fijk); - if (fijkResult) { + if (NEVER(fijkResult)) { return fijkResult; } int res = H3_GET_RESOLUTION(origin); diff --git a/h3_h3Assert.c b/h3_h3Assert.c new file mode 100644 index 0000000..11bdb8b --- /dev/null +++ b/h3_h3Assert.c @@ -0,0 +1,32 @@ +/* + * Copyright 2022 Uber Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +/** @file h3Assert.c + * @brief Support code for unit testing + */ + +#include "h3_h3Assert.h" + +#if defined(H3_COVERAGE_TEST) || defined(H3_DEBUG) +/* +** Counter used for coverage testing. Does not come into play for +** release builds. +** +** Access to this global variable is not mutex protected. This might +** result in TSAN warnings. But as the variable does not exist in +** release builds, that should not be a concern. +*/ +unsigned int h3CoverageCounter; +#endif diff --git a/h3_h3Assert.h b/h3_h3Assert.h new file mode 100644 index 0000000..125cc5a --- /dev/null +++ b/h3_h3Assert.h @@ -0,0 +1,125 @@ +/* + * Copyright 2022 Uber Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +/** @file h3Assert.h + * @brief Support code for unit testing and assertions + * + * This file defines macros needed for defensive programming in the H3 core + * library. H3 strives to have complete code and branch coverage, but this is + * not feasible if some branches cannot be reached because they are defensive - + * that is, we do not know of a test case that would exercise the branch but we + * do have an opinion of how to recover from such an error. These defensive + * branches are excluded from coverage. + * + * In other testing, such as unit tests or fuzzer testing, they trigger + * assertions if the conditions fail. + * + * Adapted from https://www.sqlite.org/testing.html and + * https://www.sqlite.org/assert.html + * + * Used under the terms of the SQLite3 project, reproduced below: + * The author disclaims copyright to this source code. In place of + * a legal notice, here is a blessing: + * + * May you do good and not evil. + * May you find forgiveness for yourself and forgive others. + * May you share freely, never taking more than you give. + */ + +#ifndef H3ASSERT_H +#define H3ASSERT_H + +#include + +/* +** The testcase() macro is used to aid in coverage testing. When +** doing coverage testing, the condition inside the argument to +** testcase() must be evaluated both true and false in order to +** get full branch coverage. The testcase() macro is inserted +** to help ensure adequate test coverage in places where simple +** condition/decision coverage is inadequate. For example, testcase() +** can be used to make sure boundary values are tested. For +** bitmask tests, testcase() can be used to make sure each bit +** is significant and used at least once. On switch statements +** where multiple cases go to the same block of code, testcase() +** can insure that all cases are evaluated. +*/ +#if defined(H3_COVERAGE_TEST) || defined(H3_DEBUG) +extern unsigned int h3CoverageCounter; +#define testcase(X) \ + if (X) { \ + h3CoverageCounter += (unsigned)__LINE__; \ + } +#else +#define testcase(X) +#endif + +/* +** Disable ALWAYS() and NEVER() (make them pass-throughs) for coverage +** and mutation testing +*/ +#if defined(H3_COVERAGE_TEST) +#define H3_OMIT_AUXILIARY_SAFETY_CHECKS 1 +#endif + +/* +** The TESTONLY macro is used to enclose variable declarations or +** other bits of code that are needed to support the arguments +** within testcase() and assert() macros. +*/ +#if !defined(NDEBUG) || defined(H3_COVERAGE_TEST) +#define TESTONLY(X) X +#else +#define TESTONLY(X) +#endif + +/* +** The DEFENSEONLY macro is used to enclose variable declarations or +** other bits of code that are needed to support the arguments +** within ALWAYS() or NEVER() macros. +*/ +#if !defined(H3_OMIT_AUXILIARY_SAFETY_CHECKS) +#define DEFENSEONLY(X) X +#else +#define DEFENSEONLY(X) +#endif + +/* +** The ALWAYS and NEVER macros surround boolean expressions which +** are intended to always be true or false, respectively. Such +** expressions could be omitted from the code completely. But they +** are included in a few cases in order to enhance the resilience +** of the H3 library to unexpected behavior - to make the code "self-healing" +** or "ductile" rather than being "brittle" and crashing at the first +** hint of unplanned behavior. +** +** In other words, ALWAYS and NEVER are added for defensive code. +** +** When doing coverage testing ALWAYS and NEVER are hard-coded to +** be true and false so that the unreachable code they specify will +** not be counted as untested code. +*/ +#if defined(H3_OMIT_AUXILIARY_SAFETY_CHECKS) +#define ALWAYS(X) (1) +#define NEVER(X) (0) +#elif !defined(NDEBUG) +#define ALWAYS(X) ((X) ? 1 : (assert(0), 0)) +#define NEVER(X) ((X) ? (assert(0), 1) : 0) +#else +#define ALWAYS(X) (X) +#define NEVER(X) (X) +#endif + +#endif diff --git a/h3_h3Index.c b/h3_h3Index.c index 97b1a63..db1eee4 100644 --- a/h3_h3Index.c +++ b/h3_h3Index.c @@ -27,6 +27,7 @@ #include "h3_alloc.h" #include "h3_baseCells.h" #include "h3_faceijk.h" +#include "h3_h3Assert.h" #include "h3_iterators.h" #include "h3_mathExtensions.h" @@ -95,15 +96,15 @@ int H3_EXPORT(isValidCell)(H3Index h) { if (H3_GET_RESERVED_BITS(h) != 0) return 0; int baseCell = H3_GET_BASE_CELL(h); - if (baseCell < 0 || baseCell >= NUM_BASE_CELLS) { // LCOV_EXCL_BR_LINE + if (NEVER(baseCell < 0) || baseCell >= NUM_BASE_CELLS) { // Base cells less than zero can not be represented in an index return 0; } int res = H3_GET_RESOLUTION(h); - if (res < 0 || res > MAX_H3_RES) { // LCOV_EXCL_BR_LINE + if (NEVER(res < 0 || res > MAX_H3_RES)) { // Resolutions less than zero can not be represented in an index - return 0; // LCOV_EXCL_LINE + return 0; } bool foundFirstNonZeroDigit = false; @@ -117,7 +118,7 @@ int H3_EXPORT(isValidCell)(H3Index h) { } } - if (digit < CENTER_DIGIT || digit >= NUM_DIGITS) return 0; + if (NEVER(digit < CENTER_DIGIT) || digit >= NUM_DIGITS) return 0; } for (int r = res + 1; r <= MAX_H3_RES; r++) { @@ -331,6 +332,7 @@ H3Error H3_EXPORT(compactCells)(const H3Index *h3Set, H3Index *compactedSet, // to track how many times a parent is duplicated for (int i = 0; i < numRemainingHexes; i++) { H3Index currIndex = remainingHexes[i]; + // TODO: This case is coverable (reachable by fuzzer) if (currIndex != 0) { // If the reserved bits were set by the caller, the // algorithm below may encounter undefined behavior @@ -357,16 +359,13 @@ H3Error H3_EXPORT(compactCells)(const H3Index *h3Set, H3Index *compactedSet, int loc = (int)(parent % numRemainingHexes); int loopCount = 0; while (hashSetArray[loc] != 0) { - if (loopCount > // LCOV_EXCL_BR_LINE - numRemainingHexes) { - // LCOV_EXCL_START + if (NEVER(loopCount > numRemainingHexes)) { // This case should not be possible because at // most one index is placed into hashSetArray // per numRemainingHexes. H3_MEMORY(free)(remainingHexes); H3_MEMORY(free)(hashSetArray); return E_FAILED; - // LCOV_EXCL_STOP } H3Index tempIndex = hashSetArray[loc] & H3_RESERVED_MASK_NEGATIVE; @@ -442,6 +441,7 @@ H3Error H3_EXPORT(compactCells)(const H3Index *h3Set, H3Index *compactedSet, int uncompactableCount = 0; for (int i = 0; i < numRemainingHexes; i++) { H3Index currIndex = remainingHexes[i]; + // TODO: This case is coverable (reachable by fuzzer) if (currIndex != H3_NULL) { H3Index parent; H3Error parentError = @@ -459,15 +459,13 @@ H3Error H3_EXPORT(compactCells)(const H3Index *h3Set, H3Index *compactedSet, int loopCount = 0; bool isUncompactable = true; do { - if (loopCount > numRemainingHexes) { // LCOV_EXCL_BR_LINE - // LCOV_EXCL_START + if (NEVER(loopCount > numRemainingHexes)) { // This case should not be possible because at most one // index is placed into hashSetArray per input hexagon. H3_MEMORY(free)(compactableHexes); H3_MEMORY(free)(remainingHexes); H3_MEMORY(free)(hashSetArray); return E_FAILED; - // LCOV_EXCL_STOP } H3Index tempIndex = hashSetArray[loc] & H3_RESERVED_MASK_NEGATIVE; @@ -785,7 +783,7 @@ H3Error H3_EXPORT(latLngToCell)(const LatLng *g, int res, H3Index *out) { FaceIJK fijk; _geoToFaceIjk(g, res, &fijk); *out = _faceIjkToH3(&fijk, res); - if (*out) { + if (ALWAYS(*out)) { return E_SUCCESS; } else { return E_FAILED; @@ -832,7 +830,7 @@ int _h3ToFaceIjkWithInitializedFijk(H3Index h, FaceIJK *fijk) { */ H3Error _h3ToFaceIjk(H3Index h, FaceIJK *fijk) { int baseCell = H3_GET_BASE_CELL(h); - if (baseCell < 0 || baseCell >= NUM_BASE_CELLS) { // LCOV_EXCL_BR_LINE + if (NEVER(baseCell < 0) || baseCell >= NUM_BASE_CELLS) { // Base cells less than zero can not be represented in an index // To prevent reading uninitialized memory, we zero the output. fijk->face = 0; @@ -979,7 +977,7 @@ H3Error H3_EXPORT(getIcosahedronFaces)(H3Index h3, int *out) { // so fill with invalid values to indicate unused slots int faceCount; H3Error maxFaceCountError = H3_EXPORT(maxFaceCount)(h3, &faceCount); - if (maxFaceCountError != E_SUCCESS) { + if (NEVER(maxFaceCountError != E_SUCCESS)) { return maxFaceCountError; } for (int i = 0; i < faceCount; i++) { @@ -1052,3 +1050,162 @@ H3Error H3_EXPORT(getPentagons)(int res, H3Index *out) { * a Class II grid. */ int isResolutionClassIII(int res) { return res % 2; } + +/** + * Validate a child position in the context of a given parent, returning + * an error if validation fails. + */ +static H3Error validateChildPos(int64_t childPos, H3Index parent, + int childRes) { + int64_t maxChildCount; + H3Error sizeError = + H3_EXPORT(cellToChildrenSize)(parent, childRes, &maxChildCount); + if (NEVER(sizeError)) { + return sizeError; + } + if (childPos < 0 || childPos >= maxChildCount) { + return E_DOMAIN; + } + return E_SUCCESS; +} + +/** + * Returns the position of the cell within an ordered list of all children of + * the cell's parent at the specified resolution + */ +H3Error H3_EXPORT(cellToChildPos)(H3Index child, int parentRes, int64_t *out) { + int childRes = H3_GET_RESOLUTION(child); + // Get the parent at res. This will catch any resolution errors + H3Index originalParent; + H3Error parentError = + H3_EXPORT(cellToParent(child, parentRes, &originalParent)); + if (parentError) { + return parentError; + } + + // Define the initial parent. Note that these variables are reassigned + // within the loop. + H3Index parent = originalParent; + int parentIsPentagon = H3_EXPORT(isPentagon)(parent); + + // Walk up the resolution digits, incrementing the index + *out = 0; + if (parentIsPentagon) { + // Pentagon logic. Pentagon parents skip the 1 digit, so the offsets are + // different from hexagons + for (int res = childRes; res > parentRes; res--) { + H3Error parentError = + H3_EXPORT(cellToParent(child, res - 1, &parent)); + if (NEVER(parentError)) { + return parentError; + } + + parentIsPentagon = H3_EXPORT(isPentagon)(parent); + int rawDigit = H3_GET_INDEX_DIGIT(child, res); + // Validate the digit before proceeding + if (rawDigit == INVALID_DIGIT || + (parentIsPentagon && rawDigit == K_AXES_DIGIT)) { + return E_CELL_INVALID; + } + int digit = + parentIsPentagon && rawDigit > 0 ? rawDigit - 1 : rawDigit; + if (digit != CENTER_DIGIT) { + int64_t hexChildCount = _ipow(7, childRes - res); + // The offset for the 0-digit slot depends on whether the + // current index is the child of a pentagon. If so, the offset + // is based on the count of pentagon children, otherwise, + // hexagon children. + *out += (parentIsPentagon + ? // pentagon children. See the explanation + // for getNumCells in h3api.h.in + 1 + (5 * (hexChildCount - 1)) / 6 + : // one hexagon's children + hexChildCount) + + // the other hexagon children + (digit - 1) * hexChildCount; + } + } + } else { + // Hexagon logic. Offsets are simple powers of 7 + for (int res = childRes; res > parentRes; res--) { + int digit = H3_GET_INDEX_DIGIT(child, res); + if (digit == INVALID_DIGIT) { + return E_CELL_INVALID; + } + *out += digit * _ipow(7, childRes - res); + } + } + + if (NEVER(validateChildPos(*out, originalParent, childRes))) { + // This is the result of an internal error, so return E_FAILED + // instead of the validation error + return E_FAILED; + } + + return E_SUCCESS; +} + +/** + * Returns the child cell at a given position within an ordered list of all + * children at the specified resolution */ +H3Error H3_EXPORT(childPosToCell)(int64_t childPos, H3Index parent, + int childRes, H3Index *child) { + // Validate resolution + if (childRes < 0 || childRes > MAX_H3_RES) { + return E_RES_DOMAIN; + } + // Validate parent resolution + int parentRes = H3_GET_RESOLUTION(parent); + if (childRes < parentRes) { + return E_RES_MISMATCH; + } + // Validate child pos + H3Error childPosErr = validateChildPos(childPos, parent, childRes); + if (childPosErr) { + return childPosErr; + } + + int resOffset = childRes - parentRes; + + *child = parent; + int64_t idx = childPos; + + H3_SET_RESOLUTION(*child, childRes); + + if (H3_EXPORT(isPentagon)(parent)) { + // Pentagon tile logic. Pentagon tiles skip the 1 digit, so the offsets + // are different + bool inPent = true; + for (int res = 1; res <= resOffset; res++) { + int64_t resWidth = _ipow(7, resOffset - res); + if (inPent) { + // While we are inside a parent pentagon, we need to check if + // this cell is a pentagon, and if not, we need to offset its + // digit to account for the skipped direction + int64_t pentWidth = 1 + (5 * (resWidth - 1)) / 6; + if (idx < pentWidth) { + H3_SET_INDEX_DIGIT(*child, parentRes + res, 0); + } else { + idx -= pentWidth; + inPent = false; + H3_SET_INDEX_DIGIT(*child, parentRes + res, + (idx / resWidth) + 2); + idx %= resWidth; + } + } else { + // We're no longer inside a pentagon, continue as for hex + H3_SET_INDEX_DIGIT(*child, parentRes + res, idx / resWidth); + idx %= resWidth; + } + } + } else { + // Hexagon tile logic. Offsets are simple powers of 7 + for (int res = 1; res <= resOffset; res++) { + int64_t resWidth = _ipow(7, resOffset - res); + H3_SET_INDEX_DIGIT(*child, parentRes + res, idx / resWidth); + idx %= resWidth; + } + } + + return E_SUCCESS; +} diff --git a/h3_h3api.h b/h3_h3api.h index 3f533d5..6d9be68 100644 --- a/h3_h3api.h +++ b/h3_h3api.h @@ -534,6 +534,26 @@ DECLSPEC H3Error H3_EXPORT(cellToCenterChild)(H3Index h, int childRes, H3Index *child); /** @} */ +/** @defgroup cellToChildPos cellToChildPos + * Functions for cellToChildPos + * @{ + */ +/** @brief Returns the position of the cell within an ordered list of all + * children of the cell's parent at the specified resolution */ +DECLSPEC H3Error H3_EXPORT(cellToChildPos)(H3Index child, int parentRes, + int64_t *out); +/** @} */ + +/** @defgroup childPosToCell childPosToCell + * Functions for childPosToCell + * @{ + */ +/** @brief Returns the child cell at a given position within an ordered list of + * all children at the specified resolution */ +DECLSPEC H3Error H3_EXPORT(childPosToCell)(int64_t childPos, H3Index parent, + int childRes, H3Index *child); +/** @} */ + /** @defgroup compactCells compactCells * Functions for compactCells * @{ diff --git a/h3_latLng.c b/h3_latLng.c index c90982c..f94071b 100644 --- a/h3_latLng.c +++ b/h3_latLng.c @@ -23,6 +23,7 @@ #include #include "h3_constants.h" +#include "h3_h3Assert.h" #include "h3_h3api.h" #include "h3_mathExtensions.h" @@ -382,9 +383,8 @@ H3Error H3_EXPORT(cellAreaRads2)(H3Index cell, double *out) { return err; } err = H3_EXPORT(cellToBoundary)(cell, &cb); - if (err) { - // TODO: Uncoverable because cellToLatLng will have returned an error - // already + if (NEVER(err)) { + // Uncoverable because cellToLatLng will have returned an error already return err; } diff --git a/h3_localij.c b/h3_localij.c index ac9dbc3..b8eda2a 100644 --- a/h3_localij.c +++ b/h3_localij.c @@ -27,6 +27,7 @@ #include "h3_baseCells.h" #include "h3_faceijk.h" +#include "h3_h3Assert.h" #include "h3_h3Index.h" #include "h3_mathExtensions.h" @@ -138,12 +139,11 @@ H3Error cellToLocalIjk(H3Index origin, H3Index h3, CoordIJK *out) { int originBaseCell = H3_GET_BASE_CELL(origin); int baseCell = H3_GET_BASE_CELL(h3); - if (originBaseCell < 0 || // LCOV_EXCL_BR_LINE - originBaseCell >= NUM_BASE_CELLS) { + if (NEVER(originBaseCell < 0) || originBaseCell >= NUM_BASE_CELLS) { // Base cells less than zero can not be represented in an index return E_CELL_INVALID; } - if (baseCell < 0 || baseCell >= NUM_BASE_CELLS) { // LCOV_EXCL_BR_LINE + if (NEVER(baseCell < 0) || baseCell >= NUM_BASE_CELLS) { // Base cells less than zero can not be represented in an index return E_CELL_INVALID; } @@ -302,8 +302,7 @@ H3Error cellToLocalIjk(H3Index origin, H3Index h3, CoordIJK *out) { H3Error localIjkToCell(H3Index origin, const CoordIJK *ijk, H3Index *out) { int res = H3_GET_RESOLUTION(origin); int originBaseCell = H3_GET_BASE_CELL(origin); - if (originBaseCell < 0 || // LCOV_EXCL_BR_LINE - originBaseCell >= NUM_BASE_CELLS) { + if (NEVER(originBaseCell < 0) || originBaseCell >= NUM_BASE_CELLS) { // Base cells less than zero can not be represented in an index return E_CELL_INVALID; } @@ -345,12 +344,18 @@ H3Error localIjkToCell(H3Index origin, const CoordIJK *ijk, H3Index *out) { CoordIJK lastCenter; if (isResolutionClassIII(r + 1)) { // rotate ccw - _upAp7(&ijkCopy); + H3Error upAp7Error = _upAp7Checked(&ijkCopy); + if (upAp7Error) { + return upAp7Error; + } lastCenter = ijkCopy; _downAp7(&lastCenter); } else { // rotate cw - _upAp7r(&ijkCopy); + H3Error upAp7rError = _upAp7rChecked(&ijkCopy); + if (upAp7rError) { + return upAp7rError; + } lastCenter = ijkCopy; _downAp7r(&lastCenter); } @@ -432,11 +437,9 @@ H3Error localIjkToCell(H3Index origin, const CoordIJK *ijk, H3Index *out) { const Direction indexLeadingDigit = _h3LeadingNonZeroDigit(*out); // This case should be unreachable because this function is building // *out, and should never generate an invalid digit, above. - // LCOV_EXCL_START - if (indexLeadingDigit == INVALID_DIGIT) { + if (NEVER(indexLeadingDigit == INVALID_DIGIT)) { return E_CELL_INVALID; } - // LCOV_EXCL_STOP if (_isBaseCellPolarPentagon(baseCell)) { pentagonRotations = PENTAGON_ROTATIONS_REVERSE_POLAR[revDir][indexLeadingDigit]; @@ -447,11 +450,10 @@ H3Error localIjkToCell(H3Index origin, const CoordIJK *ijk, H3Index *out) { } // For this to occur, revDir would need to be 1. Since revDir is // from the index base cell (which is a pentagon) towards the - // origin, this should never be the case. LCOV_EXCL_START - if (pentagonRotations < 0) { + // origin, this should never be the case. + if (NEVER(pentagonRotations < 0)) { return E_CELL_INVALID; } - // LCOV_EXCL_STOP for (int i = 0; i < pentagonRotations; i++) { *out = _h3RotatePent60ccw(*out); @@ -563,7 +565,10 @@ H3Error H3_EXPORT(localIjToCell)(H3Index origin, const CoordIJ *ij, return E_OPTION_INVALID; } CoordIJK ijk; - ijToIjk(ij, &ijk); + H3Error ijToIjkError = ijToIjk(ij, &ijk); + if (ijToIjkError) { + return ijToIjkError; + } return localIjkToCell(origin, &ijk, out); } @@ -683,14 +688,14 @@ H3Error H3_EXPORT(gridPathCells)(H3Index start, H3Index end, H3Index *out) { // Convert H3 addresses to IJK coords H3Error startError = cellToLocalIjk(start, start, &startIjk); - if (startError) { // LCOV_EXCL_BR_LINE + if (NEVER(startError)) { // Unreachable because this was called as part of gridDistance - return startError; // LCOV_EXCL_LINE + return startError; } H3Error endError = cellToLocalIjk(start, end, &endIjk); - if (endError) { // LCOV_EXCL_BR_LINE + if (NEVER(endError)) { // Unreachable because this was called as part of gridDistance - return endError; // LCOV_EXCL_LINE + return endError; } // Convert IJK to cube coordinates suitable for linear interpolation @@ -712,10 +717,10 @@ H3Error H3_EXPORT(gridPathCells)(H3Index start, H3Index end, H3Index *out) { // Convert cube -> ijk -> h3 index cubeToIjk(¤tIjk); H3Error currentError = localIjkToCell(start, ¤tIjk, &out[n]); - if (currentError) { // LCOV_EXCL_BR_LINE - // Expected to be unreachable since cells between `start` and `end` - // should have valid local IJK coordinates. - return currentError; // LCOV_EXCL_LINE + if (currentError) { + // The cells between `start` and `end` may fall in pentagon + // distortion. + return currentError; } } diff --git a/h3_mathExtensions.h b/h3_mathExtensions.h index 2ed47e3..f7dfb68 100644 --- a/h3_mathExtensions.h +++ b/h3_mathExtensions.h @@ -1,5 +1,5 @@ /* - * Copyright 2017-2018 Uber Technologies, Inc. + * Copyright 2017-2018, 2022 Uber Technologies, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,6 +27,14 @@ */ #define MAX(a, b) (((a) > (b)) ? (a) : (b)) +/** Evaluates to true if a + b would overflow for int32 */ +#define ADD_INT32S_OVERFLOWS(a, b) \ + ((a) > 0 ? (INT32_MAX - (a) < (b)) : (INT32_MIN - (a) > (b))) + +/** Evaluates to true if a - b would overflow for int32 */ +#define SUB_INT32S_OVERFLOWS(a, b) \ + ((a) >= 0 ? (INT32_MIN + (a) >= (b)) : (INT32_MAX + (a) + 1 < (b))) + // Internal functions int64_t _ipow(int64_t base, int64_t exp); diff --git a/h3_vertex.c b/h3_vertex.c index deec56a..7e072e7 100644 --- a/h3_vertex.c +++ b/h3_vertex.c @@ -25,6 +25,7 @@ #include "h3_algos.h" #include "h3_baseCells.h" #include "h3_faceijk.h" +#include "h3_h3Assert.h" #include "h3_h3Index.h" #include "h3_latLng.h" @@ -68,13 +69,17 @@ static H3Error vertexRotations(H3Index cell, int *out) { if (_isBaseCellPentagon(baseCell)) { // Find the appropriate direction-to-face mapping PentagonDirectionFaces dirFaces; - // Excluding from branch coverage as we never hit the end condition - for (int p = 0; p < NUM_PENTAGONS; p++) { // LCOV_EXCL_BR_LINE + // We never hit the end condition + int p = 0; + for (; p < NUM_PENTAGONS; p++) { if (pentagonDirectionFaces[p].baseCell == baseCell) { dirFaces = pentagonDirectionFaces[p]; break; } } + if (p == NUM_PENTAGONS) { + return E_FAILED; + } // additional CCW rotation for polar neighbors or IK neighbors if (fijk.face != baseFijk.face && @@ -237,7 +242,7 @@ H3Error H3_EXPORT(cellToVertex)(H3Index cell, int vertexNum, H3Index *out) { Direction right = directionForVertexNum( cell, (vertexNum - 1 + cellNumVerts) % cellNumVerts); // This case should be unreachable; invalid verts fail earlier - if (right == INVALID_DIGIT) return E_FAILED; // LCOV_EXCL_LINE + if (NEVER(right == INVALID_DIGIT)) return E_FAILED; int rRotations = 0; H3Index rightNeighbor; H3Error rightNeighborError =