Cherry-picking

.github/workflows/build_test.yml

@@ -81,7 +81,6 @@ jobs:
           # smaller build if only decoding to pixels is needed.
           - name: release-nojpeg
             mode: release
-            cxxflags: -DJXL_DEBUG_ON_ABORT=0
             cmake_args: >-
               -DJPEGXL_ENABLE_TRANSCODE_JPEG=OFF
               -DJPEGXL_ENABLE_PLUGINS=OFF
@@ -103,7 +102,6 @@ jobs:
           # reconstructing pixels is disabled.
           - name: release:minimal
             mode: release
-            cxxflags: -DJXL_DEBUG_ON_ABORT=0
             cmake_args: >-
               -DJPEGXL_ENABLE_TRANSCODE_JPEG=OFF
               -DJPEGXL_ENABLE_BOXES=OFF

CHANGELOG.md

@@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## Unreleased
 
 ### Added
+  - Gain Map API (#3552 and #3628):  `JxlGainMapBundle` struct and API functions
+    to read and write gain map bundles`JxlGainMapWriteBundle` and
+    `JxlGainMapReadBundle` as well as handling compressed ICC profiles:
+    `JxlICCProfileEncode` and `JxlICCProfileDecode`.
   - decoder API: added `JXL_DEC_BOX_COMPLETE` event to signal that the output
     buffer for the current box has received all contents. Previously, this was
     to be determined from the fact that the decoder had moved on either to
@@ -16,12 +20,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
     subscribed to, and that `JXL_DEC_SUCCESS` / `JXL_DEC_BOX` still occur
     afterwards and still imply that the previous box must be complete.
 
-### Removed
-
 ### Changed / clarified
-
-### Fixed
-
+  - avoiding abort in release build (#3631 and #3639)
 
 ## [0.10.2] - 2024-03-08
 

CMakeLists.txt

@@ -336,7 +336,6 @@ else ()
   if(JPEGXL_ENABLE_COVERAGE)
     set(JPEGXL_COVERAGE_FLAGS
         -g -O0 -fprofile-arcs -ftest-coverage
-        -DJXL_ENABLE_ASSERT=0 -DJXL_ENABLE_CHECK=0
     )
     set(JPEGXL_COVERAGE_LINK_FLAGS
         --coverage

bash_test.sh

@@ -8,7 +8,8 @@
 # Tests implemented in bash. These typically will run checks about the source
 # code rather than the compiled one.
 
-MYDIR=$(dirname $(realpath "$0"))
+SELF=$(realpath "$0")
+MYDIR=$(dirname "${SELF}")
 
 set -u
 

ci.sh

@@ -13,7 +13,8 @@ set -eu
 
 OS=`uname -s`
 
-MYDIR=$(dirname $(realpath "$0"))
+SELF=$(realpath "$0")
+MYDIR=$(dirname "${SELF}")
 
 ### Environment parameters:
 TEST_STACK_LIMIT="${TEST_STACK_LIMIT:-256}"
@@ -499,7 +500,7 @@ cmd_release() {
 
 cmd_opt() {
   CMAKE_BUILD_TYPE="RelWithDebInfo"
-  CMAKE_CXX_FLAGS+=" -DJXL_DEBUG_BUILD -DJXL_DEBUG_ON_ERROR"
+  CMAKE_CXX_FLAGS+=" -DJXL_IS_DEBUG_BUILD"
   cmake_configure "$@"
   cmake_build_and_test
 }
@@ -593,9 +594,9 @@ cmd_msanfuzz() {
 
 cmd_asan() {
   SANITIZER="asan"
-  CMAKE_C_FLAGS+=" -DJXL_ENABLE_ASSERT=1 -g -DADDRESS_SANITIZER \
+  CMAKE_C_FLAGS+=" -g -DADDRESS_SANITIZER \
     -fsanitize=address ${UBSAN_FLAGS[@]}"
-  CMAKE_CXX_FLAGS+=" -DJXL_ENABLE_ASSERT=1 -g -DADDRESS_SANITIZER \
+  CMAKE_CXX_FLAGS+=" -g -DADDRESS_SANITIZER \
     -fsanitize=address ${UBSAN_FLAGS[@]}"
   strip_dead_code
   cmake_configure "$@" -DJPEGXL_ENABLE_TCMALLOC=OFF
@@ -605,7 +606,6 @@ cmd_asan() {
 cmd_tsan() {
   SANITIZER="tsan"
   local tsan_args=(
-    -DJXL_ENABLE_ASSERT=1
     -g
     -DTHREAD_SANITIZER
     -fsanitize=thread
@@ -631,7 +631,6 @@ cmd_msan() {
     -fsanitize=memory
     -fno-omit-frame-pointer
 
-    -DJXL_ENABLE_ASSERT=1
     -g
     -DMEMORY_SANITIZER
 
@@ -1212,7 +1211,7 @@ cmd_lint() {
   # It is ok, if buildifier is not installed.
   #if which buildifier >/dev/null; then
   #  local buildifier_patch="${tmpdir}/buildifier.patch"
-  #  local bazel_files=`git -C ${MYDIR} ls-files | grep -E "/BUILD$|WORKSPACE|.bzl$"`
+  #  local bazel_files=`git -C "${MYDIR}" ls-files | grep -E "/BUILD$|WORKSPACE|.bzl$"`
   #  set -x
   #  buildifier -d ${bazel_files} >"${buildifier_patch}"|| true
   #  { set +x; } 2>/dev/null
@@ -1228,8 +1227,8 @@ cmd_lint() {
   # It is ok, if spell-checker is not installed.
   if which typos >/dev/null; then
     local src_ext="bazel|bzl|c|cc|cmake|gni|h|html|in|java|js|m|md|nix|py|rst|sh|ts|txt|yaml|yml"
-    local sources=`git -C ${MYDIR} ls-files | grep -E "\.(${src_ext})$"`
-    typos -c ${MYDIR}/tools/scripts/typos.toml ${sources}
+    local sources=`git -C "${MYDIR}" ls-files | grep -E "\.(${src_ext})$"`
+    typos -c "${MYDIR}/tools/scripts/typos.toml" ${sources}
   else
     echo "Consider installing https://github.com/crate-ci/typos for spell-checking"
   fi

deps.sh

@@ -10,7 +10,8 @@
 
 set -eu
 
-MYDIR=$(dirname $(realpath "$0"))
+SELF=$(realpath "$0")
+MYDIR=$(dirname "${SELF}")
 
 # Git revisions we use for the given submodules. Update these whenever you
 # update a git submodule.

doc/fuzzing.md

@@ -103,7 +103,7 @@ clang package for that version outside the container and using `clang-NN`
 (for example `clang-11`) instead of `clang` in the following commands:
 
 ```bash
-symbolizer=$($(realpath $(which clang)) -print-prog-name=llvm-symbolizer)
+symbolizer=$($(realpath "$(which clang)") -print-prog-name=llvm-symbolizer)
 export MSAN_SYMBOLIZER_PATH="${symbolizer}"
 export UBSAN_SYMBOLIZER_PATH="${symbolizer}"
 export ASAN_SYMBOLIZER_PATH="${symbolizer}"
@@ -135,14 +135,7 @@ perform an invalid operation (read/write out of bounds, perform an undefined
 behavior operation, etc). You can help the fuzzer find invalid situations by
 adding asserts:
 
- * `JXL_ASSERT()` is enabled in Release mode by default. It can be disabled
-   with `-DJXL_ENABLE_ASSERT=0` but the intention is that it will run for all
-   the users in released code. If performance of the check is not an issue (like
-   checks done once per image, once per channel, once per group, etc) a
-   JXL_ASSERT is appropriate. A failed assert is preferable to an out of bounds
-   write.
-
- * `JXL_DASSERT()` is only enabled in Debug builds, which includes all the ASan,
+ * `JXL_DASSERT` is only enabled in Debug builds, which includes all the ASan,
    MSan and UBSan builds. Performance of these checks is not an issue if kept
    within reasonable limits (automated msan/asan test should finish within 1
    hour for example). Fuzzing is more effective when the given input runs

doc/vuln_playbook.md

@@ -195,7 +195,7 @@ engineers on the required backports.
 A security bug is a bug that can potentially be exploited to let an attacker
 gain unauthorized access or privileges. For example, gaining code execution in
 libjxl decoder by decoding a malicious .jxl file is a security but hitting a
-`JXL_ASSERT()` is not necessarily one.
+`JXL_DASSERT` is not necessarily one.
 
 The supported use cases to consider in the context of security bugs that require
 a vulnerability disclosure are "release" builds. The disclosure is intended for
@@ -215,22 +215,9 @@ released software will not have the developer code. This developer code is in
 the same libjxl repository for convenience.
 
 When considering the impact of a bug, "release" mode should be assumed. In
-release mode `JXL_ASSERT()` and `JXL_CHECK()` are enabled, but `JXL_DASSERT()`
-are not. This means that if a `JXL_DASSERT()` protects an out-of-bounds (OOB)
-write, then the impact of a bug hitting the `JXL_DASSERT()` is at least an
-OOB write. On the other hand, if a bug ends up hitting a `JXL_CHECK()` instead
-of continuing, the only impact is the process abort instead of whatever else is
-possible after the `JXL_CHECK()`.
-
-Asserts in `libjxl` *tools* cause the tool process to abort, but don't affect
-the caller. Either crashing or returning an error (non-zero exit code) would
-have the same effect, so `JXL_ASSERT()` failures in the tools have no security
-or functional impact.
-
-Asserts in `libjxl` libraries, meant to be linked into other processes, cause
-the caller process to abort, potentially causing a Denial of Service, however,
-Denial of Service issues are *not* considered security bugs by this policy.
-These are still issues and should be fixed, but they are not security issues.
+non-release mode `JXL_DASSERT` is enabled. This means that if a
+`JXL_DASSERT` protects an out-of-bounds (OOB) write, then the impact of a bug
+hitting the `JXL_DASSERT` is at least an OOB write.
 
 Out-of-bounds (OOB) reads in process memory are considered security
 vulnerabilities. OOB reads may allow an attacker to read other buffers from the

lib/base/compiler_specific.h

@@ -13,6 +13,10 @@
 
 #include "lib/base/sanitizer_definitions.h"
 
+#if JXL_ADDRESS_SANITIZER || JXL_MEMORY_SANITIZER || JXL_THREAD_SANITIZER
+#include "sanitizer/common_interface_defs.h"  // __sanitizer_print_stack_trace
+#endif                                        // defined(*_SANITIZER)
+
 // #if is shorter and safer than #ifdef. *_VERSION are zero if not detected,
 // otherwise 100 * major + minor version. Note that other packages check for
 // #ifdef COMPILER_MSVC, so we cannot use that same name.
@@ -159,13 +163,59 @@ using ssize_t = intptr_t;
 #define JXL_CXX_17 201703
 
 // In most cases we consider build as "debug". Use `NDEBUG` for release build.
-#if defined(JXL_DEBUG_BUILD)
-#undef JXL_DEBUG_BUILD
-#define JXL_DEBUG_BUILD 1
+#if defined(JXL_IS_DEBUG_BUILD)
+#undef JXL_IS_DEBUG_BUILD
+#define JXL_IS_DEBUG_BUILD 1
 #elif defined(NDEBUG)
-#define JXL_DEBUG_BUILD 0
+#define JXL_IS_DEBUG_BUILD 0
+#else
+#define JXL_IS_DEBUG_BUILD 1
+#endif
+
+#if defined(JXL_CRASH_ON_ERROR)
+#undef JXL_CRASH_ON_ERROR
+#define JXL_CRASH_ON_ERROR 1
+#else
+#define JXL_CRASH_ON_ERROR 0
+#endif
+
+#if JXL_CRASH_ON_ERROR && !JXL_IS_DEBUG_BUILD
+#error "JXL_CRASH_ON_ERROR requires JXL_IS_DEBUG_BUILD"
+#endif
+
+// Pass -DJXL_DEBUG_ON_ALL_ERROR at compile time to print debug messages on
+// all error (fatal and non-fatal) status.
+#if defined(JXL_DEBUG_ON_ALL_ERROR)
+#undef JXL_DEBUG_ON_ALL_ERROR
+#define JXL_DEBUG_ON_ALL_ERROR 1
+#else
+#define JXL_DEBUG_ON_ALL_ERROR 0
+#endif
+
+#if JXL_DEBUG_ON_ALL_ERROR && !JXL_IS_DEBUG_BUILD
+#error "JXL_DEBUG_ON_ALL_ERROR requires JXL_IS_DEBUG_BUILD"
+#endif
+
+// Pass -DJXL_DEBUG_ON_ABORT={0} to disable the debug messages on
+// (debug) JXL_ENSURE and JXL_DASSERT.
+#if !defined(JXL_DEBUG_ON_ABORT)
+#define JXL_DEBUG_ON_ABORT JXL_IS_DEBUG_BUILD
+#endif  // JXL_DEBUG_ON_ABORT
+
+#if JXL_DEBUG_ON_ABORT && !JXL_IS_DEBUG_BUILD
+#error "JXL_DEBUG_ON_ABORT requires JXL_IS_DEBUG_BUILD"
+#endif
+
+#if JXL_ADDRESS_SANITIZER || JXL_MEMORY_SANITIZER || JXL_THREAD_SANITIZER
+#define JXL_PRINT_STACK_TRACE() __sanitizer_print_stack_trace();
+#else
+#define JXL_PRINT_STACK_TRACE()
+#endif
+
+#if JXL_COMPILER_MSVC
+#define JXL_CRASH() __debugbreak(), (void)abort()
 #else
-#define JXL_DEBUG_BUILD 1
+#define JXL_CRASH() (void)__builtin_trap()
 #endif
 
 #endif  // LIB_JXL_BASE_COMPILER_SPECIFIC_H_

lib/base/data_parallel.h

@@ -47,7 +47,7 @@ class ThreadPool {
   template <class InitFunc, class DataFunc>
   Status Run(uint32_t begin, uint32_t end, const InitFunc& init_func,
              const DataFunc& data_func, const char* caller) {
-    JXL_ASSERT(begin <= end);
+    JXL_ENSURE(begin <= end);
     if (begin == end) return true;
     RunCallState<InitFunc, DataFunc> call_state(init_func, data_func);
     // The runner_ uses the C convention and returns 0 in case of error, so we

lib/base/parallel_runner.h

@@ -46,19 +46,20 @@ extern "C" {
 #endif
 
 /** Return code used in the JxlParallel* functions as return value. A value
- * of 0 means success and any other value means error. The special value
- * ::JXL_PARALLEL_RET_RUNNER_ERROR can be used by the runner to indicate any
- * other error.
+ * of ::JXL_PARALLEL_RET_SUCCESS means success and any other value means error.
+ * The special value ::JXL_PARALLEL_RET_RUNNER_ERROR can be used by the runner
+ * to indicate any other error.
  */
 typedef int JxlParallelRetCode;
 
 /**
  * Code returned by the @ref JxlParallelRunInit function to indicate success.
  */
 #define JXL_PARALLEL_RET_SUCCESS (0)
+
 /**
- * General error returned by the @ref JxlParallelRunInit function to indicate
- * an error.
+ * Code returned by the @ref JxlParallelRunInit function to indicate a general
+ * error.
  */
 #define JXL_PARALLEL_RET_RUNNER_ERROR (-1)
 
@@ -151,7 +152,7 @@ typedef JxlParallelRetCode (*JxlParallelRunner)(
       // order.
       (*func)(jpegxl_opaque, i, 0);
     }
-    return 0;
+    return JXL_PARALLEL_RET_SUCCESS;
   }
  */
 

lib/base/random.h

@@ -21,7 +21,7 @@
 
 namespace jxl {
 struct Rng {
-  explicit Rng(size_t seed)
+  explicit Rng(uint64_t seed)
       : s{static_cast<uint64_t>(0x94D049BB133111EBull),
           static_cast<uint64_t>(0xBF58476D1CE4E5B9ull) + seed} {}
 

lib/base/rect.h

@@ -62,14 +62,6 @@ class RectT {
     return Intersection(RectT(0, 0, area_xsize, area_ysize));
   }
 
-  // Returns a rect that only contains `num` lines with offset `y` from `y0()`.
-  RectT Lines(size_t y, size_t num) const {
-    JXL_DASSERT(y + num <= ysize_);
-    return RectT(x0_, y0_ + y, xsize_, num);
-  }
-
-  RectT Line(size_t y) const { return Lines(y, 1); }
-
   JXL_MUST_USE_RESULT RectT Intersection(const RectT& other) const {
     return RectT(std::max(x0_, other.x0_), std::max(y0_, other.y0_), xsize_,
                  ysize_, std::min(x1(), other.x1()),
@@ -142,19 +134,14 @@ class RectT {
   RectT<T> ShiftLeft(size_t shift) const { return ShiftLeft(shift, shift); }
 
   // Requires x0(), y0() to be multiples of 1<<shiftx, 1<<shifty.
-  RectT<T> CeilShiftRight(size_t shiftx, size_t shifty) const {
-    JXL_ASSERT(x0_ % (1 << shiftx) == 0);
-    JXL_ASSERT(y0_ % (1 << shifty) == 0);
+  StatusOr<RectT<T>> CeilShiftRight(std::pair<size_t, size_t> shift) const {
+    size_t shiftx = shift.first;
+    size_t shifty = shift.second;
+    JXL_ENSURE((x0_ % (1 << shiftx) == 0) && (y0_ % (1 << shifty) == 0));
     return RectT<T>(x0_ / (1 << shiftx), y0_ / (1 << shifty),
                     DivCeil(xsize_, T{1} << shiftx),
                     DivCeil(ysize_, T{1} << shifty));
   }
-  RectT<T> CeilShiftRight(std::pair<size_t, size_t> shift) const {
-    return CeilShiftRight(shift.first, shift.second);
-  }
-  RectT<T> CeilShiftRight(size_t shift) const {
-    return CeilShiftRight(shift, shift);
-  }
 
   RectT<T> Extend(T border, RectT<T> parent) const {
     T new_x0 = x0() > parent.x0() + border ? x0() - border : parent.x0();

lib/base/sanitizers.h

@@ -159,10 +159,10 @@ static JXL_INLINE JXL_MAYBE_UNUSED void PrintImageUninitialized(
 template <typename Pixels>
 static JXL_INLINE JXL_MAYBE_UNUSED void CheckImageInitialized(
     const Pixels& im, const Rect& r, size_t c, const char* message) {
-  JXL_ASSERT(r.x0() <= im.xsize());
-  JXL_ASSERT(r.x0() + r.xsize() <= im.xsize());
-  JXL_ASSERT(r.y0() <= im.ysize());
-  JXL_ASSERT(r.y0() + r.ysize() <= im.ysize());
+  JXL_DASSERT(r.x0() <= im.xsize());
+  JXL_DASSERT(r.x0() + r.xsize() <= im.xsize());
+  JXL_DASSERT(r.y0() <= im.ysize());
+  JXL_DASSERT(r.y0() + r.ysize() <= im.ysize());
   for (size_t y = r.y0(); y < r.y0() + r.ysize(); y++) {
     const auto* row = im.Row(y);
     intptr_t ret = __msan_test_shadow(row + r.x0(), sizeof(*row) * r.xsize());

lib/base/span.h

@@ -60,10 +60,11 @@ class Span {
     return *(data() + i);
   }
 
-  void remove_prefix(size_t n) noexcept {
-    JXL_ASSERT(size() >= n);
+  Status remove_prefix(size_t n) noexcept {
+    JXL_ENSURE(size() >= n);
     ptr_ += n;
     len_ -= n;
+    return true;
   }
 
   void AppendTo(std::vector<NCT>& dst) const {