Fixed error handling in dip::ImageReadJPEG(), `dip::ImageWriteJPEG(…

…)`, etc. Fixes #80.

CMakeLists.txt

@@ -43,6 +43,7 @@ if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") # also matchs "AppleClang"
    # Compiler flags for Clang C++
    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wconversion -Wsign-conversion -pedantic -Wno-c++17-extensions")
    #set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -march=native") # This is optimal for local usage.
+   set(CMAKE_C_FLAGS_SANITIZE "${CMAKE_C_FLAGS_DEBUG} -fsanitize=address")
    set(CMAKE_CXX_FLAGS_SANITIZE "${CMAKE_CXX_FLAGS_DEBUG} -fsanitize=address")
 elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
    # Compiler flags for GNU C++
@@ -53,7 +54,9 @@ elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
    # "DIP_EXPORT" in forward class declaration sometimes causes a warning in GCC 6.0 and 7.0.
    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-attributes")
    #set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -march=native") # This is optimal for local usage; to see which flags are enabled: gcc -march=native -Q --help=target
+   set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -Og") # Does some optimization that doesn't impact debugging.
    set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -Og") # Does some optimization that doesn't impact debugging.
+   set(CMAKE_C_FLAGS_SANITIZE "${CMAKE_C_FLAGS_DEBUG} -fsanitize=address")
    set(CMAKE_CXX_FLAGS_SANITIZE "${CMAKE_CXX_FLAGS_DEBUG} -fsanitize=address")
 elseif(CMAKE_CXX_COMPILER_ID STREQUAL "Intel")
    # Compiler flags for Intel C++

changelogs/diplib_3.1.0.md

