common/Png.hpp | 66 +++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 23 deletions(-)
New commits: commit f7bd2c7e1df5c2f1e17ae2b8e1658f9013704648 Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk> AuthorDate: Sat Sep 21 21:10:18 2019 -0400 Commit: Ashod Nakashian <ashnak...@gmail.com> CommitDate: Sun Sep 22 20:24:36 2019 +0200 wsd: fix variable might be clobbered by ‘longjmp’ or ‘vfork’ As the comment details, this avoids having C++ objects in the same frame as setjmp, which may reset their contents without the dtor getting called. Change-Id: I851ae8bffb4356d465a25dfc815a1fecb489fa30 Reviewed-on: https://gerrit.libreoffice.org/79338 Reviewed-by: Ashod Nakashian <ashnak...@gmail.com> Tested-by: Ashod Nakashian <ashnak...@gmail.com> diff --git a/common/Png.hpp b/common/Png.hpp index d4e1c2cf5..d629158c2 100644 --- a/common/Png.hpp +++ b/common/Png.hpp @@ -111,13 +111,17 @@ unpremultiply_data (png_structp /*png*/, png_row_infop row_info, png_bytep data) } } -// Sadly, older libpng headers don't use const for the pixmap pointer parameter to -// png_write_row(), so can't use const here for pixmap. -inline -bool encodeSubBufferToPNG(unsigned char* pixmap, size_t startX, size_t startY, - int width, int height, - int bufferWidth, int bufferHeight, - std::vector<char>& output, LibreOfficeKitTileMode mode) +/// This function uses setjmp which may clobbers non-trivial objects. +/// So we can't use logging or create complex C++ objects in this frame. +/// Specifically, logging uses std::string objects, and GCC gives the following: +/// error: variable ‘__capacity’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered] +/// In practice, this is bogus, since __capacity is has a trivial type, but this error +/// shows up with the sanitizers. But technically we shouldn't mix C++ objects with setjmp. +/// Sadly, older libpng headers don't use const for the pixmap pointer parameter to +/// png_write_row(), so can't use const here for pixmap. +inline bool impl_encodeSubBufferToPNG(unsigned char* pixmap, size_t startX, size_t startY, + int width, int height, int bufferWidth, int bufferHeight, + std::vector<char>& output, LibreOfficeKitTileMode mode) { if (bufferWidth < width || bufferHeight < height) { @@ -148,7 +152,8 @@ bool encodeSubBufferToPNG(unsigned char* pixmap, size_t startX, size_t startY, auto initialSize = output.size(); #endif - png_set_IHDR(png_ptr, info_ptr, width, height, 8, PNG_COLOR_TYPE_RGB_ALPHA, PNG_INTERLACE_NONE, PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT); + png_set_IHDR(png_ptr, info_ptr, width, height, 8, PNG_COLOR_TYPE_RGB_ALPHA, PNG_INTERLACE_NONE, + PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT); png_set_write_fn(png_ptr, &output, user_write_fn, user_flush_fn); png_set_write_status_fn(png_ptr, user_write_status_fn); @@ -160,8 +165,6 @@ bool encodeSubBufferToPNG(unsigned char* pixmap, size_t startX, size_t startY, png_set_write_user_transform_fn (png_ptr, unpremultiply_data); } - auto a = std::chrono::steady_clock::now(); - for (int y = 0; y < height; ++y) { size_t position = ((startY + y) * bufferWidth * 4) + (startX * 4); @@ -170,19 +173,6 @@ bool encodeSubBufferToPNG(unsigned char* pixmap, size_t startX, size_t startY, png_write_end(png_ptr, info_ptr); - auto b = std::chrono::steady_clock::now(); - - std::chrono::milliseconds::rep duration = std::chrono::duration_cast<std::chrono::milliseconds>(b-a).count(); - - static std::chrono::milliseconds::rep totalDuration = 0; - static int nCalls = 0; - - totalDuration += duration; - nCalls += 1; - LOG_TRC("PNG compression took " << duration << " ms (" << output.size() - << " bytes). Average after " << std::to_string(nCalls) - << " calls: " << (totalDuration / static_cast<double>(nCalls))); - png_destroy_write_struct(&png_ptr, &info_ptr); #ifdef IOS @@ -198,6 +188,36 @@ bool encodeSubBufferToPNG(unsigned char* pixmap, size_t startX, size_t startY, return true; } +/// Sadly, older libpng headers don't use const for the pixmap pointer parameter to +/// png_write_row(), so can't use const here for pixmap. +inline bool encodeSubBufferToPNG(unsigned char* pixmap, size_t startX, size_t startY, int width, + int height, int bufferWidth, int bufferHeight, + std::vector<char>& output, LibreOfficeKitTileMode mode) +{ + const auto start = std::chrono::steady_clock::now(); + + const bool res = impl_encodeSubBufferToPNG(pixmap, startX, startY, width, height, bufferWidth, + bufferHeight, output, mode); + if (Log::traceEnabled()) + { + const auto end = std::chrono::steady_clock::now(); + + std::chrono::milliseconds::rep duration + = std::chrono::duration_cast<std::chrono::milliseconds>(end - start).count(); + + static std::chrono::milliseconds::rep totalDuration = 0; + static int nCalls = 0; + + totalDuration += duration; + ++nCalls; + LOG_TRC("PNG compression took " + << duration << " ms (" << output.size() << " bytes). Average after " << nCalls + << " calls: " << (totalDuration / static_cast<double>(nCalls))); + } + + return res; +} + inline bool encodeBufferToPNG(unsigned char* pixmap, int width, int height, std::vector<char>& output, LibreOfficeKitTileMode mode) _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits