Add guard blocks and checks to SymbolInstance (#2744)

include/mbgl/util/logging.hpp

@@ -38,8 +38,11 @@ class Log {
     Log();
     ~Log();
 
-    static void useLogThread(bool enable) noexcept;
-    static void useLogThread(bool enable, EventSeverity) noexcept;
+    /// @brief Determines whether messages of a given severity level are logged asynchronously.
+    ///
+    /// In a crash or other unexpected termination, pending asynchronous log entries will be lost.
+    /// The default is true (asynchronous) for all levels except `Error`.
+    static void useLogThread(bool enable, std::optional<EventSeverity> = {});
 
     template <typename... Args>
     static void Debug(Event event, Args&&... args) noexcept {

include/mbgl/util/string.hpp

@@ -78,9 +78,7 @@ inline std::string toString(long double t, bool decimal = false) {
     return toString(static_cast<double>(t), decimal);
 }
 
-inline std::string toString(std::thread::id threadId) {
-    return (std::ostringstream() << threadId).str();
-}
+std::string toString(const std::thread::id &);
 
 std::string toString(const std::exception_ptr &);
 

src/mbgl/layout/symbol_instance.cpp

@@ -1,5 +1,7 @@
 #include <mbgl/layout/symbol_instance.hpp>
 #include <mbgl/style/layers/symbol_layer_properties.hpp>
+#include <mbgl/util/logging.hpp>
+
 #include <utility>
 
 namespace mbgl {
@@ -219,4 +221,80 @@ std::optional<size_t> SymbolInstance::getDefaultHorizontalPlacedTextIndex() cons
     if (placedLeftTextIndex) return placedLeftTextIndex;
     return std::nullopt;
 }
+
+#if MLN_SYMBOL_GUARDS
+bool SymbolInstance::check(const std::source_location& source) const {
+    return !isFailed && check(check01, 1, source) && check(check02, 2, source) && check(check03, 3, source) &&
+           check(check04, 4, source) && check(check05, 5, source) && check(check06, 6, source) &&
+           check(check07, 7, source) && check(check08, 8, source) && check(check09, 9, source) &&
+           check(check10, 10, source) && check(check11, 11, source) && check(check12, 12, source) &&
+           check(check13, 13, source) && check(check14, 14, source) && check(check15, 15, source) &&
+           check(check16, 16, source) && check(check17, 17, source) && check(check18, 18, source) &&
+           check(check19, 19, source) && check(check20, 20, source) && check(check21, 21, source) &&
+           check(check22, 22, source) && check(check23, 23, source) && check(check24, 24, source) &&
+           check(check25, 25, source) && check(check26, 26, source) && check(check27, 27, source) &&
+           check(check28, 28, source) && checkKey(source);
+}
+
+bool SymbolInstance::checkIndexes(std::size_t textCount,
+                                  std::size_t iconSize,
+                                  std::size_t sdfSize,
+                                  const std::source_location& source) const {
+    return !isFailed && checkIndex(placedRightTextIndex, textCount, source) &&
+           checkIndex(placedCenterTextIndex, textCount, source) && checkIndex(placedLeftTextIndex, textCount, source) &&
+           checkIndex(placedVerticalTextIndex, textCount, source) &&
+           checkIndex(placedIconIndex, hasSdfIcon() ? sdfSize : iconSize, source) &&
+           checkIndex(placedVerticalIconIndex, hasSdfIcon() ? sdfSize : iconSize, source);
+}
+
+namespace {
+inline std::string locationSuffix(const std::source_location& source) {
+    return std::string(" from ") + source.function_name() + " (" + source.file_name() + ":" +
+           util::toString(source.line()) + ")";
+}
+} // namespace
+bool SymbolInstance::check(std::uint64_t v, int n, const std::source_location& source) const {
+    if (!isFailed && v != checkVal) {
+        isFailed = true;
+        Log::Error(Event::Crash,
+                   "SymbolInstance corrupted at " + util::toString(n) + " with value " + util::toString(v) +
+                       locationSuffix(source));
+    }
+    return !isFailed;
+}
+
+bool SymbolInstance::checkKey(const std::source_location& source) const {
+    if (!isFailed && key.size() > 10000) { // largest observed value=62
+        isFailed = true;
+        Log::Error(Event::Crash,
+                   "SymbolInstance key corrupted with size=" + util::toString(key.size()) + locationSuffix(source));
+    }
+    return !isFailed;
+}
+
+bool SymbolInstance::checkIndex(const std::optional<std::size_t>& index,
+                                std::size_t size,
+                                const std::source_location& source) const {
+    if (index.has_value() && *index >= size) {
+        isFailed = true;
+        Log::Error(Event::Crash,
+                   "SymbolInstance index corrupted with value=" + util::toString(*index) +
+                       " size=" + util::toString(size) + locationSuffix(source));
+    }
+    return !isFailed;
+}
+
+void SymbolInstance::forceFail() const {
+    isFailed = true;
+}
+
+// this is just to avoid warnings about the values never being set
+void SymbolInstance::forceFailInternal() {
+    check01 = check02 = check03 = check04 = check05 = check06 = check07 = check08 = check09 = check10 = check11 =
+        check12 = check13 = check14 = check15 = check16 = check17 = check18 = check19 = check20 = check21 = check22 =
+            check23 = check24 = check25 = check26 = check27 = check28 = 0;
+}
+
+#endif // MLN_SYMBOL_GUARDS
+
 } // namespace mbgl

src/mbgl/layout/symbol_instance.hpp

@@ -6,6 +6,49 @@
 #include <mbgl/style/layers/symbol_layer_properties.hpp>
 #include <mbgl/util/bitmask_operations.hpp>
 
+#include <source_location>
+
+#if !defined(MLN_SYMBOL_GUARDS)
+#define MLN_SYMBOL_GUARDS 1
+#endif
+
+#if MLN_SYMBOL_GUARDS
+#define SYM_GUARD_VALUE(N) std::uint64_t check##N = checkVal;
+#else
+#define SYM_GUARD_VALUE(N)
+#endif
+
+// A temporary shim for partial C++20 support
+#if MLN_SYMBOL_GUARDS
+#if defined(__clang__)
+#if __cplusplus <= 201703L || !__has_builtin(__builtin_source_location)
+namespace std {
+struct source_location {
+    const char* fileName_;
+    const char* functionName_;
+    unsigned line_;
+
+    constexpr uint_least32_t line() const noexcept { return line_; }
+    constexpr uint_least32_t column() const noexcept { return 0; }
+    constexpr const char* file_name() const noexcept { return fileName_; }
+    constexpr const char* function_name() const noexcept { return functionName_; }
+};
+} // namespace std
+#define SYM_GUARD_LOC                    \
+    std::source_location {               \
+        __FILE__, __FUNCTION__, __LINE__ \
+    }
+#else
+#define SYM_GUARD_LOC std::source_location::current()
+#endif
+#else
+#define SYM_GUARD_LOC std::source_location::current()
+#endif
+#else
+#define SYM_GUARD_LOC \
+    {}
+#endif
+
 namespace mbgl {
 
 class Anchor;
@@ -52,6 +95,9 @@ struct SymbolInstanceSharedData {
     std::optional<SymbolQuads> verticalIconQuads;
 };
 
+// With guards, clang complains about excessive padding here
+// NOLINTBEGIN(clang-analyzer-optin.performance.Padding)
+
 class SymbolInstance {
 public:
     SymbolInstance(Anchor& anchor_,
@@ -90,43 +136,149 @@ class SymbolInstance {
     const std::optional<SymbolQuads>& verticalIconQuads() const;
     void releaseSharedData();
 
+#if MLN_SYMBOL_GUARDS
+    /// Check all guard blocks
+    bool check(const std::source_location&) const;
+    /// Check that an index is in the valid range
+    bool checkIndex(const std::optional<std::size_t>& index, std::size_t size, const std::source_location&) const;
+    /// Check all indexes
+    bool checkIndexes(std::size_t textCount,
+                      std::size_t iconSize,
+                      std::size_t sdfSize,
+                      const std::source_location&) const;
+    /// Mark this item as failed (due to some external check) so that it cannot be used later
+    void forceFail() const;
+#else
+    bool check(std::string_view = {}) const { return true; }
+    bool checkIndex(const std::optional<std::size_t>&, std::size_t, std::string_view = {}) const { return true; }
+    bool checkIndexes(std::size_t, std::size_t, std::size_t, std::string_view = {}) const { return true; }
+    void forceFail() const {}
+#endif
+
+    const Anchor& getAnchor() const { return anchor; }
+    std::size_t getRightJustifiedGlyphQuadsSize() const { return rightJustifiedGlyphQuadsSize; }
+    std::size_t getCenterJustifiedGlyphQuadsSize() const { return centerJustifiedGlyphQuadsSize; }
+    std::size_t getLeftJustifiedGlyphQuadsSize() const { return leftJustifiedGlyphQuadsSize; }
+    std::size_t getVerticalGlyphQuadsSize() const { return verticalGlyphQuadsSize; }
+    std::size_t getIconQuadsSize() const { return iconQuadsSize; }
+    const CollisionFeature& getTextCollisionFeature() const { return textCollisionFeature; }
+    const CollisionFeature& getIconCollisionFeature() const { return iconCollisionFeature; }
+    const std::optional<CollisionFeature>& getVerticalTextCollisionFeature() const {
+        return verticalTextCollisionFeature;
+    }
+    const std::optional<CollisionFeature>& getVerticalIconCollisionFeature() const {
+        return verticalIconCollisionFeature;
+    }
+    WritingModeType getWritingModes() const { return writingModes; }
+    std::size_t getLayoutFeatureIndex() const { return layoutFeatureIndex; }
+    std::size_t getDataFeatureIndex() const { return dataFeatureIndex; }
+    std::array<float, 2> getTextOffset() const { return textOffset; }
+    std::array<float, 2> getIconOffset() const { return iconOffset; }
+    const std::u16string& getKey() const { return key; }
+    std::optional<size_t> getPlacedRightTextIndex() const { return placedRightTextIndex; }
+    std::optional<size_t> getPlacedCenterTextIndex() const { return placedCenterTextIndex; }
+    std::optional<size_t> getPlacedLeftTextIndex() const { return placedLeftTextIndex; }
+    std::optional<size_t> getPlacedVerticalTextIndex() const { return placedVerticalTextIndex; }
+    std::optional<size_t> getPlacedIconIndex() const { return placedIconIndex; }
+    std::optional<size_t> getPlacedVerticalIconIndex() const { return placedVerticalIconIndex; }
+    float getTextBoxScale() const { return textBoxScale; }
+    std::array<float, 2> getVariableTextOffset() const { return variableTextOffset; }
+    bool getSingleLine() const { return singleLine; }
+
+    uint32_t getCrossTileID() const { return crossTileID; }
+    void setCrossTileID(uint32_t x) { crossTileID = x; }
+
+    std::optional<size_t>& refPlacedRightTextIndex() { return placedRightTextIndex; }
+    std::optional<size_t>& refPlacedCenterTextIndex() { return placedCenterTextIndex; }
+    std::optional<size_t>& refPlacedLeftTextIndex() { return placedLeftTextIndex; }
+    std::optional<size_t>& refPlacedVerticalTextIndex() { return placedVerticalTextIndex; }
+    std::optional<size_t>& refPlacedIconIndex() { return placedIconIndex; }
+    std::optional<size_t>& refPlacedVerticalIconIndex() { return placedVerticalIconIndex; }
+
+    void setPlacedRightTextIndex(std::optional<size_t> x) { placedRightTextIndex = x; }
+    void setPlacedCenterTextIndex(std::optional<size_t> x) { placedCenterTextIndex = x; }
+    void setPlacedLeftTextIndex(std::optional<size_t> x) { placedLeftTextIndex = x; }
+
+    static constexpr uint32_t invalidCrossTileID = std::numeric_limits<uint32_t>::max();
+
+protected:
+#if MLN_SYMBOL_GUARDS
+    bool check(std::uint64_t v, int n, const std::source_location&) const;
+    bool checkKey(const std::source_location&) const;
+    void forceFailInternal(); // this is just to avoid warnings about the values never being set
+#else
+    bool checkKey(std::string_view) const { return true; }
+#endif
+
 private:
     std::shared_ptr<SymbolInstanceSharedData> sharedData;
 
-public:
+    static constexpr std::uint64_t checkVal = 0x123456780ABCDEFFULL;
+
+    SYM_GUARD_VALUE(01)
     Anchor anchor;
+    SYM_GUARD_VALUE(02)
     SymbolContent symbolContent;
+    SYM_GUARD_VALUE(03)
 
     std::size_t rightJustifiedGlyphQuadsSize;
+    SYM_GUARD_VALUE(04)
     std::size_t centerJustifiedGlyphQuadsSize;
+    SYM_GUARD_VALUE(05)
     std::size_t leftJustifiedGlyphQuadsSize;
+    SYM_GUARD_VALUE(06)
     std::size_t verticalGlyphQuadsSize;
+    SYM_GUARD_VALUE(07)
     std::size_t iconQuadsSize;
+    SYM_GUARD_VALUE(08)
 
     CollisionFeature textCollisionFeature;
+    SYM_GUARD_VALUE(09)
     CollisionFeature iconCollisionFeature;
+    SYM_GUARD_VALUE(10)
     std::optional<CollisionFeature> verticalTextCollisionFeature = std::nullopt;
+    SYM_GUARD_VALUE(11)
     std::optional<CollisionFeature> verticalIconCollisionFeature = std::nullopt;
+    SYM_GUARD_VALUE(12)
     WritingModeType writingModes;
+    SYM_GUARD_VALUE(13)
     std::size_t layoutFeatureIndex; // Index into the set of features included at layout time
-    std::size_t dataFeatureIndex;   // Index into the underlying tile data feature set
+    SYM_GUARD_VALUE(14)
+    std::size_t dataFeatureIndex; // Index into the underlying tile data feature set
+    SYM_GUARD_VALUE(15)
     std::array<float, 2> textOffset;
+    SYM_GUARD_VALUE(16)
     std::array<float, 2> iconOffset;
+    SYM_GUARD_VALUE(17)
     std::u16string key;
+    SYM_GUARD_VALUE(18)
     std::optional<size_t> placedRightTextIndex;
+    SYM_GUARD_VALUE(19)
     std::optional<size_t> placedCenterTextIndex;
+    SYM_GUARD_VALUE(20)
     std::optional<size_t> placedLeftTextIndex;
+    SYM_GUARD_VALUE(21)
     std::optional<size_t> placedVerticalTextIndex;
+    SYM_GUARD_VALUE(22)
     std::optional<size_t> placedIconIndex;
+    SYM_GUARD_VALUE(23)
     std::optional<size_t> placedVerticalIconIndex;
+    SYM_GUARD_VALUE(24)
     float textBoxScale;
+    SYM_GUARD_VALUE(25)
     std::array<float, 2> variableTextOffset;
+    SYM_GUARD_VALUE(26)
     bool singleLine;
+    SYM_GUARD_VALUE(27)
     uint32_t crossTileID = 0;
-
-    static constexpr uint32_t invalidCrossTileID() { return std::numeric_limits<uint32_t>::max(); }
+    SYM_GUARD_VALUE(28)
+#if MLN_SYMBOL_GUARDS
+    mutable bool isFailed = false;
+#endif
 };
 
+// NOLINTEND(clang-analyzer-optin.performance.Padding)
+
 using SymbolInstanceReferences = std::vector<std::reference_wrapper<const SymbolInstance>>;
 
 } // namespace mbgl