Reasoning:
The errors reported for `tokens` in `CodeRepeatedValues`
(src/utils/huffman_encode_utils.c, lines 274, 283, 289, 294)
indicated pointer arithmetic on a pointer assumed to be `__single`.
Since `CodeRepeatedValues` is static and uses `tokens` as an iterator,
its signature was changed to use `HuffmanTreeToken* __indexable` for
both the parameter and return type. A similar static function,
`CodeRepeatedZeros`, was also updated to use `__indexable` pointers.
The caller, `VP8LCreateCompressedHuffmanTree`, passes a buffer `tokens`
with size `max_tokens`. Its signature (in .c and .h files) was
annotated to reflect this using `__counted_by(max_tokens)`. Because
`__counted_by` requires the size parameter to be updated alongside the
pointer if the pointer is modified, the implementation of
`VP8LCreateCompressedHuffmanTree` was refactored. Instead of modifying
the `tokens` parameter directly, a local iterator variable
`current_token` was introduced and explicitly annotated as
`__indexable` to ensure correct type propagation, especially when
`WEBP_ASSUME_UNSAFE_INDEXABLE_ABI` is active. This local iterator is
used for calls to `CodeRepeatedValues` and `CodeRepeatedZeros`.
Bug: 432511821
Change-Id: I07fe553341a613a0ea4d5284817098c31f3aefeb
Reasoning:
The `fbounds-safety` compiler extension reported out-of-bounds accesses
on the `buf` member of the `VP8BitWriter` struct (defined in
`src/utils/bit_writer_utils.h`, line 36). These occurred in
`src/utils/bit_writer_utils.c` at lines 70, 74, 76, and 189, where
`buf` was used with array indexing or pointer arithmetic despite being a
`__single` pointer by default.
To fix this, the `buf` member was annotated as
`__sized_by_or_null(max_pos)` in `src/utils/bit_writer_utils.h`
(line 36), associating it with the `max_pos` member which stores the
buffer size.
This annotation introduced a new build error in the `BitWriterResize`
function (`src/utils/bit_writer_utils.c`, line 55) when assigning
the result of `WebPSafeMalloc` (an `__unsafe_indexable` pointer) to the
now-annotated `bw->buf`. This was resolved by:
1. Using `bw->buf = __unsafe_forge_bidi_indexable(uint8_t*, new_buf,
new_size);` (line 55) to create a properly bounded pointer from the
`malloc` result (`new_buf`) using its size (`new_size`) before
assigning it to `bw->buf`.
Bug: 432511821
Change-Id: I1a24a9a432388ccce53a5e9b3b5e58d22aa61d0c
WEBP_SELF_ASSIGN exists to clearly designate when a self-assignment is
intentional for the purposes of -fbounds-safety support. It also exists
to silence any compiler warnings that may exist when building without
-fboudnds-safety.
Sample showing behavior across the three major toolchains (clang, gcc,
msvc): https://godbolt.org/z/6r99dncW7
Change-Id: Ibbad5c0c3348dd6e2c4a70297b3e9cb6cf0d2817
Reasoning:
Analysis of the `VP8LBitReader` struct and its initialization function
`VP8LInitBitReader` confirmed that the `len` member holds the size of
the buffer pointed to by `buf`. Therefore, `__counted_by(len)` was added
to `buf` in the struct definition.
This introduced new errors related to function parameters being treated
as `__single` and the `__counted_by` side-by-side assignment rule:
1. In `VP8LInitBitReader`, the `start` parameter was indexed
(`start[i]`) but was implicitly `__single`. It was annotated with
`__counted_by(length)` in both the definition and prototype.
2. `br->buf = start` was moved immediately after `br->len = length`.
3. In `VP8LBitReaderSetBuffer`, the `buf` parameter was assigned to
`br->buf`, which requires the parameter to also be annotated.
`__counted_by(len)` was added to the `buf` parameter in the
definition and `__counted_by(length)` to the `buffer` parameter in
the prototype.
Bug: 432511821
Change-Id: Ie2d5b7b321fb02e8c6b6d3cbd933d056b1bb82cf
Reasoning:
Analysis showed `bit_depths` is passed from `VP8LCreateHuffmanTree` (as
`huff_code->code_lengths`) to `GenerateOptimalTree` (as `bit_depths`
with size `histogram_size` = `huff_code->num_symbols`) and then to
`SetBitDepths`. The `HuffmanTreeCode` struct stores `code_lengths` and
`codes` pointers, both sized by `num_symbols`. These arrays are
allocated in `GetHuffBitLengthsAndCodes` (called by
`EncodeImageInternal`) based on `num_symbols`.
The fix involves:
- Annotating `HuffmanTreeCode::code_lengths` and
`HuffmanTreeCode::codes` with `__counted_by(num_symbols)` in
`src/utils/huffman_encode_utils.h`.
- Annotating the `bit_depths` parameter in `GenerateOptimalTree` with
`__counted_by(histogram_size)` in `src/utils/huffman_encode_utils.c`.
- Annotating the `bit_depths` parameter in `SetBitDepths` with
`__indexable` in `src/utils/huffman_encode_utils.c`, as the size
parameter (`histogram_size`) is not directly available but indexing is
known to be safe based on caller logic (indices `tree->value` are
within `[0, histogram_size - 1]`).
Bug: 432511821
Change-Id: Icfd32f15d0744983b5912d527e5bc59ac58343a5
Reasoning:
The function `VP8LInitBitReader` in `src/utils/bit_reader_utils.c`
takes a pointer `start` and a `length`. Inside the function, `start`
is accessed in a loop (lines 167-168) with index `i` ranging from 0
up to a potentially modified `length` (capped at `sizeof(br->val)` on
lines 164-165). The original `length` parameter accurately describes
the intended size of the buffer pointed to by `start` before this
capping occurs. Therefore, `start` is annotated with
`__counted_by(length)` in both its definition
(src/utils/bit_reader_utils.c:151) and declaration
(src/utils/bit_reader_utils.h:157) to reflect this relationship and
resolve the array subscript error.
Bug: 432511821
Change-Id: Ibefe213e8011ca9b0f6ea4f22651b866261153c5
This reverts commit 2246828be3.
Reason for revert: NULL dereferences in the fuzzers.
The `VP8LPredictorsAdd_SSE` table is not completely initialized (index
14 and 15) which may be accessed with an invalid bitstream.
Bug: 435213378
Original change's description:
> dsp/lossless{,_enc}_sse2.c: reorder *_SSE assignments
>
> When `WEBP_USE_THREAD` is not defined the assignments of *_SSE and their
> unsuffixed counterparts may race. Assigning *_SSE directly rather than
> relying on the unsuffixed values avoids a case where the *_SSE variants
> may refer to the calling function (i.e., AVX2) resulting in infinite
> recursion.
>
> Defining `WEBP_USE_THREAD` is recommended when decode/encode calls can
> be made from different threads.
>
> Bug: 435213378
> Change-Id: Id5549730cb72be99b3014ed8e4e355f3ea988659
Bug: 435213378, 438295348, 438294044, 438264629, 438294033
Change-Id: I3299d6fbb29c45872e2ea1f8f1c3d0ebbda64a69
When `WEBP_USE_THREAD` is not defined the assignments of *_SSE and their
unsuffixed counterparts may race. Assigning *_SSE directly rather than
relying on the unsuffixed values avoids a case where the *_SSE variants
may refer to the calling function (i.e., AVX2) resulting in infinite
recursion.
Defining `WEBP_USE_THREAD` is recommended when decode/encode calls can
be made from different threads.
Bug: 435213378
Change-Id: Id5549730cb72be99b3014ed8e4e355f3ea988659
SRWLOCK is faster than CRITICAL_SECTION. MSVC implements the C11 mtx_t
and C++ std::mutex using SRWLOCK.
Change-Id: I144d2f53aeaa135030ea8c057a2ae8543e6c91b8
Assume the CONDITION_VARIABLE added in Windows Vista is available.
Remove an unneeded WaitForSingleObject() macro that converts
WaitForSingleObject() calls to WaitForSingleObjectEx() calls with
bAlertable=FALSE. The WaitForSingleObject() function does not enter an
alertable wait state, so it is equivalent to WaitForSingleObjectEx()
with bAlertable=FALSE.
Remove code for Windows older than Vista in src/dsp/cpu.h.
Change-Id: I7df95557713923e05a7bfb62e095ec6172cfd708
Import bounds_safety.h across all of webputils, with one exception being
dsp.h, since it's imported by webputils.h in one place. Also prepend
WEBP_ASSUME_UNSAFE_INDEXABLE_ABI to every webputil file to indicate to
the compiler that every pointer should be treated as __unsafe_indexable.
We also need to replace memcpy/memset/memmove with the unsafe variants
WEBP_UNSAFE_*, as memcpy/memset/memmove require bounded/sized pointers.
With this change, all of libwebputils (and libwebp) should build with
-DWEBP_ENABLE_FBOUNDS_SAFETY=true
Change-Id: Iad87be0455182d534c074ef6dc1a30fa66b74b6c
A slim reader/writer (SRW) lock can be initialized statically with the
constant SRWLOCK_INIT. It is the only Windows synchronization object I
can find with this property.
Note: On old Windows versions that don't have SRWLOCK, use the fallback,
thread-unsafe implementation.
Change a NOLINT comment to a NOLINTNEXTLINE comment to prevent
clang-format from aligning the #else and #endif comments in undesired
way.
Bug: 435213378
Change-Id: Iecff615a14a1905aedd2c05ad9444889f711cc17
Function static variables are initialized on the first call to the
function. In C the initialization of function static variables is not
thread-safe. Use file static variables instead in the
WEBP_DSP_INIT_FUNC() macro.
Remove the volatile qualifier for the pthread version of the
func##_last_cpuinfo_used variable because the variable is only accessed
while holding the mutex.
Change-Id: I1237904a49d2467d7ce79fc53f9e7f966aa7a5c1
(Debian clang-format version 19.1.7 (3+build4)) with `--style=Google`.
Manual changes:
* clang-format disabled around macros with stringification (mostly
assembly)
* some inline assembly strings were adjusted to avoid awkward line
breaks
* trailing commas, `//` or suffixes (`ull`) added to help array
formatting
* thread_utils.c: parameter comments were changed to the more common
/*...=*/ style to improve formatting
The automatically generated code under swig/ was skipped.
Bug: 433996651
Change-Id: Iea3f24160d78d2a2653971cdf13fa932e47ff1b3
Rather than encoding all candidates then finding the smallest output,
only keep the frame candidate leading to the shortest bitstream at all
times. Some memory is released slightly sooner when there are more
than two frame candidates.
The simple assignments in PickBestCandidate() are now done for each
new best candidate so far instead of once for the one best overall
candidate, but they are cheap and overwritten. This change should
have no impact on encoded animation files.
Change-Id: Ie540f104a70de7a84535f8f2c49d1a71fa5b4bfa
When the next frame is forced to be a keyframe by setting
enc->best_delta to DELTA_INFINITY, setting enc->keyframe to NONE means
the current frame will never be converted from a candidate keyframe to
a subframe. So enc->prev_candidate_undecided should be set to 0.
This is a no-op change because enc->prev_candidate_undecided is only
used to update prev_rect and dispose method, which is of no value when
the next frame is a key frame.
Change-Id: Ie51c7e3b6ba797b75af5e06439e8568d8185693f
The macros in src/utils/bounds_safety.h exist to ensure libwebp
stays portable. This also provides wrappers around common functions like
memcpy/memset/memmove, which are useful as part of the migration. Memcpy
(et al.) wrappers aren't expected to exist forever, and can be removed
after the codebase is fully annotated.
There's some complexity here due to the number of states we need to
support:
1) Off everywhere
2) Building libwebp (some or all) with -fbounds-safety
3) Linking against libwebp that was built with -fbounds-safety
4) Inter-operation with C++
Change-Id: I789f0a94f25b70cab172d5b3f5e6b12de3a34bb4
Rename them to WebPConvertRGBToY/WebPConvertBGRToY and accept the
'step' parameter (3 for RGB, 4 for ARGB).
Change-Id: I930a23894e4135a34fff2174e6a5bbee1eac2ba0
libwebp-1.6.0
- 6/30/2025 version 1.6.0
This is a binary compatible release.
API changes:
- libwebp: WebPValidateDecoderConfig
* additional x86 (AVX2, SSE2), general optimizations and compression
improvements for lossless
* `-mt` returns same results as single-threaded lossless (regressed in
1.5.0, #426506716)
* miscellaneous warning, bug & build fixes (#393104377, #397130631,
#398288323, #398066379, #427503509)
Tool updates:
* cwebp can restrict the use of `-resize` with `-resize_mode` (#405437935)
Bug: webp:427525168
* tag 'v1.6.0':
update ChangeLog
webp_js/README.md: add some more code formatting (``)
CMakeLists: add warning for incorrect emscripten config
update ChangeLog
api.md: add WebPValidateDecoderConfig to pseudocode
update NEWS
bump version to 1.6.0
update AUTHORS
Change-Id: Ia4962eff9c197c42c77c9eadd35cdeee3586510e
And provide a clear comment explaining why the index of offset[] is
always checked within bounds.
Bug:webp:622
Change-Id: Id9b973a804b74c53dfb291f1a9dae649c0daed9d
This is a refinement of
654bfb04 Avoid nullptr arithmetic in VP8BitReaderSetBuffer
and removes an unneeded/redundant check in 2 of the 3 calls to this
function:
* VP8InitBitReader: `start` is guaranteed to be non-NULL
* CopyParts0Data: `start` is allocated and checked
In `DoRemap()` `last_start` will be NULL before the partitions are
parsed. This is the only call that was missing a check. The offsetting
of a NULL pointer in `VP8BitReaderSetBuffer` was harmless in this case
as the bitreader will not be used meaningfully until there is enough
data to begin decoding partition 0. In that case the bitreader will be
initialized by `ParsePartitions()` and updated by `DoRemap()` when more
data is available.
Bug: 393104377
Change-Id: Ib44bc35e00e5129c592d742a2469420cd3d0e858
enc cross_color_transform_bits and predictor_transform_bits were
modified between configurations, leading to inconsistent results.
Change-Id: I42809495a63dbacdda977ecbcc98d8de63d51184
This reduces manual offsetting inside a large chunk of memory to
hit the right histogram and replaces with types for the histogram
buckets and a container Histograms.
Change-Id: I1f80fcc2da38cadd9e4bc57d0693ed11dc5b3581
Fixes:
dsp/enc_neon.c:1192:11: warning: implicit declaration of function
'vld1_u8_x2'; did you mean 'vld1_u32'? [-Wimplicit-function-declaration]
inner = vld1_u8_x2(top);
^~~~~~~~~~
vld1_u32
Change-Id: I8d0175561efd69bc9614a68dca1d0fc19cdf91be
Semi-automatically taking the the misc-include-cleaner warnings
by clang-tidy and fixing files to be self-contained.
Change-Id: Iaaa2b2ec9d6dcce547fa5cb6b4f056dfc8c781ff
Empty histograms or one of two merged histograms were set to NULL.
That made the code harder to understand.
This changes the order of the histograms and therefore the goldens,
but at the noise level.
Change-Id: I1702637bdcdbaaad1244a1345ca5297459f61132
- move HistogramAdd to histogram_enc.cc: it is too high level
- homogenize the argument naming (e.g. h for histogram, p for
population)
- separate a bit the data from the stats (only used within
VP8LGetHistoImageSymbols)
Change-Id: I274546e3ff96297383bcae0a95696c11f18decbf
For now, this is used for histograms where A,R,B are
trivial. This can be done on a per-symbol basis for
speed-ups.
Only the entropy bin merge criterion is kept with
A,R,B to not create speed regressions (but compression
improvements).
Change-Id: Iaff6f6d5f157066e481bf43553ea5edd01ff1cde
After:
44f91b0d Speed DispatchAlpha_SSE2 up
_mm_set1_epi8 takes a char argument; add a `char` cast for 0xff.
from clang-14 integer sanitizer:
implicit conversion from type 'int' of value 255 (32-bit, signed) to
type 'char' changed the value to -1 (8-bit, signed)
Change-Id: I0f4ed092eddc0beb311f44bf3d4b74a4d1177040
This is a follow up to:
ee8e8c62 Fix member naming for VP8LHistogram
This better matches Google style and clears some clang-tidy warnings.
This is the final change in this set. It is rather large due to the
shared dependencies between dec/enc.
Change-Id: I89de06b5653ae0bb627f904fa6060334831f7e3b
This is a follow up to:
ee8e8c62 Fix member naming for VP8LHistogram
This better matches Google style and clears some clang-tidy warnings.
Change-Id: Ie2f82401e1ba28bd0575b6bb82d12ed55c71718f
