Remove endianness-dependent assertion in polygonToCells fuzzer test (#964)#1182
Conversation
|
|
|
coverage: 99.176%. remained the same — Scottcjn:fix/issue-964-bigendian-test into uber:master |
isaacbrodsky
left a comment
isaacbrodsky
left a comment
There was a problem hiding this comment.
Maybe we can more portably specify the exact double literals? That being said this does not seem to be an important part of this test.
c519470 to
55c3fd1
Compare
|
Good call — done. I replaced the raw byte buffer with the exact Verified on the native big-endian POWER8 (ppc64) host: the test now passes with |
55c3fd1 to
ddd1cda
Compare
| 0xff, 0xff, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, | ||
| 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xa, 0x0, 0x0, 0xff, | ||
| 0xff, 0x0, 0x0, 0x0, 0xa, 0xa, 0xa, 0xa, 0xa, 0xff, | ||
| // The vertices below are the exact doubles that the original fuzzer |
There was a problem hiding this comment.
I think make format is needed here
The fuzzer_crash regression test built its polygon by reinterpreting a raw byte buffer as LatLng doubles (verts = (LatLng *)data), so the decoded vertices -- and the resulting cell count -- depended on the host's floating-point byte order. On big-endian architectures the same bytes decode to different doubles (the first vertex's latitude is even NaN), so maxPolygonToCellsSizeExperimental returns sz=3 instead of 1 and the sz == 1 assertion fails. Specify the vertices as explicit hex-float literals (the exact values the original bytes decoded to) so the test uses identical input on every architecture. This keeps the sz == 1 check meaningful and reproduces the original fuzzer scenario everywhere. Verified on a native big-endian POWER8 (ppc64) host: the test passes, and it continues to pass on little-endian.
ddd1cda to
31d1e6a
Compare
|
Good catch — ran |
5 participants
Fixes #964.
testPolygonToCellsReportedExperimental'sfuzzer_crashtest reinterprets a raw byte array asLatLngdoubles (verts = (LatLng *)data), so the decoded polygon — and the cell count it produces — depends on the host's floating-point byte order.I reproduced the failure on a native big-endian POWER8 (ppc64) host (independent of the s390x build in #964). On big-endian the same bytes decode to different doubles:
NaN(vs a tiny subnormal on little-endian)maxPolygonToCellsSizeExperimentalsucceeds (no error) but returnssz = 3instead of1, so thesz == 1assertion failsSo this is the test data, not the algorithm — exactly as suspected in the issue. The
maxPolygonToCellsSizeExperimental/polygonToCellsExperimentalcalls (the part the fuzzer originally crashed in) never crash here; only the input-dependent count differs.Since this is a regression test for the fuzzer crash rather than a check of a meaningful output count, this PR drops the non-portable
sz == 1assertion and documents the endianness dependence in a comment. The calls that actually exercise the crash path are kept and still verified to succeed.Verified on the native big-endian POWER8 host: the full
testPolygonToCellsReportedExperimentalsuite now passes (3452 assertions), and it continues to pass on little-endian.