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

Bug fixes for compactCells #558

Merged
merged 38 commits into from
Jan 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
0277ce5
Add LLVM fuzzer harness
Dec 22, 2021
1c8ec2e
Add AFL++ test case generator
Dec 22, 2021
b89c76b
Fuzz more gridDisk functions
Dec 22, 2021
79d2e2c
add fuzzerH3SetToLinkedGeo
Dec 22, 2021
a28a807
Add more fuzzers
Dec 22, 2021
9445cbb
Additional fuzzers
Dec 22, 2021
f971dcf
add fuzzerVertexes
Dec 22, 2021
e0c8841
Add test-fuzzer script
Dec 22, 2021
007b7c3
Fix linux build
Dec 22, 2021
02abb99
Fix fuzzerIndexIO
Dec 22, 2021
caedc90
test-fuzzer use subshell for ls
isaacbrodsky Dec 22, 2021
de5a2c3
Update test-fuzzer again
isaacbrodsky Dec 22, 2021
8bbc36a
Fix test-fuzzer again
isaacbrodsky Dec 22, 2021
021c994
fuzzerCompact
Dec 23, 2021
2195863
Update readme
Dec 23, 2021
4a65123
libFuzzer tests
Dec 23, 2021
1e7066e
reformat header
Dec 23, 2021
0ede718
README updates
Dec 23, 2021
65b4ef3
fuzzerDirectedEdge
Dec 23, 2021
79b1f44
fuzzerLocalIj
Dec 23, 2021
25360ef
fix fuzzerDirectedEdge build
Dec 24, 2021
ac4b918
Fix fuzzer programs
isaacbrodsky Dec 27, 2021
1145bce
remove logging
isaacbrodsky Dec 27, 2021
cd14266
remove h3Println
isaacbrodsky Dec 27, 2021
76532d8
add fuzzerPoylgonToCells
isaacbrodsky Jan 3, 2022
84bc4e9
Update per review
Jan 3, 2022
323d9e9
Merge branch 'master' into llvm-fuzzer-harness
Jan 3, 2022
e04c62c
Add comment on memcpy per review
Jan 3, 2022
0016f1c
Fix potential crash in vertexRotations
Jan 3, 2022
7807131
Merge branch 'master' into llvm-fuzzer-harness
Jan 3, 2022
4b4e623
Catch possible failure in getIcosahedronFaces
Jan 3, 2022
d501e51
Don't assert specific error in testVertex
Jan 3, 2022
caf34a9
Compact fuzzer updates and bugfixes
Jan 13, 2022
7244efe
Merge branch 'master' into llvm-fuzzer-compact-2
Jan 17, 2022
82f5531
Fix assertion message in testCompactCells.c
isaacbrodsky Jan 22, 2022
05980d0
Change assert messages
isaacbrodsky Jan 23, 2022
560f06f
Merge branch 'master' into llvm-fuzzer-compact-2
isaacbrodsky Jan 23, 2022
2392c4e
Fix formatting
isaacbrodsky Jan 23, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 2 additions & 25 deletions src/apps/fuzzers/fuzzerCompact.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,35 +30,13 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
}

uint8_t res = *data;
uint8_t uncompactRes = (*(data + 1)) % (MAX_UNCOMPACT_RES + 1);
H3Index *input = (H3Index *)(data + sizeof(H3Index));
size_t inputSize = (size - sizeof(H3Index)) / sizeof(H3Index);

// fuzz compactCells
H3Index *compacted = calloc(inputSize, sizeof(H3Index));
H3Error compactErr = H3_EXPORT(compactCells)(input, compacted, inputSize);

// fuzz uncompactCells using output of above
if (!compactErr) {
int compactedCount = 0;
for (int i = 0; i < inputSize; i++) {
if (compacted[i] != H3_NULL) {
compactedCount++;
}
}
if (compactedCount < 2) {
int64_t uncompactedSize;
H3Error err = H3_EXPORT(uncompactCellsSize)(
compacted, inputSize, uncompactRes, &uncompactedSize);
if (!err && uncompactedSize < MAX_UNCOMPACT_SIZE) {
H3Index *uncompacted = calloc(uncompactedSize, sizeof(H3Index));
H3_EXPORT(uncompactCells)
(compacted, compactedCount, uncompacted, uncompactedSize,
uncompactRes);
free(uncompacted);
}
}
}
H3_EXPORT(compactCells)(input, compacted, inputSize);
free(compacted);

// fuzz uncompactCells using the original input
int64_t uncompactedSize;
Expand All @@ -71,7 +49,6 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
(input, inputSize, uncompacted, uncompactedSize, res);
free(uncompacted);
}
free(compacted);

return 0;
}
Expand Down
25 changes: 25 additions & 0 deletions src/apps/testapps/testCompactCells.c
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,31 @@ SUITE(compactCells) {
}
}

TEST(compactCells_reservedBitsSet) {
const int numHex = 7;
H3Index bad[] = {
0x0010000000010000, 0x0180c6c6c6c61616, 0x1616ffffffffffff,
0xffff8affffffffff, 0xffffffffffffc6c6, 0xffffffffffffffc6,
0xc6c6c6c6c66fffe0,
};
H3Index output[] = {0, 0, 0, 0, 0, 0, 0};

t_assert(H3_EXPORT(compactCells)(bad, output, numHex) == E_CELL_INVALID,
"compactCells returns E_CELL_INVALID on bad input");
}

TEST(compactCells_parentError) {
const int numHex = 3;
H3Index bad[] = {0, 0, 0};
H3Index output[] = {0, 0, 0};
H3_SET_RESOLUTION(bad[0], 10);
H3_SET_RESOLUTION(bad[1], 5);

t_assert(
H3_EXPORT(compactCells)(bad, output, numHex) == E_RES_MISMATCH,
"compactCells returns E_RES_MISMATCH on bad input (parent error)");
}

TEST(uncompactCells_wrongRes) {
int numHex = 3;
H3Index someHexagons[] = {0, 0, 0};
Expand Down
20 changes: 17 additions & 3 deletions src/h3lib/lib/h3Index.c
Original file line number Diff line number Diff line change
Expand Up @@ -330,16 +330,27 @@ H3Error H3_EXPORT(compactCells)(const H3Index *h3Set, H3Index *compactedSet,
for (int i = 0; i < numRemainingHexes; i++) {
H3Index currIndex = remainingHexes[i];
if (currIndex != 0) {
// If the reserved bits were set by the caller, the
// algorithm below may encounter undefined behavior
// because it expects to have set the reserved bits
// itself.
if (H3_GET_RESERVED_BITS(currIndex) != 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not required, but I wonder if it would make sense to do this kind of validity check first, and fail faster? We could do this without allocating any memory we need to free, and it wouldn't change the complexity

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd leave that to a separate PR with benchmarks to say

H3_MEMORY(free)(remainingHexes);
H3_MEMORY(free)(hashSetArray);
return E_CELL_INVALID;
}

H3Index parent;
H3Error parentError =
H3_EXPORT(cellToParent)(currIndex, parentRes, &parent);
// LCOV_EXCL_START
// Should never be reachable as a result of the compact
// algorithm.
// algorithm. Can happen if cellToParent errors e.g.
// because of incompatible resolutions.
if (parentError) {
H3_MEMORY(free)(remainingHexes);
H3_MEMORY(free)(hashSetArray);
return parentError;
}
// LCOV_EXCL_STOP
// Modulus hash the parent into the temp array
int loc = (int)(parent % numRemainingHexes);
int loopCount = 0;
Expand Down Expand Up @@ -432,6 +443,9 @@ H3Error H3_EXPORT(compactCells)(const H3Index *h3Set, H3Index *compactedSet,
// Should never be reachable as a result of the compact
// algorithm.
if (parentError) {
// TODO: Determine if this is somehow reachable.
H3_MEMORY(free)(remainingHexes);
H3_MEMORY(free)(hashSetArray);
return parentError;
}
// LCOV_EXCL_STOP
Expand Down