Trade: whitelist (exported) growable array deleters in imported data.

src/Magnum/Trade/AbstractImporter.cpp

@@ -35,6 +35,7 @@
 #include "Magnum/FileCallback.h"
 #include "Magnum/Trade/AbstractMaterialData.h"
 #include "Magnum/Trade/AnimationData.h"
+#include "Magnum/Trade/ArrayAllocator.h"
 #include "Magnum/Trade/CameraData.h"
 #include "Magnum/Trade/ImageData.h"
 #include "Magnum/Trade/LightData.h"
@@ -269,7 +270,7 @@ Containers::Optional<AnimationData> AbstractImporter::animation(const UnsignedIn
     CORRADE_ASSERT(id < doAnimationCount(), "Trade::AbstractImporter::animation(): index" << id << "out of range for" << doAnimationCount() << "entries", {});
     Containers::Optional<AnimationData> animation = doAnimation(id);
     CORRADE_ASSERT(!animation ||
-        ((!animation->_data.deleter() || animation->_data.deleter() == Implementation::nonOwnedArrayDeleter) &&
+        ((!animation->_data.deleter() || animation->_data.deleter() == Implementation::nonOwnedArrayDeleter || animation->_data.deleter() == ArrayAllocator<char>::deleter) &&
         (!animation->_tracks.deleter() || animation->_tracks.deleter() == reinterpret_cast<void(*)(AnimationTrackData*, std::size_t)>(Implementation::nonOwnedArrayDeleter))),
         "Trade::AbstractImporter::animation(): implementation is not allowed to use a custom Array deleter", {});
     return animation;
@@ -434,8 +435,8 @@ Containers::Optional<MeshData> AbstractImporter::mesh(const UnsignedInt id) {
     CORRADE_ASSERT(id < doMeshCount(), "Trade::AbstractImporter::mesh(): index" << id << "out of range for" << doMeshCount() << "entries", {});
     Containers::Optional<MeshData> mesh = doMesh(id);
     CORRADE_ASSERT(!mesh || (
-        (!mesh->_indexData.deleter() || mesh->_indexData.deleter() == Implementation::nonOwnedArrayDeleter) &&
+        (!mesh->_indexData.deleter() || mesh->_indexData.deleter() == Implementation::nonOwnedArrayDeleter || mesh->_indexData.deleter() == ArrayAllocator<char>::deleter) &&
-        (!mesh->_vertexData.deleter() || mesh->_vertexData.deleter() == Implementation::nonOwnedArrayDeleter) &&
+        (!mesh->_vertexData.deleter() || mesh->_vertexData.deleter() == Implementation::nonOwnedArrayDeleter || mesh->_vertexData.deleter() == ArrayAllocator<char>::deleter) &&
         (!mesh->_attributes.deleter() || mesh->_attributes.deleter() == reinterpret_cast<void(*)(MeshAttributeData*, std::size_t)>(Implementation::nonOwnedArrayDeleter))),
         "Trade::AbstractImporter::mesh(): implementation is not allowed to use a custom Array deleter", {});
     return mesh;
@@ -640,7 +641,7 @@ Containers::Optional<ImageData1D> AbstractImporter::image1D(const UnsignedInt id
     }
     #endif
     Containers::Optional<ImageData1D> image = doImage1D(id, level);
-    CORRADE_ASSERT(!image || !image->_data.deleter() || image->_data.deleter() == Implementation::nonOwnedArrayDeleter, "Trade::AbstractImporter::image1D(): implementation is not allowed to use a custom Array deleter", {});
+    CORRADE_ASSERT(!image || !image->_data.deleter() || image->_data.deleter() == Implementation::nonOwnedArrayDeleter || image->_data.deleter() == ArrayAllocator<char>::deleter, "Trade::AbstractImporter::image1D(): implementation is not allowed to use a custom Array deleter", {});
     return image;
 }
 
@@ -696,7 +697,7 @@ Containers::Optional<ImageData2D> AbstractImporter::image2D(const UnsignedInt id
     }
     #endif
     Containers::Optional<ImageData2D> image = doImage2D(id, level);
-    CORRADE_ASSERT(!image || !image->_data.deleter() || image->_data.deleter() == Implementation::nonOwnedArrayDeleter, "Trade::AbstractImporter::image2D(): implementation is not allowed to use a custom Array deleter", {});
+    CORRADE_ASSERT(!image || !image->_data.deleter() || image->_data.deleter() == Implementation::nonOwnedArrayDeleter || image->_data.deleter() == ArrayAllocator<char>::deleter, "Trade::AbstractImporter::image2D(): implementation is not allowed to use a custom Array deleter", {});
     return image;
 }
 
@@ -752,7 +753,7 @@ Containers::Optional<ImageData3D> AbstractImporter::image3D(const UnsignedInt id
     }
     #endif
     Containers::Optional<ImageData3D> image = doImage3D(id, level);
-    CORRADE_ASSERT(!image || !image->_data.deleter() || image->_data.deleter() == Implementation::nonOwnedArrayDeleter, "Trade::AbstractImporter::image3D(): implementation is not allowed to use a custom Array deleter", {});
+    CORRADE_ASSERT(!image || !image->_data.deleter() || image->_data.deleter() == Implementation::nonOwnedArrayDeleter || image->_data.deleter() == ArrayAllocator<char>::deleter, "Trade::AbstractImporter::image3D(): implementation is not allowed to use a custom Array deleter", {});
     return image;
 }
 

src/Magnum/Trade/AbstractImporter.h

@@ -255,9 +255,10 @@ checked by the implementation:
     As @ref Trade-AbstractImporter-data-dependency "mentioned above",
     @ref Corrade::Containers::Array instances returned from plugin
     implementations are not allowed to use anything else than the default
-    deleter, otherwise this could cause dangling function pointer call on array
+    deleter or the deleter used by @ref Trade::ArrayAllocator, otherwise this
-    destruction if the plugin gets unloaded before the array is destroyed. This
+    could cause dangling function pointer call on array destruction if the
-    is asserted by the base implementation on return.
+    plugin gets unloaded before the array is destroyed. This is asserted by the
+    base implementation on return.
 @par
     Similarly for interpolator functions passed through
     @ref Animation::TrackView instances to @ref AnimationData --- to avoid

src/Magnum/Trade/ArrayAllocator.cpp

@@ -0,0 +1,34 @@
+/*
+    This file is part of Magnum.
+
+    Copyright © 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019
+              Vladimír Vondruš <mosra@centrum.cz>
+
+    Permission is hereby granted, free of charge, to any person obtaining a
+    copy of this software and associated documentation files (the "Software"),
+    to deal in the Software without restriction, including without limitation
+    the rights to use, copy, modify, merge, publish, distribute, sublicense,
+    and/or sell copies of the Software, and to permit persons to whom the
+    Software is furnished to do so, subject to the following conditions:
+
+    The above copyright notice and this permission notice shall be included
+    in all copies or substantial portions of the Software.
+
+    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+    THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+    FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+    DEALINGS IN THE SOFTWARE.
+*/
+
+#include "ArrayAllocator.h"
+
+namespace Magnum { namespace Trade {
+
+void ArrayAllocator<char>::deleter(char* const data, std::size_t) {
+    deallocate(data);
+}
+
+}}

src/Magnum/Trade/ArrayAllocator.h

