From fc335e2221f534ab19067432ee9f862619f5b353 Mon Sep 17 00:00:00 2001 From: Eric Fischer Date: Mon, 17 Jun 2019 17:11:31 -0700 Subject: [PATCH] Be careful to avoid undefined behavior from shifting negative numbers --- CHANGELOG.md | 4 ++++ serial.cpp | 27 ++++++++++++++++----------- tile.cpp | 8 ++++++-- version.hpp | 2 +- 4 files changed, 27 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9351d0..15cb196 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 1.34.4 + +* Be careful to avoid undefined behavior from shifting negative numbers + ## 1.34.3 * Add an option to keep intersection nodes from being simplified away diff --git a/serial.cpp b/serial.cpp index 149de01..797e4b5 100644 --- a/serial.cpp +++ b/serial.cpp @@ -21,6 +21,11 @@ #include "evaluator.hpp" #include "milo/dtoa_milo.h" +// Offset coordinates to keep them positive +#define COORD_OFFSET (4LL << 32) +#define SHIFT_RIGHT(a) ((((a) + COORD_OFFSET) >> geometry_scale) - (COORD_OFFSET >> geometry_scale)) +#define SHIFT_LEFT(a) ((((a) + (COORD_OFFSET >> geometry_scale)) << geometry_scale) - COORD_OFFSET) + size_t fwrite_check(const void *ptr, size_t size, size_t nitems, FILE *stream, const char *fname) { size_t w = fwrite(ptr, size, nitems, stream); if (w != nitems) { @@ -359,8 +364,8 @@ static long long scale_geometry(struct serialization_state *sst, long long *bbox *(sst->initial_x) = 1LL << 31; *(sst->initial_y) = 1LL << 31; } else { - *(sst->initial_x) = (x >> geometry_scale) << geometry_scale; - *(sst->initial_y) = (y >> geometry_scale) << geometry_scale; + *(sst->initial_x) = (((x + COORD_OFFSET) >> geometry_scale) << geometry_scale) - COORD_OFFSET; + *(sst->initial_y) = (((y + COORD_OFFSET) >> geometry_scale) << geometry_scale) - COORD_OFFSET; } *(sst->initialized) = 1; @@ -374,8 +379,8 @@ static long long scale_geometry(struct serialization_state *sst, long long *bbox geom[i].x = std::round(x * scale); geom[i].y = std::round(y * scale); } else { - geom[i].x = x >> geometry_scale; - geom[i].y = y >> geometry_scale; + geom[i].x = SHIFT_RIGHT(x); + geom[i].y = SHIFT_RIGHT(y); } } } @@ -412,12 +417,12 @@ int serialize_feature(struct serialization_state *sst, serial_feature &sf) { for (auto &c : clipbboxes) { if (sf.t == VT_POLYGON) { - sf.geometry = simple_clip_poly(sf.geometry, c.minx >> geometry_scale, c.miny >> geometry_scale, c.maxx >> geometry_scale, c.maxy >> geometry_scale); + sf.geometry = simple_clip_poly(sf.geometry, SHIFT_RIGHT(c.minx), SHIFT_RIGHT(c.miny), SHIFT_RIGHT(c.maxx), SHIFT_RIGHT(c.maxy)); } else if (sf.t == VT_LINE) { - sf.geometry = clip_lines(sf.geometry, c.minx >> geometry_scale, c.miny >> geometry_scale, c.maxx >> geometry_scale, c.maxy >> geometry_scale); + sf.geometry = clip_lines(sf.geometry, SHIFT_RIGHT(c.minx), SHIFT_RIGHT(c.miny), SHIFT_RIGHT(c.maxx), SHIFT_RIGHT(c.maxy)); sf.geometry = remove_noop(sf.geometry, sf.t, 0); } else if (sf.t == VT_POINT) { - sf.geometry = clip_point(sf.geometry, c.minx >> geometry_scale, c.miny >> geometry_scale, c.maxx >> geometry_scale, c.maxy >> geometry_scale); + sf.geometry = clip_point(sf.geometry, SHIFT_RIGHT(c.minx), SHIFT_RIGHT(c.miny), SHIFT_RIGHT(c.maxx), SHIFT_RIGHT(c.maxy)); } sf.bbox[0] = LLONG_MAX; @@ -426,8 +431,8 @@ int serialize_feature(struct serialization_state *sst, serial_feature &sf) { sf.bbox[3] = LLONG_MIN; for (auto &g : sf.geometry) { - long long x = g.x << geometry_scale; - long long y = g.y << geometry_scale; + long long x = SHIFT_LEFT(g.x); + long long y = SHIFT_LEFT(g.y); if (x < sf.bbox[0]) { sf.bbox[0] = x; @@ -460,7 +465,7 @@ int serialize_feature(struct serialization_state *sst, serial_feature &sf) { std::vector locs; for (size_t i = 0; i < sf.geometry.size(); i++) { if (sf.geometry[i].op == VT_MOVETO || sf.geometry[i].op == VT_LINETO) { - locs.push_back(encode_index(sf.geometry[i].x << geometry_scale, sf.geometry[i].y << geometry_scale)); + locs.push_back(encode_index(SHIFT_LEFT(sf.geometry[i].x), SHIFT_LEFT(sf.geometry[i].y))); } } std::sort(locs.begin(), locs.end()); @@ -662,7 +667,7 @@ int serialize_feature(struct serialization_state *sst, serial_feature &sf) { } long long geomstart = r->geompos; - serialize_feature(r->geomfile, &sf, &r->geompos, sst->fname, *(sst->initial_x) >> geometry_scale, *(sst->initial_y) >> geometry_scale, false); + serialize_feature(r->geomfile, &sf, &r->geompos, sst->fname, SHIFT_RIGHT(*(sst->initial_x)), SHIFT_RIGHT(*(sst->initial_y)), false); struct index index; index.start = geomstart; diff --git a/tile.cpp b/tile.cpp index 8f7f315..2cf9552 100644 --- a/tile.cpp +++ b/tile.cpp @@ -48,6 +48,10 @@ extern "C" { #define CMD_BITS 3 +// Offset coordinates to keep them positive +#define COORD_OFFSET (4LL << 32) +#define SHIFT_RIGHT(a) ((((a) + COORD_OFFSET) >> geometry_scale) - (COORD_OFFSET >> geometry_scale)) + #define XSTRINGIFY(s) STRINGIFY(s) #define STRINGIFY(s) #s @@ -284,7 +288,7 @@ void rewrite(drawvec &geom, int z, int nextzoom, int maxzoom, long long *bbox, u drawvec geom2; for (size_t i = 0; i < geom.size(); i++) { - geom2.push_back(draw(geom[i].op, (geom[i].x + sx) >> geometry_scale, (geom[i].y + sy) >> geometry_scale)); + geom2.push_back(draw(geom[i].op, SHIFT_RIGHT(geom[i].x + sx), SHIFT_RIGHT(geom[i].y + sy))); } for (xo = bbox2[0]; xo <= bbox2[2]; xo++) { @@ -344,7 +348,7 @@ void rewrite(drawvec &geom, int z, int nextzoom, int maxzoom, long long *bbox, u } } - serialize_feature(geomfile[j], &sf, &geompos[j], fname, initial_x[segment] >> geometry_scale, initial_y[segment] >> geometry_scale, true); + serialize_feature(geomfile[j], &sf, &geompos[j], fname, SHIFT_RIGHT(initial_x[segment]), SHIFT_RIGHT(initial_y[segment]), true); } } } diff --git a/version.hpp b/version.hpp index 6670115..10a8e40 100644 --- a/version.hpp +++ b/version.hpp @@ -1,6 +1,6 @@ #ifndef VERSION_HPP #define VERSION_HPP -#define VERSION "v1.34.3" +#define VERSION "v1.34.4" #endif