@@ -102,6 +102,8 @@ title: "Changes DIPlib 3.1.0"
 
 - libics had a typo that caused out-of-bounds read (#81). 
 
+- Fixed error handling in `dip::ImageReadJPEG()` and `dip::ImageWriteJPEG()`, which previously would crash when libjpeg produced an error (#80).
+
 
 
 

src/file_io/jpeg.cpp

@@ -24,31 +24,40 @@
 #include "diplib/file_io.h"
 
 #include "jpeglib.h"
-#include <setjmp.h>
-
-namespace dip {
-
-namespace {
+#include <csetjmp>
 
 // JPEG error handling stuff - modified from example.c in libjpeg source
+extern "C" {
+   static void my_error_exit( j_common_ptr cinfo );
+   static void my_output_message( j_common_ptr );
+}
+
 struct my_error_mgr {
-   struct jpeg_error_mgr pub;   // "public" fields
-   jmp_buf setjmp_buffer;      // for return to caller
+   struct jpeg_error_mgr pub; // "public" fields
+   std::jmp_buf setjmp_buffer;     // for return to caller
 };
 using my_error_ptr = struct my_error_mgr*;
 
-void my_error_exit( j_common_ptr cinfo ) {
+static void my_error_exit( j_common_ptr cinfo ) {
    // cinfo->err really points to a my_error_mgr struct, so coerce pointer
-   my_error_ptr myerr = reinterpret_cast<my_error_ptr>(cinfo->err);
+   my_error_ptr myerr = reinterpret_cast<my_error_ptr>( cinfo->err );
    // Return control to the setjmp point
    longjmp( myerr->setjmp_buffer, 1 );
 }
 
-void my_output_message( j_common_ptr ) {} // Don't do anything with messages!
+static void my_output_message( j_common_ptr ) {} // Don't do anything with messages!
+
+#define DIP__DECLARE_JPEG_EXIT( message ) \
+std::jmp_buf setjmp_buffer; if( setjmp( setjmp_buffer )) { DIP_THROW_RUNTIME( message ); }
+
+
+namespace dip {
+
+namespace {
 
 class JpegInput {
    public:
-      JpegInput( String filename ) : filename_( std::move( filename )) {
+      JpegInput( String filename, std::jmp_buf const& setjmp_buffer ) : filename_( std::move( filename )) {
          infile_ = std::fopen( filename_.c_str(), "rb" );
          if( infile_ == nullptr ) {
             if( !FileHasExtension( filename_ )) {
@@ -66,10 +75,7 @@ class JpegInput {
          cinfo_.err = jpeg_std_error( &jerr_.pub );
          jerr_.pub.error_exit = my_error_exit;
          jerr_.pub.output_message = my_output_message;
-         if( setjmp( jerr_.setjmp_buffer )) {
-            // If we get here, the JPEG code has signaled an error.
-            DIP_THROW_RUNTIME( "Error reading JPEG file." );
-         }
+         std::memcpy( jerr_.setjmp_buffer, setjmp_buffer, sizeof( setjmp_buffer ));
          jpeg_create_decompress( &cinfo_ );
          initialized_ = true;
          jpeg_stdio_src( &cinfo_, infile_ );
@@ -102,7 +108,7 @@ class JpegInput {
 
 class JpegOutput {
    public:
-      explicit JpegOutput( String const& filename ) {
+      explicit JpegOutput( String const& filename, std::jmp_buf const& setjmp_buffer ) {
          // Open the file for writing
          if( FileHasExtension( filename )) {
             outfile_ = std::fopen(filename.c_str(), "wb");
@@ -115,10 +121,7 @@ class JpegOutput {
          cinfo_.err = jpeg_std_error( &jerr_.pub );
          jerr_.pub.error_exit = my_error_exit;
          jerr_.pub.output_message = my_output_message;
-         if( setjmp( jerr_.setjmp_buffer )) {
-            // If we get here, the JPEG code has signaled an error.
-            DIP_THROW_RUNTIME( "Error writing JPEG file." );
-         }
+         std::memcpy( jerr_.setjmp_buffer, setjmp_buffer, sizeof( setjmp_buffer ));
          jpeg_create_compress( &cinfo_ );
          initialized_ = true;
          jpeg_stdio_dest( &cinfo_, outfile_ );
@@ -178,7 +181,8 @@ FileInformation ImageReadJPEG(
       String const& filename
 ) {
    // Open the file
-   JpegInput jpeg( filename );
+   DIP__DECLARE_JPEG_EXIT( "Error reading JPEG file" );
+   JpegInput jpeg( filename, setjmp_buffer );
 
    // Get info
    FileInformation info = GetJPEGInfo( jpeg );
@@ -223,14 +227,16 @@ FileInformation ImageReadJPEG(
 }
 
 FileInformation ImageReadJPEGInfo( String const& filename ) {
-   JpegInput jpeg( filename );
+   DIP__DECLARE_JPEG_EXIT( "Error reading JPEG file" );
+   JpegInput jpeg( filename, setjmp_buffer );
    FileInformation info = GetJPEGInfo( jpeg );
    return info;
 }
 
 bool ImageIsJPEG( String const& filename ) {
    try {
-      JpegInput jpeg( filename );
+      DIP__DECLARE_JPEG_EXIT( "Error reading JPEG file" );
+      JpegInput jpeg( filename, setjmp_buffer );
    } catch( ... ) {
       return false;
    }
@@ -244,10 +250,10 @@ void ImageWriteJPEG(
 ) {
    DIP_THROW_IF( !image.IsForged(), E::IMAGE_NOT_FORGED );
    DIP_THROW_IF( image.Dimensionality() != 2, E::DIMENSIONALITY_NOT_SUPPORTED );
-   jpegLevel = clamp< dip::uint >( jpegLevel, 1, 100 );
 
    // Open the file
-   JpegOutput jpeg( filename );
+   DIP__DECLARE_JPEG_EXIT( "Error writing JPEG file" );
+   JpegOutput jpeg( filename, setjmp_buffer );
 
    // Set image properties
    int nchan = static_cast< int >( image.TensorElements() );
@@ -256,7 +262,7 @@ void ImageWriteJPEG(
    jpeg.cinfo().input_components = nchan;
    jpeg.cinfo().in_color_space = nchan > 1 ? JCS_RGB : JCS_GRAYSCALE;
    jpeg_set_defaults( jpeg.cinfoptr() );
-   jpeg_set_quality( jpeg.cinfoptr(), static_cast< int >( jpegLevel ), FALSE );
+   jpeg_set_quality( jpeg.cinfoptr(), static_cast< int >( clamp< dip::uint >( jpegLevel, 1, 100 )), FALSE );
    jpeg.cinfo().density_unit = 2; // dots per cm
    jpeg.cinfo().X_density = static_cast< UINT16 >( 0.01 / image.PixelSize( 0 ).RemovePrefix().magnitude ); // let's assume it's meter
    jpeg.cinfo().Y_density = static_cast< UINT16 >( 0.01 / image.PixelSize( 1 ).RemovePrefix().magnitude );