Skip to content

Commit

Permalink
Fix some undefined behavior (#209)
Browse files Browse the repository at this point in the history
* Fix some undefined behavior

* Avoid overflow in line simplification calculations

* Oops, missed a test

* Didn't mean to add that to the Makefile

* Revert "Revert "[ci] test in debug mode (#202)""

This reverts commit c95c328.

* Fix reference to out-of-scope pointer

* Fix invalid shift and out-of-bounds vector element reference

* Update changelog and version
  • Loading branch information
e-n-f authored Mar 1, 2024
1 parent 312e156 commit 6a8f1b8
Show file tree
Hide file tree
Showing 38 changed files with 7,903 additions and 9,076 deletions.
7 changes: 5 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ on: [push]
jobs:
test:
runs-on: ubuntu-latest
strategy:
matrix:
version: ['Release', 'Debug']
steps:
- uses: actions/checkout@v3
- run: uname -a; make
- run: make test
- run: uname -a; BUILDTYPE=${{ matrix.version }} make
- run: make test
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# 2.48.0

* Fix some undefined behavior bugs, one of which results in slight changes to line simplification choices

# 2.47.0

* Stabilize feature order in tippecanoe-overzoom when --preserve-feature-order is specified but the sequence attribute is not present
Expand Down
4 changes: 1 addition & 3 deletions clip.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -904,9 +904,7 @@ std::string overzoom(const mvt_tile &tile, int oz, int ox, int oy, int nz, int n
flush_multiplier_cluster = true;
feature.tags.erase(feature.tags.begin() + i, feature.tags.begin() + i + 2);
}
}

if (layer.keys[feature.tags[i]] == retain_points_multiplier_sequence) {
} else if (i < (ssize_t) feature.tags.size() && layer.keys[feature.tags[i]] == retain_points_multiplier_sequence) {
mvt_value v = layer.values[feature.tags[i + 1]];
feature.seq = mvt_value_to_long_long(v);
feature.tags.erase(feature.tags.begin() + i, feature.tags.begin() + i + 2);
Expand Down
3 changes: 2 additions & 1 deletion geojson.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ int serialize_geojson_feature(struct serialization_state *sst, json_object *geom
if (id->type == JSON_NUMBER) {
if (id->value.number.number >= 0) {
char *err = NULL;
id_value = strtoull(milo::dtoa_milo(id->value.number.number).c_str(), &err, 10);
std::string id_number = milo::dtoa_milo(id->value.number.number);
id_value = strtoull(id_number.c_str(), &err, 10);

if (id->value.number.large_unsigned != 0) {
id_value = id->value.number.large_unsigned;
Expand Down
11 changes: 9 additions & 2 deletions geometry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,15 @@ bool point_within_tile(long long x, long long y, int z) {
double distance_from_line(long long point_x, long long point_y, long long segA_x, long long segA_y, long long segB_x, long long segB_y) {
long long p2x = segB_x - segA_x;
long long p2y = segB_y - segA_y;
double something = p2x * p2x + p2y * p2y;
double u = (0 == something) ? 0 : ((point_x - segA_x) * p2x + (point_y - segA_y) * p2y) / (something);

// These calculations must be made in integers instead of floating point
// to make them consistent between x86 and arm floating point implementations.
//
// Coordinates may be up to 34 bits, so their product is up to 68 bits,
// making their sum up to 69 bits. Downshift before multiplying to keep them in range.
double something = ((p2x / 4) * (p2x / 8) + (p2y / 4) * (p2y / 8)) * 32.0;
// likewise
double u = (0 == something) ? 0 : ((point_x - segA_x) / 4 * (p2x / 8) + (point_y - segA_y) / 4 * (p2y / 8)) * 32.0 / (something);

if (u >= 1) {
u = 1;
Expand Down
2 changes: 1 addition & 1 deletion pool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ inline int swizzlecmp(const char *a, int atype, unsigned long long ahash, const
return atype - btype;
}
} else {
return (int) ahash - (int) bhash;
return (int) (ahash - bhash);
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/coalesce-id/out/-z1_--coalesce_--reorder.json

Large diffs are not rendered by default.

16,726 changes: 7,768 additions & 8,958 deletions ...i/out/-z10_--retain-points-multiplier_10_-M10000_--drop-smallest-as-needed.json

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@
,
{ "type": "Feature", "properties": { "tippecanoe:retain_points_multiplier_first": true, "tippecanoe:retain_points_multiplier_sequence": 96 }, "geometry": { "type": "MultiPolygon", "coordinates": [ [ [ [ 174.375000, -40.979898 ], [ 172.968750, -44.087585 ], [ 168.750000, -47.040182 ], [ 165.937500, -46.073231 ], [ 171.562500, -40.979898 ], [ 174.375000, -40.979898 ] ] ], [ [ [ 172.968750, -34.307144 ], [ 175.781250, -37.718590 ], [ 178.593750, -37.718590 ], [ 175.781250, -42.032974 ], [ 172.968750, -34.307144 ] ] ], [ [ [ -187.031250, -35.460670 ], [ -181.406250, -37.718590 ], [ -184.218750, -42.032974 ], [ -187.031250, -35.460670 ] ] ] ] } }
,
{ "type": "Feature", "properties": { "tippecanoe:retain_points_multiplier_first": true, "tippecanoe:retain_points_multiplier_sequence": 4 }, "geometry": { "type": "MultiPolygon", "coordinates": [ [ [ [ -60.468750, -79.687184 ], [ -59.062500, -79.935918 ], [ -60.468750, -81.093214 ], [ -66.093750, -80.647035 ], [ -66.093750, -80.178713 ], [ -61.875000, -80.415707 ], [ -60.468750, -79.687184 ] ] ], [ [ [ -46.406250, -77.767582 ], [ -43.593750, -78.349411 ], [ -43.593750, -79.935918 ], [ -50.625000, -81.093214 ], [ -54.843750, -80.647035 ], [ -50.625000, -79.687184 ], [ -49.218750, -78.061989 ], [ -46.406250, -77.767582 ] ] ], [ [ [ -149.062500, -85.622069 ], [ -143.437500, -85.051129 ], [ -143.437500, -84.541361 ], [ -150.468750, -84.267172 ], [ -150.468750, -83.829945 ], [ -153.281250, -83.676943 ], [ -153.281250, -82.118384 ], [ -157.500000, -81.093214 ], [ -150.468750, -81.308321 ], [ -146.250000, -79.935918 ], [ -154.687500, -79.171335 ], [ -158.906250, -76.840816 ], [ -151.875000, -77.466028 ], [ -146.250000, -76.516819 ], [ -144.843750, -75.140778 ], [ -139.218750, -75.140778 ], [ -136.406250, -74.402163 ], [ -116.718750, -74.402163 ], [ -113.906250, -73.627789 ], [ -112.500000, -74.775843 ], [ -111.093750, -74.402163 ], [ -106.875000, -75.140778 ], [ -101.250000, -75.140778 ], [ -99.843750, -74.775843 ], [ -104.062500, -72.816074 ], [ -98.437500, -72.816074 ], [ -95.625000, -73.627789 ], [ -90.000000, -73.226700 ], [ -88.593750, -72.395706 ], [ -85.781250, -73.627789 ], [ -81.562500, -74.019543 ], [ -80.156250, -73.226700 ], [ -77.343750, -73.226700 ], [ -74.531250, -74.019543 ], [ -73.125000, -73.226700 ], [ -67.500000, -72.816074 ], [ -68.906250, -69.162558 ], [ -66.093750, -65.946472 ], [ -57.656250, -63.548552 ], [ -63.281250, -65.366837 ], [ -61.875000, -65.946472 ], [ -66.093750, -67.609221 ], [ -61.875000, -70.140364 ], [ -60.468750, -73.627789 ], [ -70.312500, -76.516819 ], [ -77.343750, -76.840816 ], [ -73.125000, -77.767582 ], [ -74.531250, -78.349411 ], [ -77.343750, -78.349411 ], [ -75.937500, -80.178713 ], [ -59.062500, -82.308893 ], [ -57.656250, -83.194896 ], [ -52.031250, -81.923186 ], [ -47.812500, -81.723188 ], [ -42.187500, -82.118384 ], [ -40.781250, -81.308321 ], [ -28.125000, -80.415707 ], [ -29.531250, -79.171335 ], [ -36.562500, -79.171335 ], [ -35.156250, -78.061989 ], [ -29.531250, -77.157163 ], [ -29.531250, -76.516819 ], [ -22.500000, -76.184995 ], [ -16.875000, -75.140778 ], [ -15.468750, -73.226700 ], [ -9.843750, -71.074056 ], [ 1.406250, -71.524909 ], [ 8.437500, -69.657086 ], [ 9.843750, -70.612614 ], [ 15.468750, -70.612614 ], [ 18.281250, -69.657086 ], [ 26.718750, -70.612614 ], [ 33.750000, -68.656555 ], [ 39.375000, -69.657086 ], [ 52.031250, -65.946472 ], [ 56.250000, -65.946472 ], [ 61.875000, -68.138852 ], [ 68.906250, -68.138852 ], [ 70.312500, -69.162558 ], [ 67.500000, -70.140364 ], [ 67.500000, -71.965388 ], [ 70.312500, -72.395706 ], [ 73.125000, -70.140364 ], [ 78.750000, -69.162558 ], [ 78.750000, -68.138852 ], [ 87.187500, -67.067433 ], [ 88.593750, -65.946472 ], [ 88.593750, -67.067433 ], [ 95.625000, -67.609221 ], [ 99.843750, -67.067433 ], [ 102.656250, -65.366837 ], [ 106.875000, -67.067433 ], [ 109.687500, -67.067433 ], [ 111.093750, -65.946472 ], [ 116.718750, -67.067433 ], [ 133.593750, -66.513260 ], [ 135.000000, -65.366837 ], [ 137.812500, -67.067433 ], [ 146.250000, -67.067433 ], [ 146.250000, -68.138852 ], [ 154.687500, -68.656555 ], [ 161.718750, -70.612614 ], [ 170.156250, -71.074056 ], [ 170.156250, -73.226700 ], [ 167.343750, -73.627789 ], [ 163.125000, -75.845169 ], [ 163.125000, -77.157163 ], [ 167.343750, -78.630006 ], [ 161.718750, -79.171335 ], [ 160.312500, -80.872827 ], [ 163.125000, -82.308893 ], [ 168.750000, -83.359511 ], [ 168.750000, -83.829945 ], [ 172.968750, -83.979259 ], [ 172.968750, -84.405941 ], [ 175.781250, -84.124973 ], [ 178.593750, -84.405941 ], [ 180.000000, -85.051129 ], [ 180.000000, -84.673513 ], [ 181.406250, -84.124973 ], [ 182.812500, -84.405941 ], [ 184.218750, -84.124973 ], [ 185.625000, -84.541361 ], [ 187.031250, -84.124973 ], [ 187.031250, -85.622069 ], [ 180.000000, -85.622069 ], [ -149.062500, -85.622069 ] ] ], [ [ [ -163.125000, -78.349411 ], [ -158.906250, -79.171335 ], [ -161.718750, -79.687184 ], [ -163.125000, -78.349411 ] ] ], [ [ [ -120.937500, -72.816074 ], [ -120.937500, -73.627789 ], [ -123.750000, -73.627789 ], [ -123.750000, -72.816074 ], [ -120.937500, -72.816074 ] ] ], [ [ [ -101.250000, -71.524909 ], [ -99.843750, -71.965388 ], [ -95.625000, -72.395706 ], [ -101.250000, -72.395706 ], [ -101.250000, -71.524909 ] ] ], [ [ [ -70.312500, -68.656555 ], [ -68.906250, -71.965388 ], [ -74.531250, -72.395706 ], [ -74.531250, -71.074056 ], [ -71.718750, -71.074056 ], [ -70.312500, -68.656555 ] ] ], [ [ [ -149.062500, -85.622069 ], [ -180.000000, -85.622069 ], [ -187.031250, -85.622069 ], [ -187.031250, -84.405941 ], [ -184.218750, -84.124973 ], [ -180.000000, -84.673513 ], [ -180.000000, -85.051129 ], [ -178.593750, -84.124973 ], [ -177.187500, -84.405941 ], [ -175.781250, -84.124973 ], [ -174.375000, -84.541361 ], [ -170.156250, -83.829945 ], [ -163.125000, -85.051129 ], [ -157.500000, -85.402036 ], [ -154.687500, -85.051129 ], [ -150.468750, -85.287916 ], [ -149.062500, -85.622069 ] ] ] ] } }
{ "type": "Feature", "properties": { "tippecanoe:retain_points_multiplier_first": true, "tippecanoe:retain_points_multiplier_sequence": 4 }, "geometry": { "type": "MultiPolygon", "coordinates": [ [ [ [ -60.468750, -79.687184 ], [ -59.062500, -79.935918 ], [ -60.468750, -81.093214 ], [ -66.093750, -80.647035 ], [ -66.093750, -80.178713 ], [ -61.875000, -80.415707 ], [ -60.468750, -79.687184 ] ] ], [ [ [ -46.406250, -77.767582 ], [ -43.593750, -78.349411 ], [ -43.593750, -79.935918 ], [ -50.625000, -81.093214 ], [ -54.843750, -80.647035 ], [ -50.625000, -79.687184 ], [ -49.218750, -78.061989 ], [ -46.406250, -77.767582 ] ] ], [ [ [ -149.062500, -85.622069 ], [ -143.437500, -85.051129 ], [ -143.437500, -84.541361 ], [ -150.468750, -84.267172 ], [ -150.468750, -83.829945 ], [ -153.281250, -83.676943 ], [ -153.281250, -82.118384 ], [ -157.500000, -81.093214 ], [ -150.468750, -81.308321 ], [ -146.250000, -79.935918 ], [ -154.687500, -79.171335 ], [ -158.906250, -76.840816 ], [ -151.875000, -77.466028 ], [ -146.250000, -76.516819 ], [ -144.843750, -75.140778 ], [ -139.218750, -75.140778 ], [ -136.406250, -74.402163 ], [ -116.718750, -74.402163 ], [ -113.906250, -73.627789 ], [ -112.500000, -74.775843 ], [ -111.093750, -74.402163 ], [ -106.875000, -75.140778 ], [ -101.250000, -75.140778 ], [ -99.843750, -74.775843 ], [ -104.062500, -72.816074 ], [ -98.437500, -72.816074 ], [ -95.625000, -73.627789 ], [ -90.000000, -73.226700 ], [ -88.593750, -72.395706 ], [ -85.781250, -73.627789 ], [ -81.562500, -74.019543 ], [ -80.156250, -73.226700 ], [ -77.343750, -73.226700 ], [ -74.531250, -74.019543 ], [ -73.125000, -73.226700 ], [ -67.500000, -72.816074 ], [ -68.906250, -69.162558 ], [ -66.093750, -65.946472 ], [ -57.656250, -63.548552 ], [ -63.281250, -65.366837 ], [ -61.875000, -65.946472 ], [ -66.093750, -67.609221 ], [ -61.875000, -70.140364 ], [ -60.468750, -73.627789 ], [ -70.312500, -76.516819 ], [ -77.343750, -76.840816 ], [ -73.125000, -77.767582 ], [ -74.531250, -78.349411 ], [ -77.343750, -78.349411 ], [ -75.937500, -80.178713 ], [ -59.062500, -82.308893 ], [ -57.656250, -83.194896 ], [ -52.031250, -81.923186 ], [ -47.812500, -81.723188 ], [ -42.187500, -82.118384 ], [ -40.781250, -81.308321 ], [ -28.125000, -80.415707 ], [ -29.531250, -79.171335 ], [ -36.562500, -79.171335 ], [ -35.156250, -78.061989 ], [ -29.531250, -77.157163 ], [ -29.531250, -76.516819 ], [ -22.500000, -76.184995 ], [ -16.875000, -75.140778 ], [ -15.468750, -73.226700 ], [ -9.843750, -71.074056 ], [ 1.406250, -71.524909 ], [ 8.437500, -69.657086 ], [ 9.843750, -70.612614 ], [ 15.468750, -70.612614 ], [ 18.281250, -69.657086 ], [ 26.718750, -70.612614 ], [ 33.750000, -68.656555 ], [ 39.375000, -69.657086 ], [ 52.031250, -65.946472 ], [ 56.250000, -65.946472 ], [ 61.875000, -68.138852 ], [ 68.906250, -68.138852 ], [ 70.312500, -69.162558 ], [ 67.500000, -70.140364 ], [ 67.500000, -71.965388 ], [ 70.312500, -72.395706 ], [ 73.125000, -70.140364 ], [ 78.750000, -69.162558 ], [ 78.750000, -68.138852 ], [ 87.187500, -67.067433 ], [ 88.593750, -65.946472 ], [ 88.593750, -67.067433 ], [ 95.625000, -67.609221 ], [ 99.843750, -67.067433 ], [ 102.656250, -65.366837 ], [ 106.875000, -67.067433 ], [ 109.687500, -67.067433 ], [ 111.093750, -65.946472 ], [ 116.718750, -67.067433 ], [ 133.593750, -66.513260 ], [ 135.000000, -65.366837 ], [ 137.812500, -67.067433 ], [ 146.250000, -67.067433 ], [ 146.250000, -68.138852 ], [ 154.687500, -68.656555 ], [ 161.718750, -70.612614 ], [ 167.343750, -70.612614 ], [ 171.562500, -71.524909 ], [ 170.156250, -73.226700 ], [ 167.343750, -73.627789 ], [ 163.125000, -75.845169 ], [ 163.125000, -77.157163 ], [ 167.343750, -78.630006 ], [ 161.718750, -79.171335 ], [ 160.312500, -80.872827 ], [ 163.125000, -82.308893 ], [ 168.750000, -83.359511 ], [ 168.750000, -83.829945 ], [ 172.968750, -83.979259 ], [ 172.968750, -84.405941 ], [ 175.781250, -84.124973 ], [ 178.593750, -84.405941 ], [ 180.000000, -85.051129 ], [ 180.000000, -84.673513 ], [ 181.406250, -84.124973 ], [ 182.812500, -84.405941 ], [ 184.218750, -84.124973 ], [ 185.625000, -84.541361 ], [ 187.031250, -84.124973 ], [ 187.031250, -85.622069 ], [ 180.000000, -85.622069 ], [ -149.062500, -85.622069 ] ] ], [ [ [ -163.125000, -78.349411 ], [ -158.906250, -79.171335 ], [ -161.718750, -79.687184 ], [ -163.125000, -78.349411 ] ] ], [ [ [ -120.937500, -72.816074 ], [ -120.937500, -73.627789 ], [ -123.750000, -73.627789 ], [ -123.750000, -72.816074 ], [ -120.937500, -72.816074 ] ] ], [ [ [ -101.250000, -71.524909 ], [ -99.843750, -71.965388 ], [ -95.625000, -72.395706 ], [ -101.250000, -72.395706 ], [ -101.250000, -71.524909 ] ] ], [ [ [ -70.312500, -68.656555 ], [ -68.906250, -71.965388 ], [ -74.531250, -72.395706 ], [ -74.531250, -71.074056 ], [ -71.718750, -71.074056 ], [ -70.312500, -68.656555 ] ] ], [ [ [ -149.062500, -85.622069 ], [ -180.000000, -85.622069 ], [ -187.031250, -85.622069 ], [ -187.031250, -84.405941 ], [ -184.218750, -84.124973 ], [ -180.000000, -84.673513 ], [ -180.000000, -85.051129 ], [ -178.593750, -84.124973 ], [ -177.187500, -84.405941 ], [ -175.781250, -84.124973 ], [ -174.375000, -84.541361 ], [ -170.156250, -83.829945 ], [ -163.125000, -85.051129 ], [ -157.500000, -85.402036 ], [ -154.687500, -85.051129 ], [ -150.468750, -85.287916 ], [ -149.062500, -85.622069 ] ] ] ] } }
] }
] }
] }
Loading

0 comments on commit 6a8f1b8

Please sign in to comment.