- Fix an issue whereby a build with ENABLE_SHARED=0 could not be
installed when using the Ninja Multi-Config CMake generator.
- Fix an issue whereby a Windows installer could not be built when using
the Ninja Multi-Config CMake generator.
- Fix an issue whereby the Java regression tests failed when using the
Ninja Multi-Config CMake generator.
Based on:
4f169deeb0Closes#626
... on platforms that support TLS, which should include all
currently-supported platforms
(https://libjpeg-turbo.org/Documentation/OfficialBinaries)
Addresses a concern raised in #87
Although it is still my opinion that the data race in init_simd() was
innocuous, we can now fix it for free thanks to
ae87a95861, so why not?
This commit reverts 4dbc293125 and
9f8f683e74 (the previous two commits) and
fixes#613 the correct way. The crux of the issue wasn't the size of
the whole_image virtual array but rather that, since last_iMCU_row is
unsigned, (last_iMCU_row - 1) wrapped around to 0xFFFFFFFF when
last_iMCU_row was 0. This caused the interblock smoothing algorithm
introduced in 6d91e950c8 to erroneously
try to access the next two iMCU rows, neither of which existed. The
first attempt at a fix (4dbc293125)
exposed a NULL dereference, detected by OSS-Fuzz, that occurred when
attempting to decompress a specially-crafted malformed JPEG image to a
YUV buffer using tjDecompressToYUV*() with 1/4 IDCT scaling.
Fixes#613 (again)
Also fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=49898
Regression introduced by 6d91e950c8
Because we're now using a 5x5 smoothing window when decompressing
progressive JPEG images, we need to ensure that the whole_image virtual
array contains at least five rows. Previously that was not always the
case unless the progressive JPEG image being decompressed had at least
five iMCU rows. Since an iMCU has a height of (8 * the vertical
sampling factor), attempting to decompress 4:2:2 and 4:4:4 images <= 32
pixels in height or 4:2:0 images <= 64 pixels in height triggered a
JERR_BAD_VIRTUAL_ACCESS error in decompress_smooth_data(), because
access_rows exceeded the number of rows in the virtual array.
Fixes#613
libjpeg-turbo's AltiVec SIMD extensions previously assumed that AltiVec
instructions were available on all Power Macs that supported OS X 10.4
"Tiger" (the earliest version of OS X that libjpeg-turbo has ever
supported), but Tiger can actually run on PowerPC G3 processors, which
lack AltiVec instructions. This commit enables run-time detection of
AltiVec instructions on OS X/PowerPC systems if AltiVec instructions are
not force-enabled at compile time (using -maltivec). This allows the
same build of libjpeg-turbo to support G3, G4, and G5 Power Macs.
Closes#609
Although it is uncommon, some downstream implementations undefine one or
more of the *_SUPPORTED macros in jmorecfg.h in order to reduce the size
of the library. In the interest of maintaining backward compatibility
with libjpeg, this is still a supported use case.
Regression introduced by 9120a24743
Based on:
74c4d032f0Closes#601
MinGW defines strcasecmp() and strncasecmp() as macros in string.h if
__CRT__NO_INLINE is defined, which will be the case when including any
of the Win32 API headers.
Closes#594
Referring to https://github.com/google/oss-fuzz/issues/7575, if the
fuzzer suffix contains periods, it can cause ClusterFuzz to misinterpret
the file extension of the fuzzer executables and thus misidentify them.
When building without the SIMD extensions, memory allocations are
currently aligned to sizeof(double). However, this may be insufficient
on architectures such as Arm Morello or 64-bit CHERI-RISC-V where
pointers require 16-byte rather than 8-byte alignment. This patch
causes memory allocations to be aligned to
MAX(sizeof(void *), sizeof(double)) when building without the SIMD
extensions.
(NOTE: With C11 we could instead use alignof(max_align_t), but C89
compatibility is still necessary in libjpeg-turbo.)
Closes#587
Referring to the conversation in
https://github.com/google/oss-fuzz/issues/7479 and #559, there was a
misunderstanding regarding how CIFuzz works. It cannot be used to fuzz
arbitrary PRs or code branches, and it has a 90-day delay in downloading
corpora from OSS-Fuzz. That makes it unsuitable for libjpeg-turbo.
Newer versions of the 32-bit x86 Visual Studio compiler produce results
compatible with FLOATTEST=no-fp-contract, so we can no longer
intelligently set a default FLOATTEST value for that platform.
When 12-bit-per-component JPEG support is enabled (WITH_12BIT=1) or the
TurboJPEG API library and associated test programs are disabled
(WITH_TURBOJPEG=0), the Windows installer target should not depend on
the turbojpeg, turbojpeg-static, and tjbench targets.
(broken by 607b668ff9)
- Visual Studio 2010 apparently doesn't have the snprintf() inline
function, so restore the macro that emulates that function using
_snprintf_s().
- Explicitly include errno.h in strtest.c, since jinclude.h doesn't
include it when building with Visual Studio.
- Remove the section in libjpeg.txt that advised against building
libjpeg as a shared library. We obviously do not follow that advice,
and libjpeg-turbo does guarantee backward ABI compatibility in our
libjpeg API library, even though libjpeg did not and does not.
(Future expansion of our libjpeg API library, if necessary, will be
accomplished using get/set functions that store the new parameters
in the opaque master structs. Refer to
db2986c96f.)
- Unmention install.txt, which was never relevant to libjpeg-turbo and
was removed in v1.3 (6f96153c67).
- Remove extraneous spaces.
- Document the fact that TWO_FILE_COMMANDLINE must be defined in order
to use the two-file interface with cjpeg, djpeg, jpegtran, and
wrjpgcom. libjpeg-turbo never enables that interface by default.
People keep trying to include libjpeg-turbo into downstream CMake-based
build systems by way of the add_subdirectory() function and requesting
upstream support when something inevitably breaks.
(Refer to: #122, #173, #176, #202, #241, #349, #353, #412, #504,
a3d4aadd0d (commitcomment-67575889)).
libjpeg-turbo has never supported that method of sub-project
integration, because doing so would require that we (minimally):
1. avoid using certain CMake variables, such as CMAKE_SOURCE_DIR,
CMAKE_BINARY_DIR, and CMAKE_PROJECT_NAME;
2. avoid using implicit include directories and relative paths;
3. provide a way to optionally skip the installation of libjpeg-turbo
components in response to 'make install';
4. provide a way to optionally postfix target names, to avoid namespace
conflicts;
5. restructure the top-level CMakeLists.txt so that it properly sets
the PROJECT_VERSION variable; and
6. design automated CI tests to ensure that new commits don't break
any of the above.
Even if we did all of that, issues would still arise, because it is
impossible for one upstream build system to anticipate the widely
varying needs of every downstream build system. That's why the CMake
ExternalProject_Add() function exists, and it is my sincere hope that
adding a blurb to BUILDING.md mentioning the need to use that function
will head off future GitHub issues on this topic. If not, then I can at
least post a link to this commit and the blurb and avoid doing the same
song and dance over and over again.
The loop in jsimd_quantize_neon() is only executed twice and should be
unrolled for AArch64 targets. GCC does that by default, but Clang 11
and later versions available at the time of this writing do not. This
patch adds an unroll pragma when targetting AArch64 with Clang. We do
not use the unroll pragma for AArch32 targets, because it causes the
Clang-generated assembly code to exhaust the available Neon registers
(32 x 64-bit) and spill to the stack. (DRC: Referring to the discussion
in #570, this is likely due to compiler confusion that results in poor
register allocation. It is possible to eliminate the spillage and
reduce the instruction count by loading the data on a just-in-time
basis, thus explicitly interleaving compute and I/O, but the performance
implications of that are currently unknown.)
The effects of unrolling the quantization loop are:
1) elimination of the loop control flow overhead and
2) enabling the use of LDP/STP instructions that work from a single
base pointer, instead of using double the number of LDR/STR
instructions, each requiring an address calculation.
Closes#570
- Suppress a UBSan warning regarding storing a 64-bit value to a
non-64-bit-aligned address. That behavior is technically undefined
per the C spec but is supported in the context of the AArch64
architecture and compilers.
- Explicitly promote block_diff[i] to unsigned int prior to left
shifting it, in order to avoid a UBSan warning. This warning also
described behavior that is technically undefined per the C spec but is
supported in the context of the AArch64 architecture and compilers.
Changing the type cast order eliminated the warning without changing
the generated assembly code.
Closes#582
- Make better use of 128-bit vector registers, thus reducing the number
of Neon instructions required to construct the AC coefficient bitmap.
- Refactor the Neon computations of 'nbits' and 'diff' to use shorter
and higher-throughput instruction sequences.
DRC's notes:
This commit partially integrates #570. Arm reported a 1-4% speedup on
Cortex-A55 and Neoverse-N1 cores when using recent compilers but little
or no speedup with Clang 10. I observed no speedup with Clang 10 on my
Cortex-A53 and Cortex-A72 cores. Thus, referring to #582, the primary
purpose of this commit is to fix UBSan warnings regarding the shift
operations previously located at Line 253:
d640a45730/simd/arm/aarch64/jchuff-neon.c (L253)
The primary purpose of this is to encourage adoption of libjpeg-turbo in
downstream Windows projects that forbid the use of "deprecated"
functions. libjpeg-turbo's usage of those functions was not actually
unsafe, because:
- libjpeg-turbo always checks the return value of fopen() and ensures
that a NULL filename can never be passed to it.
- libjpeg-turbo always checks the return value of getenv() and never
passes a NULL argument to it.
- The sprintf() calls in format_message() (jerror.c) could never
overflow the destination string buffer or leave it unterminated as
long as the buffer was at least JMSG_LENGTH_MAX bytes in length, as
instructed. (Regardless, this commit replaces those calls with
snprintf() calls.)
- libjpeg-turbo never uses sscanf() to read strings or multi-byte
character arrays.
- Because of b7d6e84d6a, wrjpgcom
explicitly checks the bounds of the source and destination strings
before calling strcat() and strcpy().
- libjpeg-turbo always ensures that the destination string is
terminated when using strncpy().
(548490fe5e made this explicit.)
Regarding thread safety:
Technically speaking, getenv() is not thread-safe, because the returned
pointer may be invalidated if another thread sets the same environment
variable between the time that the first thread calls getenv() and the
time that that thread uses the return value. In practice, however, this
could only occur with libjpeg-turbo if:
(1) A multithreaded calling application used the deprecated and
undocumented TJFLAG_FORCEMMX/TJFLAG_FORCESSE/TJFLAG_FORCESSE2 flags in
the TurboJPEG API or set one of the corresponding environment variables
(which are only intended for testing purposes.) Since the TurboJPEG API
library only ever passed string constants to putenv(), the only inherent
risk (i.e. the only risk introduced by the library and not the calling
application) was that the SIMD extensions may have read an incorrect
value from one of the aforementioned environment variables.
or
(2) A multithreaded calling application modified the value of the
JPEGMEM environment variable in one thread while another thread was
reading the value of that environment variable (in the body of
jpeg_create_compress() or jpeg_create_decompress().) Given that the
libjpeg API provides a thread-safe way for applications to modify the
default memory limit without using the JPEGMEM environment variable,
direct modification of that environment variable by calling applications
is not supported.
Microsoft's implementation of getenv_s() does not claim to be
thread-safe either, so this commit uses getenv_s() solely to mollify
Visual Studio. New inline functions and macros (GETENV_S() and
PUTENV_S) wrap getenv_s()/_putenv_s() when building for Visual Studio
and getenv()/setenv() otherwise, but GETENV_S()/PUTENV_S() provide no
advantages over getenv()/setenv() other than parameter validation. They
are implemented solely for convenience.
Technically speaking, strerror() is not thread-safe, because the
returned pointer may be invalidated if another thread changes the locale
and/or calls strerror() between the time that the first thread calls
strerror() and the time that that thread uses the return value. In
practice, however, this could only occur with libjpeg-turbo if a
multithreaded calling application encountered a file I/O error in
tjLoadImage() or tjSaveImage(). Since both of those functions
immediately copy the string returned from strerror() into a thread-local
buffer, the risk is minimal, and the worst case would involve an
incorrect error string being reported to the calling application.
Regardless, this commit uses strerror_s() in the TurboJPEG API library
when building for Visual Studio. Note that strerror_r() could have been
used on Un*x systems, but it would have been necessary to handle both
the POSIX and GNU implementations of that function and perform
widespread compatibility testing. Such is left as an exercise for
another day.
Fixes#568
- Since the ERREXITS() and TRACEMSS() macros are never used internally
(they are a relic of the legacy memory managers that libjpeg
provided), the only risk was that an external program might have
invoked one of those macros with a string longer than 79 characters
(JMSG_STR_PARM_MAX - 1).
- TJBench never invokes the THROW_TJ() macro with a string longer than
199 (JMSG_LENGTH_MAX - 1) characters, so there was no risk. However,
it's a good idea to explicitly terminate the destination strings so
that anyone looking at the code can immediately tell that it is safe.
The h2v2 (4:2:0) merged upsampler uses a spare row buffer so that it can
upsample two rows at a time but return only one row to the application,
if necessary. merged_2v_upsample() copies from this spare row buffer
into the application-supplied output buffer, using the out_row_width
field in the my_merged_upsampler struct to determine how many samples to
copy. out_row_width is set in jinit_merged_upsampler(), which is called
within the body of jpeg_start_decompress(). Since jpeg_crop_scanline()
must be called after jpeg_start_decompress(), jpeg_crop_scanline() must
modify the value of out_row_width if the h2v2 merged upsampler will be
used. Otherwise, merged_2v_upsample() can overflow the output buffer if
the number of bytes between the current output buffer position and the
end of the buffer is less than the number of bytes required to represent
an uncropped scanline of the output image. All of the destination
managers used by djpeg allocate either a whole image buffer or a
scanline buffer based on the uncropped output image width, so this issue
is not reproducible using djpeg.
Fixes#574