@@ -0,0 +1,60 @@
+#ifndef Magnum_Trade_ArrayAllocator_h
+#define Magnum_Trade_ArrayAllocator_h
+/*
+    This file is part of Magnum.
+
+    Copyright © 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019
+              Vladimír Vondruš <mosra@centrum.cz>
+
+    Permission is hereby granted, free of charge, to any person obtaining a
+    copy of this software and associated documentation files (the "Software"),
+    to deal in the Software without restriction, including without limitation
+    the rights to use, copy, modify, merge, publish, distribute, sublicense,
+    and/or sell copies of the Software, and to permit persons to whom the
+    Software is furnished to do so, subject to the following conditions:
+
+    The above copyright notice and this permission notice shall be included
+    in all copies or substantial portions of the Software.
+
+    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+    THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+    FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+    DEALINGS IN THE SOFTWARE.
+*/
+
+/** @file
+ * @brief Class @ref Magnum::Trade::ArrayAllocator
+ * @m_since_latest
+ */
+
+#include <Corrade/Containers/GrowableArray.h>
+
+#include "Magnum/Magnum.h"
+#include "Magnum/Trade/visibility.h"
+
+namespace Magnum { namespace Trade {
+
+/**
+@brief Growable array allocator to be used in importer plugins
+@m_since_latest
+
+Compared to @ref Corrade::Containers::ArrayMallocAllocator ensures that the
+@ref Array deleter function pointer is defined in the @ref Trade library and
+not in the plugin binary itself, avoiding dangling function pointer call when
+the data array is destructed after the plugin has been unloaded. Other than
+that the behavior is identical.
+*/
+template<class T> struct ArrayAllocator: Containers::ArrayMallocAllocator<T> {};
+
+#ifndef DOXYGEN_GENERATING_OUTPUT
+template<> struct ArrayAllocator<char>: Containers::ArrayMallocAllocator<char> {
+    MAGNUM_TRADE_EXPORT static void deleter(char* data, std::size_t size);
+};
+#endif
+
+}}
+
+#endif

src/Magnum/Trade/CMakeLists.txt

