Fix undefined memory access in getCoveringRanges() (#9227)

* Add simple unit tests for SymbolSizeBinder

* Fix bug in symbol size uniform value calculation

For camera functions we were setting the zoom levels in "covering ranges" to
`[(zoom stop <= tile zoom), (zoom stop >= 1 + tile zoom)]`, but then evaluating
the function at `[tile_zoom, tile_zoom + 1]`.

* Check for it != end() before accessing it->first

cmake/test-files.cmake

@@ -40,6 +40,7 @@ set(MBGL_TEST_FILES
 
     # programs
     test/programs/binary_program.test.cpp
+    test/programs/symbol_program.test.cpp
 
     # renderer
     test/renderer/group_by_layout.test.cpp

include/mbgl/style/function/composite_function.hpp

@@ -59,7 +59,7 @@ class CompositeFunction {
 
                 // lower_bound yields first element >= zoom, but we want the *last*
                 // element <= zoom, so if we found a stop > zoom, back up by one.
-                if (minIt != s.stops.begin() && minIt->first > zoom) {
+                if (minIt != s.stops.begin() && minIt != s.stops.end() && minIt->first > zoom) {
                     minIt--;
                 }
 

src/mbgl/programs/symbol_program.hpp

@@ -121,7 +121,7 @@ Range<float> getCoveringStops(Stops s, float lowerZoom, float upperZoom) {
 
     // lower_bound yields first element >= lowerZoom, but we want the *last*
     // element <= lowerZoom, so if we found a stop > lowerZoom, back up by one.
-    if (minIt != s.stops.begin() && minIt->first > lowerZoom) {
+    if (minIt != s.stops.begin() && minIt != s.stops.end() && minIt->first > lowerZoom) {
         minIt--;
     }
     return Range<float> {
@@ -144,9 +144,10 @@ class ConstantSymbolSizeBinder final : public SymbolSizeBinder {
       : layoutSize(function_.evaluate(tileZoom + 1)) {
         function_.stops.match(
             [&] (const style::ExponentialStops<float>& stops) {
+                const auto& zoomLevels = getCoveringStops(stops, tileZoom, tileZoom + 1);
                 coveringRanges = std::make_tuple(
-                    getCoveringStops(stops, tileZoom, tileZoom + 1),
-                    Range<float> { function_.evaluate(tileZoom), function_.evaluate(tileZoom + 1) }
+                    zoomLevels,
+                    Range<float> { function_.evaluate(zoomLevels.min), function_.evaluate(zoomLevels.max) }
                 );
                 functionInterpolationBase = stops.base;
             },

test/programs/symbol_program.test.cpp

@@ -0,0 +1,61 @@
+#include <mbgl/test/util.hpp>
+
+#include <mbgl/programs/symbol_program.hpp>
+
+using namespace mbgl;
+
+TEST(SymbolProgram, SymbolSizeBinder) {
+    auto binder = SymbolSizeBinder::create(5.0f, 12.0f, 0.0f);
+    auto uniformValues = binder->uniformValues(5.5f);
+    EXPECT_EQ(uniformValues.get<uniforms::u_is_size_zoom_constant>().t, true);
+    EXPECT_EQ(uniformValues.get<uniforms::u_is_size_feature_constant>().t, true);
+    EXPECT_EQ(uniformValues.get<uniforms::u_size>().t, 12.0f);
+    EXPECT_EQ(uniformValues.get<uniforms::u_layout_size>().t, 12.0f);
+
+    binder = SymbolSizeBinder::create(1.0f, style::CameraFunction<float>(style::ExponentialStops<float>({
+        {0.0f, 8.0f},
+        {10.0f, 18.0f}
+    }, 1.0f)), 0.0f);
+    uniformValues = binder->uniformValues(1.5f);
+    EXPECT_EQ(uniformValues.get<uniforms::u_is_size_zoom_constant>().t, false);
+    EXPECT_EQ(uniformValues.get<uniforms::u_is_size_feature_constant>().t, true);
+    EXPECT_EQ(uniformValues.get<uniforms::u_size>().t, 9.5f);
+    EXPECT_EQ(uniformValues.get<uniforms::u_layout_size>().t, 10.0f);
+
+    binder = SymbolSizeBinder::create(0.0f, style::CameraFunction<float>(style::ExponentialStops<float>({
+        {1.0f, 8.0f},
+        {11.0f, 18.0f}
+    }, 1.0f)), 0.0f);
+    uniformValues = binder->uniformValues(0.5f);
+    EXPECT_EQ(uniformValues.get<uniforms::u_is_size_zoom_constant>().t, false);
+    EXPECT_EQ(uniformValues.get<uniforms::u_is_size_feature_constant>().t, true);
+    EXPECT_EQ(uniformValues.get<uniforms::u_size>().t, 8.0f);
+    EXPECT_EQ(uniformValues.get<uniforms::u_layout_size>().t, 8.0f);
+
+    binder = SymbolSizeBinder::create(12.0f, style::CameraFunction<float>(style::ExponentialStops<float>({
+        {1.0f, 8.0f},
+        {11.0f, 18.0f}
+    }, 1.0f)), 0.0f);
+    uniformValues = binder->uniformValues(12.5f);
+    EXPECT_EQ(uniformValues.get<uniforms::u_is_size_zoom_constant>().t, false);
+    EXPECT_EQ(uniformValues.get<uniforms::u_is_size_feature_constant>().t, true);
+    EXPECT_EQ(uniformValues.get<uniforms::u_size>().t, 18.0f);
+    EXPECT_EQ(uniformValues.get<uniforms::u_layout_size>().t, 18.0f);
+
+    binder = SymbolSizeBinder::create(0.0f, style::SourceFunction<float>("x", style::ExponentialStops<float>({
+        {1.0f, 8.0f},
+        {11.0f, 18.0f}
+    }, 1.0f)), 0.0f);
+    uniformValues = binder->uniformValues(12.5f);
+    EXPECT_EQ(uniformValues.get<uniforms::u_is_size_zoom_constant>().t, true);
+    EXPECT_EQ(uniformValues.get<uniforms::u_is_size_feature_constant>().t, false);
+
+    binder = SymbolSizeBinder::create(5.0f, style::CompositeFunction<float>("x", style::CompositeExponentialStops<float>({
+        {1.0f, {{0.0f, 8.0f}, {100.0f, 18.0f}}},
+        {11.0f, {{0.0f, 12.0f}, {100.0f, 24.9f}}}
+    }, 1.0f)), 0.0f);
+    uniformValues = binder->uniformValues(5.5f);
+    EXPECT_EQ(uniformValues.get<uniforms::u_is_size_zoom_constant>().t, false);
+    EXPECT_EQ(uniformValues.get<uniforms::u_is_size_feature_constant>().t, false);
+    EXPECT_EQ(uniformValues.get<uniforms::u_size_t>().t, 0.45f);
+}