Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix undefined behavior in polygonToCells test #636

Merged
merged 4 commits into from
Aug 16, 2022

Conversation

isaacbrodsky
Copy link
Collaborator

@isaacbrodsky isaacbrodsky commented Aug 8, 2022

  • Fixes a test that had an invalid read. This was found by running the tests with ENABLE_FUZZERS, which enabled ASAN etc. We don't run this in CI but perhaps should.
  • Added a couple tests that exercise what happens when non-finite values are passed into polygonToCells. We previously had some casts from double to int64_t that were unsafe if the value was not finite because that cast would be undefined behavior.

Edit: happy to break this out into two PRs if that's easier.

Edit 2: This does not block #633

@coveralls
Copy link

coveralls commented Aug 8, 2022

Coverage Status

Coverage increased (+0.2%) to 98.863% when pulling 9378b6c on isaacbrodsky:fix-polygon-undefined-test into 6c77803 on uber:master.

@@ -116,7 +116,7 @@ SUITE(polygon) {
// This is a carefully crafted shape + point to hit an otherwise
// missed branch in coverage
LatLng verts[] = {{0, 0}, {1, 0.5}, {0, 1}};
GeoLoop geoloop = {.numVerts = 4, .verts = verts};
GeoLoop geoloop = {.numVerts = 3, .verts = verts};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏

src/h3lib/lib/bbox.c Outdated Show resolved Hide resolved
src/h3lib/lib/bbox.c Outdated Show resolved Hide resolved
src/apps/testapps/testPolygonToCells.c Outdated Show resolved Hide resolved
src/apps/testapps/testPolygonToCells.c Outdated Show resolved Hide resolved
@isaacbrodsky isaacbrodsky merged commit f6669a7 into uber:master Aug 16, 2022
@isaacbrodsky isaacbrodsky deleted the fix-polygon-undefined-test branch August 16, 2022 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants