Be careful to avoid undefined behavior from shifting negative numbers

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

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<unsigned long long> 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;

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);
 				}
 			}
 		}

version.hpp

@@ -1,6 +1,6 @@
 #ifndef VERSION_HPP
 #define VERSION_HPP
 
-#define VERSION "v1.34.3"
+#define VERSION "v1.34.4"
 
 #endif