@@ -27,6 +27,7 @@ find_package(Corrade REQUIRED PluginManager)
 
 set(MagnumTrade_SRCS
     AbstractMaterialData.cpp
+    ArrayAllocator.cpp
     Data.cpp
     LightData.cpp
     MeshData2D.cpp
@@ -52,6 +53,7 @@ set(MagnumTrade_HEADERS
     AbstractImageConverter.h
     AbstractMaterialData.h
     AnimationData.h
+    ArrayAllocator.h
     CameraData.h
     Data.h
     ImageData.h

src/Magnum/Trade/Test/AbstractImporterTest.cpp

@@ -34,6 +34,7 @@
 #include "Magnum/FileCallback.h"
 #include "Magnum/Trade/AbstractImporter.h"
 #include "Magnum/Trade/AnimationData.h"
+#include "Magnum/Trade/ArrayAllocator.h"
 #include "Magnum/Trade/CameraData.h"
 #include "Magnum/Trade/ImageData.h"
 #include "Magnum/Trade/LightData.h"
@@ -107,6 +108,7 @@ struct AbstractImporterTest: TestSuite::Tester {
     void animationNoFile();
     void animationOutOfRange();
     void animationNonOwningDeleters();
+    void animationGrowableDeleters();
     void animationCustomDataDeleter();
     void animationCustomTrackDeleter();
 
@@ -170,6 +172,7 @@ struct AbstractImporterTest: TestSuite::Tester {
     void meshNoFile();
     void meshOutOfRange();
     void meshNonOwningDeleters();
+    void meshGrowableDeleters();
     void meshCustomIndexDataDeleter();
     void meshCustomVertexDataDeleter();
     void meshCustomAttributesDeleter();
@@ -243,6 +246,7 @@ struct AbstractImporterTest: TestSuite::Tester {
     void image1DOutOfRange();
     void image1DLevelOutOfRange();
     void image1DNonOwningDeleter();
+    void image1DGrowableDeleter();
     void image1DCustomDeleter();
 
     void image2D();
@@ -262,6 +266,7 @@ struct AbstractImporterTest: TestSuite::Tester {
     void image2DOutOfRange();
     void image2DLevelOutOfRange();
     void image2DNonOwningDeleter();
+    void image2DGrowableDeleter();
     void image2DCustomDeleter();
 
     void image3D();
@@ -281,6 +286,7 @@ struct AbstractImporterTest: TestSuite::Tester {
     void image3DOutOfRange();
     void image3DLevelOutOfRange();
     void image3DNonOwningDeleter();
+    void image3DGrowableDeleter();
     void image3DCustomDeleter();
 
     void importerState();
@@ -346,6 +352,7 @@ AbstractImporterTest::AbstractImporterTest() {
               &AbstractImporterTest::animationNoFile,
               &AbstractImporterTest::animationOutOfRange,
               &AbstractImporterTest::animationNonOwningDeleters,
+              &AbstractImporterTest::animationGrowableDeleters,
               &AbstractImporterTest::animationCustomDataDeleter,
               &AbstractImporterTest::animationCustomTrackDeleter,
 
@@ -409,6 +416,7 @@ AbstractImporterTest::AbstractImporterTest() {
               &AbstractImporterTest::meshNoFile,
               &AbstractImporterTest::meshOutOfRange,
               &AbstractImporterTest::meshNonOwningDeleters,
+              &AbstractImporterTest::meshGrowableDeleters,
               &AbstractImporterTest::meshCustomIndexDataDeleter,
               &AbstractImporterTest::meshCustomVertexDataDeleter,
               &AbstractImporterTest::meshCustomAttributesDeleter,
@@ -482,6 +490,7 @@ AbstractImporterTest::AbstractImporterTest() {
               &AbstractImporterTest::image1DOutOfRange,
               &AbstractImporterTest::image1DLevelOutOfRange,
               &AbstractImporterTest::image1DNonOwningDeleter,
+              &AbstractImporterTest::image1DGrowableDeleter,
               &AbstractImporterTest::image1DCustomDeleter,
 
               &AbstractImporterTest::image2D,
@@ -501,6 +510,7 @@ AbstractImporterTest::AbstractImporterTest() {
               &AbstractImporterTest::image2DOutOfRange,
               &AbstractImporterTest::image2DLevelOutOfRange,
               &AbstractImporterTest::image2DNonOwningDeleter,
+              &AbstractImporterTest::image2DGrowableDeleter,
               &AbstractImporterTest::image2DCustomDeleter,
 
               &AbstractImporterTest::image3D,
@@ -520,6 +530,7 @@ AbstractImporterTest::AbstractImporterTest() {
               &AbstractImporterTest::image3DOutOfRange,
               &AbstractImporterTest::image3DLevelOutOfRange,
               &AbstractImporterTest::image3DNonOwningDeleter,
+              &AbstractImporterTest::image3DGrowableDeleter,
               &AbstractImporterTest::image3DCustomDeleter,
 
               &AbstractImporterTest::importerState,
@@ -1405,6 +1416,25 @@ void AbstractImporterTest::animationNonOwningDeleters() {
     CORRADE_COMPARE(static_cast<const void*>(data->data()), importer.data);
 }
 
+void AbstractImporterTest::animationGrowableDeleters() {
+    struct: AbstractImporter {
+        ImporterFeatures doFeatures() const override { return {}; }
+        bool doIsOpened() const override { return true; }
+        void doClose() override {}
+
+        UnsignedInt doAnimationCount() const override { return 1; }
+        Containers::Optional<AnimationData> doAnimation(UnsignedInt) override {
+            Containers::Array<char> data;
+            Containers::arrayAppend<ArrayAllocator>(data, '\x37');
+            return AnimationData{std::move(data), {AnimationTrackData{}}};
+        }
+    } importer;
+
+    auto data = importer.animation(0);
+    CORRADE_VERIFY(data);
+    CORRADE_COMPARE(data->data()[0], '\x37');
+}
+
 void AbstractImporterTest::animationCustomDataDeleter() {
     struct: AbstractImporter {
         ImporterFeatures doFeatures() const override { return {}; }
@@ -2300,6 +2330,33 @@ void AbstractImporterTest::meshNonOwningDeleters() {
     CORRADE_COMPARE(static_cast<const void*>(data->indexData()), importer.indexData);
 }
 
+void AbstractImporterTest::meshGrowableDeleters() {
+    struct: AbstractImporter {
+        ImporterFeatures doFeatures() const override { return {}; }
+        bool doIsOpened() const override { return true; }
+        void doClose() override {}
+
+        UnsignedInt doMeshCount() const override { return 1; }
+        Containers::Optional<MeshData> doMesh(UnsignedInt) override {
+            Containers::Array<char> indexData;
+            Containers::arrayAppend<ArrayAllocator>(indexData, '\xab');
+            Containers::Array<Vector3> vertexData;
+            Containers::arrayAppend<ArrayAllocator>(vertexData, Vector3{});
+            MeshIndexData indices{MeshIndexType::UnsignedByte, indexData};
+            MeshAttributeData positions{MeshAttribute::Position, Containers::arrayView(vertexData)};
+
+            return MeshData{MeshPrimitive::Triangles,
+                std::move(indexData), indices,
+                Containers::arrayAllocatorCast<char, ArrayAllocator>(std::move(vertexData)), {positions}};
+        }
+    } importer;
+
+    auto data = importer.mesh(0);
+    CORRADE_VERIFY(data);
+    CORRADE_COMPARE(data->indexData()[0], '\xab');
+    CORRADE_COMPARE(data->vertexData().size(), 12);
+}
+
 void AbstractImporterTest::meshCustomIndexDataDeleter() {
     struct: AbstractImporter {
         ImporterFeatures doFeatures() const override { return {}; }
@@ -3344,6 +3401,25 @@ void AbstractImporterTest::image1DNonOwningDeleter() {
     CORRADE_COMPARE(static_cast<const void*>(data->data()), importer.data);
 }
 
+void AbstractImporterTest::image1DGrowableDeleter() {
+    struct: AbstractImporter {
+        ImporterFeatures doFeatures() const override { return {}; }
+        bool doIsOpened() const override { return true; }
+        void doClose() override {}
+
+        UnsignedInt doImage1DCount() const override { return 1; }
+        Containers::Optional<ImageData1D> doImage1D(UnsignedInt, UnsignedInt) override {
+            Containers::Array<char> data;
+            Containers::arrayAppend<ArrayAllocator>(data, '\xff');
+            return ImageData1D{PixelFormat::RGBA8Unorm, {}, std::move(data)};
+        }
+    } importer;
+
+    auto data = importer.image1D(0);
+    CORRADE_VERIFY(data);
+    CORRADE_COMPARE(data->data()[0], '\xff');
+}
+
 void AbstractImporterTest::image1DCustomDeleter() {
     struct: AbstractImporter {
         ImporterFeatures doFeatures() const override { return {}; }
@@ -3630,6 +3706,25 @@ void AbstractImporterTest::image2DNonOwningDeleter() {
     CORRADE_COMPARE(static_cast<const void*>(data->data()), importer.data);
 }
 
+void AbstractImporterTest::image2DGrowableDeleter() {
+    struct: AbstractImporter {
+        ImporterFeatures doFeatures() const override { return {}; }
+        bool doIsOpened() const override { return true; }
+        void doClose() override {}
+
+        UnsignedInt doImage2DCount() const override { return 1; }
+        Containers::Optional<ImageData2D> doImage2D(UnsignedInt, UnsignedInt) override {
+            Containers::Array<char> data;
+            Containers::arrayAppend<ArrayAllocator>(data, '\xff');
+            return ImageData2D{PixelFormat::RGBA8Unorm, {}, std::move(data)};
+        }
+    } importer;
+
+    auto data = importer.image2D(0);
+    CORRADE_VERIFY(data);
+    CORRADE_COMPARE(data->data()[0], '\xff');
+}
+
 void AbstractImporterTest::image2DCustomDeleter() {
     struct: AbstractImporter {
         ImporterFeatures doFeatures() const override { return {}; }
@@ -3917,6 +4012,25 @@ void AbstractImporterTest::image3DNonOwningDeleter() {
     CORRADE_COMPARE(static_cast<const void*>(data->data()), importer.data);
 }
 
+void AbstractImporterTest::image3DGrowableDeleter() {
+    struct: AbstractImporter {
+        ImporterFeatures doFeatures() const override { return {}; }
+        bool doIsOpened() const override { return true; }
+        void doClose() override {}
+
+        UnsignedInt doImage3DCount() const override { return 1; }
+        Containers::Optional<ImageData3D> doImage3D(UnsignedInt, UnsignedInt) override {
+            Containers::Array<char> data;
+            Containers::arrayAppend<ArrayAllocator>(data, '\xff');
+            return ImageData3D{PixelFormat::RGBA8Unorm, {}, std::move(data)};
+        }
+    } importer;
+
+    auto data = importer.image3D(0);
+    CORRADE_VERIFY(data);
+    CORRADE_COMPARE(data->data()[0], '\xff');
+}
+
 void AbstractImporterTest::image3DCustomDeleter() {
     struct: AbstractImporter {
         ImporterFeatures doFeatures() const override { return {}; }