Move setjmp() calls from .cpp to .c files.
Avoid issue where C++ objects may be in an inconsistent state
following the duplicate return from a corresponding longjmp().
Doesn't fix the underlying issue in the referenced bug.
-- rename functions in jpeg_common.c for consistency.
Bug: 379259821
Change-Id: I9c9eff840ad336b9dd5ce5364c7fc68920bc184a
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/126254
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Thomas Sepez <tsepez@google.com>
diff --git a/core/fxcodec/jpeg/jpeg_common.c b/core/fxcodec/jpeg/jpeg_common.c
index a021761..7ea70bf 100644
--- a/core/fxcodec/jpeg/jpeg_common.c
+++ b/core/fxcodec/jpeg/jpeg_common.c
@@ -6,19 +6,56 @@
#include "core/fxcodec/jpeg/jpeg_common.h"
-void src_do_nothing(j_decompress_ptr cinfo) {}
+// This is a thin C wrapper around the JPEG API to avoid calling setjmp() from
+// C++ code.
-boolean src_fill_buffer(j_decompress_ptr cinfo) {
+boolean jpeg_common_create_decompress(JpegCommon* jpeg_common) {
+ if (setjmp(jpeg_common->jmpbuf) == -1) {
+ return FALSE;
+ }
+ jpeg_create_decompress(&jpeg_common->cinfo);
+ return TRUE;
+}
+
+void jpeg_common_destroy_decompress(JpegCommon* jpeg_common) {
+ jpeg_destroy_decompress(&jpeg_common->cinfo);
+}
+
+boolean jpeg_common_start_decompress(JpegCommon* jpeg_common) {
+ if (setjmp(jpeg_common->jmpbuf) == -1) {
+ return FALSE;
+ }
+ return jpeg_start_decompress(&jpeg_common->cinfo);
+}
+
+int jpeg_common_read_header(JpegCommon* jpeg_common, boolean flag) {
+ if (setjmp(jpeg_common->jmpbuf) == -1) {
+ return -1;
+ }
+ return jpeg_read_header(&jpeg_common->cinfo, flag);
+}
+
+int jpeg_common_read_scanlines(JpegCommon* jpeg_common,
+ void* buf,
+ unsigned int count) {
+ if (setjmp(jpeg_common->jmpbuf) == -1) {
+ return -1;
+ }
+ return jpeg_read_scanlines(&jpeg_common->cinfo, buf, count);
+}
+
+void jpeg_common_src_do_nothing(j_decompress_ptr cinfo) {}
+
+boolean jpeg_common_src_fill_buffer(j_decompress_ptr cinfo) {
return FALSE;
}
-boolean src_resync(j_decompress_ptr cinfo, int desired) {
+boolean jpeg_common_src_resync(j_decompress_ptr cinfo, int desired) {
return FALSE;
}
-void error_do_nothing(j_common_ptr cinfo) {}
+void jpeg_common_error_do_nothing(j_common_ptr cinfo) {}
-void error_do_nothing_int(j_common_ptr cinfo, int arg) {}
+void jpeg_common_error_do_nothing_int(j_common_ptr cinfo, int arg) {}
-void error_do_nothing_char(j_common_ptr cinfo, char* arg) {}
-
+void jpeg_common_error_do_nothing_char(j_common_ptr cinfo, char* arg) {}
diff --git a/core/fxcodec/jpeg/jpeg_common.h b/core/fxcodec/jpeg/jpeg_common.h
index 4d8e986..677e44d 100644
--- a/core/fxcodec/jpeg/jpeg_common.h
+++ b/core/fxcodec/jpeg/jpeg_common.h
@@ -44,12 +44,21 @@
};
typedef struct JpegCommon JpegCommon;
-void src_do_nothing(j_decompress_ptr cinfo);
-boolean src_fill_buffer(j_decompress_ptr cinfo);
-boolean src_resync(j_decompress_ptr cinfo, int desired);
-void error_do_nothing(j_common_ptr cinfo);
-void error_do_nothing_int(j_common_ptr cinfo, int arg);
-void error_do_nothing_char(j_common_ptr cinfo, char* arg);
+boolean jpeg_common_create_decompress(JpegCommon* jpeg_common);
+void jpeg_common_destroy_decompress(JpegCommon* jpeg_common);
+boolean jpeg_common_start_decompress(JpegCommon* jpeg_common);
+int jpeg_common_read_header(JpegCommon* jpeg_common, boolean flag);
+int jpeg_common_read_scanlines(JpegCommon* jpeg_common,
+ void* buf,
+ unsigned int count);
+
+// Callbacks.
+void jpeg_common_src_do_nothing(j_decompress_ptr cinfo);
+boolean jpeg_common_src_fill_buffer(j_decompress_ptr cinfo);
+boolean jpeg_common_src_resync(j_decompress_ptr cinfo, int desired);
+void jpeg_common_error_do_nothing(j_common_ptr cinfo);
+void jpeg_common_error_do_nothing_int(j_common_ptr cinfo, int arg);
+void jpeg_common_error_do_nothing_char(j_common_ptr cinfo, char* arg);
#ifdef __cplusplus
} // extern "C"
diff --git a/core/fxcodec/jpeg/jpeg_progressive_decoder.cpp b/core/fxcodec/jpeg/jpeg_progressive_decoder.cpp
index ab0581c..9f6072a 100644
--- a/core/fxcodec/jpeg/jpeg_progressive_decoder.cpp
+++ b/core/fxcodec/jpeg/jpeg_progressive_decoder.cpp
@@ -16,7 +16,6 @@
#include "core/fxcrt/check.h"
#include "core/fxcrt/compiler_specific.h"
#include "core/fxcrt/fx_safe_types.h"
-#include "core/fxcrt/ptr_util.h"
#include "core/fxge/dib/cfx_dibbase.h"
#include "core/fxge/dib/fx_dib.h"
@@ -65,16 +64,16 @@
m_Common.cinfo.err = &m_Common.error_mgr;
m_Common.error_mgr.error_exit = error_fatal;
- m_Common.error_mgr.emit_message = error_do_nothing_int;
- m_Common.error_mgr.output_message = error_do_nothing;
- m_Common.error_mgr.format_message = error_do_nothing_char;
- m_Common.error_mgr.reset_error_mgr = error_do_nothing;
+ m_Common.error_mgr.emit_message = jpeg_common_error_do_nothing_int;
+ m_Common.error_mgr.output_message = jpeg_common_error_do_nothing;
+ m_Common.error_mgr.format_message = jpeg_common_error_do_nothing_char;
+ m_Common.error_mgr.reset_error_mgr = jpeg_common_error_do_nothing;
- m_Common.source_mgr.init_source = src_do_nothing;
- m_Common.source_mgr.term_source = src_do_nothing;
+ m_Common.source_mgr.init_source = jpeg_common_src_do_nothing;
+ m_Common.source_mgr.term_source = jpeg_common_src_do_nothing;
m_Common.source_mgr.skip_input_data = src_skip_data;
- m_Common.source_mgr.fill_input_buffer = src_fill_buffer;
- m_Common.source_mgr.resync_to_restart = src_resync;
+ m_Common.source_mgr.fill_input_buffer = jpeg_common_src_fill_buffer;
+ m_Common.source_mgr.resync_to_restart = jpeg_common_src_resync;
}
CJpegContext::~CJpegContext() {
@@ -109,17 +108,13 @@
// static
std::unique_ptr<ProgressiveDecoderIface::Context>
JpegProgressiveDecoder::Start() {
- // Use ordinary pointer until past the possibility of a longjump.
- auto* pContext = new CJpegContext();
- if (setjmp(pContext->m_Common.jmpbuf) == -1) {
- delete pContext;
+ auto pContext = std::make_unique<CJpegContext>();
+ if (!jpeg_common_create_decompress(&pContext->m_Common)) {
return nullptr;
}
-
- jpeg_create_decompress(&pContext->m_Common.cinfo);
pContext->m_Common.cinfo.src = &pContext->m_Common.source_mgr;
pContext->m_SkipSize = 0;
- return pdfium::WrapUnique(pContext);
+ return pContext;
}
// static
diff --git a/core/fxcodec/jpeg/jpegmodule.cpp b/core/fxcodec/jpeg/jpegmodule.cpp
index 6d97e0e..5d30070 100644
--- a/core/fxcodec/jpeg/jpegmodule.cpp
+++ b/core/fxcodec/jpeg/jpegmodule.cpp
@@ -6,8 +6,6 @@
#include "core/fxcodec/jpeg/jpegmodule.h"
-#include <setjmp.h>
-
#include <memory>
#include <optional>
#include <type_traits>
@@ -65,35 +63,30 @@
static bool JpegLoadInfo(pdfium::span<const uint8_t> src_span,
JpegModule::ImageInfo* pInfo) {
src_span = JpegScanSOI(src_span);
+
JpegCommon jpeg_common = {};
jpeg_common.error_mgr.error_exit = error_fatal;
- jpeg_common.error_mgr.emit_message = error_do_nothing_int;
- jpeg_common.error_mgr.output_message = error_do_nothing;
- jpeg_common.error_mgr.format_message = error_do_nothing_char;
- jpeg_common.error_mgr.reset_error_mgr = error_do_nothing;
+ jpeg_common.error_mgr.emit_message = jpeg_common_error_do_nothing_int;
+ jpeg_common.error_mgr.output_message = jpeg_common_error_do_nothing;
+ jpeg_common.error_mgr.format_message = jpeg_common_error_do_nothing_char;
+ jpeg_common.error_mgr.reset_error_mgr = jpeg_common_error_do_nothing;
jpeg_common.error_mgr.trace_level = 0;
jpeg_common.cinfo.err = &jpeg_common.error_mgr;
jpeg_common.cinfo.client_data = &jpeg_common.jmpbuf;
- if (setjmp(jpeg_common.jmpbuf) == -1) {
+ if (!jpeg_common_create_decompress(&jpeg_common)) {
return false;
}
- jpeg_create_decompress(&jpeg_common.cinfo);
- jpeg_common.source_mgr.init_source = src_do_nothing;
- jpeg_common.source_mgr.term_source = src_do_nothing;
+ jpeg_common.source_mgr.init_source = jpeg_common_src_do_nothing;
+ jpeg_common.source_mgr.term_source = jpeg_common_src_do_nothing;
jpeg_common.source_mgr.skip_input_data = src_skip_data;
- jpeg_common.source_mgr.fill_input_buffer = src_fill_buffer;
- jpeg_common.source_mgr.resync_to_restart = src_resync;
+ jpeg_common.source_mgr.fill_input_buffer = jpeg_common_src_fill_buffer;
+ jpeg_common.source_mgr.resync_to_restart = jpeg_common_src_resync;
jpeg_common.source_mgr.bytes_in_buffer = src_span.size();
jpeg_common.source_mgr.next_input_byte = src_span.data();
jpeg_common.cinfo.src = &jpeg_common.source_mgr;
- if (setjmp(jpeg_common.jmpbuf) == -1) {
- jpeg_destroy_decompress(&jpeg_common.cinfo);
- return false;
- }
- int ret = jpeg_read_header(&jpeg_common.cinfo, TRUE);
- if (ret != JPEG_HEADER_OK) {
- jpeg_destroy_decompress(&jpeg_common.cinfo);
+ if (jpeg_common_read_header(&jpeg_common, TRUE) != JPEG_HEADER_OK) {
+ jpeg_common_destroy_decompress(&jpeg_common);
return false;
}
pInfo->width = jpeg_common.cinfo.image_width;
@@ -102,7 +95,7 @@
pInfo->color_transform = jpeg_common.cinfo.jpeg_color_space == JCS_YCbCr ||
jpeg_common.cinfo.jpeg_color_space == JCS_YCCK;
pInfo->bits_per_components = jpeg_common.cinfo.data_precision;
- jpeg_destroy_decompress(&jpeg_common.cinfo);
+ jpeg_common_destroy_decompress(&jpeg_common);
return true;
}
@@ -134,7 +127,7 @@
void CalcPitch();
void InitDecompressSrc();
- // Can only be called inside a jpeg_read_header() setjmp handler.
+ // Only called when initial jpeg_read_header() fails.
bool HasKnownBadHeaderWithInvalidHeight(size_t dimension_offset) const;
// Is a JPEG SOFn marker, which is defined as 0xff, 0xc[0-9a-f].
@@ -165,8 +158,9 @@
JpegDecoder::JpegDecoder() = default;
JpegDecoder::~JpegDecoder() {
- if (m_bInited)
- jpeg_destroy_decompress(&m_Common.cinfo);
+ if (m_bInited) {
+ jpeg_common_destroy_decompress(&m_Common);
+ }
// Span in superclass can't outlive our buffer.
m_pLastScanline = pdfium::span<uint8_t>();
@@ -175,15 +169,14 @@
bool JpegDecoder::InitDecode(bool bAcceptKnownBadHeader) {
m_Common.cinfo.err = &m_Common.error_mgr;
m_Common.cinfo.client_data = &m_Common.jmpbuf;
- if (setjmp(m_Common.jmpbuf) == -1) {
+ if (!jpeg_common_create_decompress(&m_Common)) {
return false;
}
-
- jpeg_create_decompress(&m_Common.cinfo);
InitDecompressSrc();
m_bInited = true;
-
- if (setjmp(m_Common.jmpbuf) == -1) {
+ m_Common.cinfo.image_width = m_OrigWidth;
+ m_Common.cinfo.image_height = m_OrigHeight;
+ if (jpeg_common_read_header(&m_Common, TRUE) != JPEG_HEADER_OK) {
std::optional<size_t> known_bad_header_offset;
if (bAcceptKnownBadHeader) {
for (size_t offset : kKnownBadHeaderWithInvalidHeightByteOffsetStarts) {
@@ -193,23 +186,23 @@
}
}
}
- jpeg_destroy_decompress(&m_Common.cinfo);
+ jpeg_common_destroy_decompress(&m_Common);
if (!known_bad_header_offset.has_value()) {
m_bInited = false;
return false;
}
-
PatchUpKnownBadHeaderWithInvalidHeight(known_bad_header_offset.value());
-
- jpeg_create_decompress(&m_Common.cinfo);
+ if (!jpeg_common_create_decompress(&m_Common)) {
+ return false;
+ }
InitDecompressSrc();
+ m_Common.cinfo.image_width = m_OrigWidth;
+ m_Common.cinfo.image_height = m_OrigHeight;
+ if (jpeg_common_read_header(&m_Common, TRUE) != JPEG_HEADER_OK) {
+ jpeg_common_destroy_decompress(&m_Common);
+ return false;
+ }
}
- m_Common.cinfo.image_width = m_OrigWidth;
- m_Common.cinfo.image_height = m_OrigHeight;
- int ret = jpeg_read_header(&m_Common.cinfo, TRUE);
- if (ret != JPEG_HEADER_OK)
- return false;
-
if (m_Common.cinfo.saw_Adobe_marker) {
m_bJpegTransform = true;
}
@@ -238,15 +231,15 @@
PatchUpTrailer();
m_Common.error_mgr.error_exit = error_fatal;
- m_Common.error_mgr.emit_message = error_do_nothing_int;
- m_Common.error_mgr.output_message = error_do_nothing;
- m_Common.error_mgr.format_message = error_do_nothing_char;
- m_Common.error_mgr.reset_error_mgr = error_do_nothing;
- m_Common.source_mgr.init_source = src_do_nothing;
- m_Common.source_mgr.term_source = src_do_nothing;
+ m_Common.error_mgr.emit_message = jpeg_common_error_do_nothing_int;
+ m_Common.error_mgr.output_message = jpeg_common_error_do_nothing;
+ m_Common.error_mgr.format_message = jpeg_common_error_do_nothing_char;
+ m_Common.error_mgr.reset_error_mgr = jpeg_common_error_do_nothing;
+ m_Common.source_mgr.init_source = jpeg_common_src_do_nothing;
+ m_Common.source_mgr.term_source = jpeg_common_src_do_nothing;
m_Common.source_mgr.skip_input_data = src_skip_data;
- m_Common.source_mgr.fill_input_buffer = src_fill_buffer;
- m_Common.source_mgr.resync_to_restart = src_resync;
+ m_Common.source_mgr.fill_input_buffer = jpeg_common_src_fill_buffer;
+ m_Common.source_mgr.resync_to_restart = jpeg_common_src_resync;
m_bJpegTransform = ColorTransform;
m_OutputWidth = m_OrigWidth = width;
m_OutputHeight = m_OrigHeight = height;
@@ -271,19 +264,16 @@
bool JpegDecoder::Rewind() {
if (m_bStarted) {
- jpeg_destroy_decompress(&m_Common.cinfo);
+ jpeg_common_destroy_decompress(&m_Common);
if (!InitDecode(/*bAcceptKnownBadHeader=*/false)) {
return false;
}
}
- if (setjmp(m_Common.jmpbuf) == -1) {
- return false;
- }
m_Common.cinfo.scale_denom = m_nDefaultScaleDenom;
m_OutputWidth = m_OrigWidth;
m_OutputHeight = m_OrigHeight;
- if (!jpeg_start_decompress(&m_Common.cinfo)) {
- jpeg_destroy_decompress(&m_Common.cinfo);
+ if (!jpeg_common_start_decompress(&m_Common)) {
+ jpeg_common_destroy_decompress(&m_Common);
return false;
}
CHECK_LE(static_cast<int>(m_Common.cinfo.output_width), m_OrigWidth);
@@ -292,15 +282,11 @@
}
pdfium::span<uint8_t> JpegDecoder::GetNextLine() {
- if (setjmp(m_Common.jmpbuf) == -1) {
+ uint8_t* row_array[] = {m_ScanlineBuf.data()};
+ int nlines = jpeg_common_read_scanlines(&m_Common, row_array, 1u);
+ if (nlines <= 0) {
return pdfium::span<uint8_t>();
}
-
- uint8_t* row_array[] = {m_ScanlineBuf.data()};
- int nlines = jpeg_read_scanlines(&m_Common.cinfo, row_array, 1);
- if (nlines <= 0)
- return pdfium::span<uint8_t>();
-
return m_ScanlineBuf;
}
@@ -410,11 +396,11 @@
uint8_t** dest_buf,
size_t* dest_size) {
jpeg_error_mgr jerr;
- jerr.error_exit = error_do_nothing;
- jerr.emit_message = error_do_nothing_int;
- jerr.output_message = error_do_nothing;
- jerr.format_message = error_do_nothing_char;
- jerr.reset_error_mgr = error_do_nothing;
+ jerr.error_exit = jpeg_common_error_do_nothing;
+ jerr.emit_message = jpeg_common_error_do_nothing_int;
+ jerr.output_message = jpeg_common_error_do_nothing;
+ jerr.format_message = jpeg_common_error_do_nothing_char;
+ jerr.reset_error_mgr = jpeg_common_error_do_nothing;
jpeg_compress_struct cinfo = {}; // Aggregate initialization.
static_assert(std::is_aggregate_v<decltype(cinfo)>);
