Memcmp is used in other parts of libwebp which we didn't see yet in
webputils. The places that use memcp use it as an expression, (i.e. if
(memcmp(...)), so explicitly do not wrap in do { } while (0) guard.
Change-Id: Ifa972640bae052717a1af02112c62dc197dfe0f1
Note that config.exact defaults to 0 and point users to WebPEncode() if
the default isn't acceptable. This duplicates the comment from encode.h
for visibility. Follow up to:
6d2e11ec encode.h: mention 'exact' default in WebPEncodeLossless*
Bug: 449296530
Change-Id: Iba44dbcbb179bd80ea0a6b9e129b2c7e0a406916
There was a duplicated functionality with a lower quality which
could lead to decoded lossless WebP to YUV being different from
lossless WebP to PNG to YUV.
The rescaler is not using it yet.
Bug: 432241412
Change-Id: Id794880957935b69729d4b34ae453551d13364dc
The armv7, armv7s and i386 iOS targets were deprecated in Xcode 14 [1]
and failed to build with Xcode 16.4.
[1]: https://developer.apple.com/documentation/xcode-release-notes/xcode-14-release-notes
Building iOS projects with deployment targets for the armv7, armv7s, and
i386 architectures is no longer supported. (92831716)
Change-Id: I08d376fea64638d056258798bd7e586ca6880454
Reasoning:
Image Data Buffers:
The `data` parameter of `WebPDequantizeLevels` (in both .c and .h)
and `InitParams` (src/utils/quant_levels_dec_utils.c:232) is annotated
with `WEBP_SIZED_BY((long)stride * height)`, as it points to the start
of the image buffer.
The `src` and `dst` fields in `SmoothParams`
(src/utils/quant_levels_dec_utils.c:54) are annotated as
`WEBP_INDEXABLE`. They are initialized from `data` in `InitParams`
(L266) and are advanced row by row using pointer arithmetic (e.g.,
`p->src += p->stride` in `VFilter` L111, `p->dst += p->stride` in
`ApplyFilter` L165). `WEBP_INDEXABLE` is used because the pointers
iterate within the buffer and are only accessed with positive indices.
Scratch Buffers (`SmoothParams`):
Scratch buffers are allocated in `InitParams` via `WebPSafeMalloc`.
The local variable `mem` holding this allocation (L245) is explicitly
annotated as `WEBP_BIDI_INDEXABLE` to ensure safety when compiling with
error suppression.
- `start`, `cur`, `top`: These pointers are used for iteration and
pointer arithmetic within the circular scratch buffer. They are
annotated as `WEBP_INDEXABLE`.
- `end`: This pointer is annotated as `WEBP_BIDI_INDEXABLE` because it
is used in subtraction (`p->end - width`) in `InitParams` (L257) to
calculate `p->top`.
- `average`: This buffer is accessed sequentially up to `width`. It is
annotated as `WEBP_COUNTED_BY(width)`. Initialization in `InitParams`
is reordered (L261) to ensure `p->width` is set before `p->average`.
- `correction`: This lookup table requires negative indexing. To avoid
using `WEBP_BIDI_INDEXABLE` in the struct, it is annotated as
`WEBP_COUNTED_BY_OR_NULL(CORRECTION_LUT_SIZE)` (L75), pointing to the
start of the buffer. `CORRECTION_LUT_SIZE` is defined (L33).
`InitCorrectionLUT` (L188) and `ApplyFilter` (L147) calculate a local
middle pointer which is explicitly annotated as `WEBP_BIDI_INDEXABLE`
to allow safe negative indexing.
Local Pointers:
To ensure safety when compiling with error suppression (where locals
default to unsafe), explicit annotations are added to local pointers
derived from safe struct members:
- `VFilter` (L87): `src`, `cur`, `top`, `out` are `WEBP_INDEXABLE`.
- `HFilter` (L121): `in`, `out` are `WEBP_INDEXABLE`.
- `ApplyFilter` (L145): `average`, `dst` are `WEBP_INDEXABLE`.
- `CountLevels` (L214): `data` is `WEBP_INDEXABLE`.
Bug: 432511821
Change-Id: I6bdf86f80c94a5b182c5aef7e4092fe4ea24afb8
Reasoning:
In `HuffmanTablesSegment` (`src/utils/huffman_utils.h`), `start`
was annotated `WEBP_COUNTED_BY_OR_NULL(size)` as it points to an
allocation of `size` elements. `curr_table` was annotated
`WEBP_UNSAFE_INDEXABLE` because it iterates within `[start, start
+ size)`, a bound that cannot be expressed statically in the struct
without ABI changes. The code manually checks bounds for
`curr_table` (e.g., `src/utils/huffman_utils.c:240-241`). To
support the annotation on `start`, allocation sites in
`VP8LBuildHuffmanTable` and `VP8LHuffmanTablesAllocate`
(`src/utils/huffman_utils.c`) were refactored to assign `start` and
`size` side-by-side, using `WEBP_BIDI_INDEXABLE` local variables to
hold the safe pointer returned by `WebPSafeMalloc`.
`VP8LHuffmanTablesDeallocate` was updated to set `size` to 0 when
`start` is freed.
The `root_table` parameter of `BuildHuffmanTable`
(`src/utils/huffman_utils.c:86`) was annotated `WEBP_BIDI_INDEXABLE` to
accommodate accesses to secondary tables beyond the `root table` size
since with explicitly annotating the local variable `table`
as `WEBP_BIDI_INDEXABLE`, `table` inherits its bounds from `root_table`.
Call sites in `VP8LBuildHuffmanTable`
required `WEBP_UNSAFE_FORGE_BIDI_INDEXABLE` to convert the unsafe
`curr_table` to the safe `root_table`.
The `table` parameter of `ReplicateValue`
(`src/utils/huffman_utils.c:59`) was annotated
`WEBP_COUNTED_BY(end - step + 1)` and the function was refactored to
avoid modifying `end`. Call sites in `BuildHuffmanTable` required
`WEBP_UNSAFE_FORGE_BIDI_INDEXABLE` because the strided access
patterns used for Huffman table construction cannot be statically
verified by the compiler.
Bug: 432511821
Change-Id: I77c5c82ac36bc9bb79cd5119a4113ac5d62af762
Memory footprint is increased by twice the canvas pixel count in bytes
at encoding. There should be little impact on encoding speed because
only buffer allocs/reads/writes are introduced, with little to no
added logic. Animation encoding may be 2% slower.
Bug: 42340478
Change-Id: I8f0048107a2bfbee7a8124c100f78eac93447d80
The calling function, `GetFilterMap()` only takes `width` and `height`;
the alpha data is assumed to have a stride equal to its width. The
`WebPEstimateBestFilter()` was inconsistently using the parameters,
setting up the current row with `stride`, but accessing the previous one
with `width`.
Change-Id: I9dd90222b6923eea3626e426a61bdef3985546ff
Reasoning:
The `irow` and `frow` pointers in `WebPRescaler`
(src/utils/rescaler_utils.h:49) were annotated with
`WEBP_COUNTED_BY(dst_width * num_channels)`. This is based on their
initialization in `WebPRescalerInit` (src/utils/rescaler_utils.c:82-83)
where they are assigned parts of the `work` buffer, whose total size
is `2 * dst_width * num_channels`. The `work` parameter in
`WebPRescalerInit` (src/utils/rescaler_utils.h:58,
src/utils/rescaler_utils.c:33) was also annotated accordingly.
To satisfy the side-by-side assignment requirement for external bounds,
assignments to `rescaler->irow` and `rescaler->frow` in
`WebPRescalerInit` were moved closer to the assignments of
`dst_width` and `num_channels` (src/utils/rescaler_utils.c:50-53).
Since `work` have bound information, `WEBP_UNSAFE_MEMSET` has
been changed to `memset`.
In `WebPRescalerImport` (src/utils/rescaler_utils.c:140-150), where
`irow` and `frow` are swapped, self-assignments for `dst_width` and
`num_channels` were added side-by-side with the pointer assignments.
Additionally, `WEBP_UNSAFE_FORGE_BIDI_INDEXABLE` was used for the
pointer assignments to handle the `WEBP_ASSUME_UNSAFE_INDEXABLE_ABI`
setting used during testing.
Bug: 432511821
Change-Id: If716fb79a06dee9e807eff060806daf038810523
This was necessary after:
44257cb8 apply clang-format
Which made some single-line statements into multi-line. Using braces on
multi-line statements better conforms with the Google style guide.
Bug: 433996651
Change-Id: I615c0ecf3b94571f67fceadfe8c15914aea45ccb
Reasoning:
The `sorted` parameter in `BuildHuffmanTable`
(`src/utils/huffman_utils.c:87`) is annotated with
`WEBP_COUNTED_BY_OR_NULL(code_lengths_size)`. Analysis of the
access patterns (lines 137, 177, 207) shows that the indices used
are bounded by `code_lengths_size`. Since `sorted` can be NULL,
`_OR_NULL` is used. When compiling, calls
to `BuildHuffmanTable` in `VP8LBuildHuffmanTable` (line 272) required forging bounds because the `sorted` buffer, allocated
via `WebPSafeMalloc` or on the stack, was treated as unsafe.
`WEBP_UNSAFE_FORGE_BIDI_INDEXABLE` is used at the call sites to
provide the necessary bounds information.
Bug: 432511821
Change-Id: I6fea3ac5d77cb56139f9748ba0277a4f0ad21737
Reasoning:
The `data` pointer in `WebPEstimateBestFilter` is annotated with
`WEBP_COUNTED_BY((size_t)height * width)`. This reflects the
allocation size found by tracing the call graph back to
`EncodeAlpha` (src/enc/alpha_enc.c:332), where the buffer is
allocated using `WebPSafeMalloc` with a size of `width * height`.
Although the function signature includes a `stride` parameter used
for pointer arithmetic (`data + j * stride` at
src/utils/filters_utils.c:44), the implementation implicitly assumes
`stride == width` for vertical and gradient filtering logic (e.g.,
accesses like `p[i - width]` at line 49). The only caller
(src/enc/alpha_enc.c:220) respects this assumption by passing `width`
as the `stride`. Therefore, `height * width` accurately represents
the required buffer size.
Bug: 432511821
Change-Id: I7d308cf8dcccd7d6128a17dbc0f7b177ee9282a1
webpdecodeutils is a subset of webputils, so we add -fbounds-safety to
it to ensure that the files are being compiled the same way between
modules.
Bug: webp:432511821
Change-Id: I5c01a87601a13c331b628c605238e645d1efa77f
Reasoning:
The `palette` parameter in `PaletteSortModifiedZeng` (src/utils/palette.c)
was accessed using `palette[i]` in a loop from 0 to `num_colors` (line
386), indicating it's an array of size `num_colors`. The `palette_in`
parameter was also deduced to be of size `num_colors` based on its
usage and how the indices accessing it are generated. Therefore, both
`palette` and `palette_in` were annotated with
`WEBP_COUNTED_BY(num_colors)`. This change required updating the caller
function `PaletteSort` (lines 393-395 in src/utils/palette.c and lines
61-63 in src/utils/palette.h) to match the new signature by adding the
same annotations to its `palette_sorted` and `palette` parameters.
The `palette` parameter in
`PaletteHasNonMonotonousDeltas` was being indexed like an array but was
typed as a `WEBP_SINGLE` pointer. Since `palette` is indexed up to
`num_colors`, it was annotated with `WEBP_COUNTED_BY(num_colors)`. This
introduced a warning at the call site in `PaletteSortMinimizeDeltas`
(src/utils/palette.c:197), as it passed a `WEBP_SINGLE` pointer
(`palette_sorted`) where `WEBP_COUNTED_BY` was expected. Analysis showed
both `palette_sorted` and `palette` parameters in
`PaletteSortMinimizeDeltas` are accessed up to `num_colors`, so they were
annotated with `WEBP_COUNTED_BY(num_colors)`.
The `sorted` parameter in `SearchColorNoIdx`
was annotated with `__counted_by(num_colors)` in both the definition
(src/utils/palette.c:68) and declaration (src/utils/palette.h:40).
This change led to cascading errors during testing.
These errors occurred because callers
passed pointers that were considered `__unsafe_indexable` under this
setting. To resolve this, the following functions were also
annotated:
- `PrepareMapToPalette` (src/utils/palette.c:85, src/utils/palette.h:46):
`palette`, `sorted`, and `idx_map` were annotated with
`__counted_by(num_colors)`.
The pointer `cooccurrence` in `src/utils/palette.c` is used as a
flattened 2D array of size `num_colors * num_colors` in functions
`CoOccurrenceFindMax` (lines 226-252) and `CoOccurrenceBuild` (lines
255-300). The parameters in these functions were annotated with
`WEBP_COUNTED_BY(num_colors * num_colors)` to reflect this usage and
fix the original bounds safety errors during indexing.
In the caller function `PaletteSortModifiedZeng` (lines 307-391),
`cooccurrence` is a local variable allocated using `WebPSafeCalloc`.
To ensure compatibility with the annotated parameters, especially
during test builds where local variables appeared to be treated as
unsafe pointers, `WEBP_UNSAFE_FORGE_BIDI_INDEXABLE` was used when
passing `cooccurrence` to `CoOccurrenceBuild` (line 327) and
`CoOccurrenceFindMax` (line 333). This ensures a pointer with the
correct bounds is passed to the callees in all build configurations.
Bug: 432511821
Change-Id: I7540968ecca67645c5ca57e542433971b235e582
Previously, WebPAnimEncoder::prev_rect was set in
SetFrame()>PickBestCandidate(), and SetFrame() may be called twice in
a row in CacheFrame(), making prev_rect value wrong in the second
SetFrame() call. Fortunately, the second call is always for a
keyframe, and in that case SetFrame() does not use the value of
prev_rect.
Since commit b1feaa4 that may have been a bigger issue as
PickBestCandidate() was moved from the end of SetFrame() to
SetFrame()>GenerateCandidates()>PickBestCandidate(), effectively
changing the value of prev_rect for non-keyframe candidates within the
same SetFrame() call. Fortunately, prev_rect is only used at the
beginning of SetFrame(), before any call to GenerateCandidates(),
hence the no-op change.
Change-Id: I693dbf9d264b8768d25a29d75b511e616a3db56b
Reasoning:
The `data` parameter in `QuantizeLevels` (defined in
`src/utils/quant_levels_utils.c` line 33) was causing bounds safety
errors because it was accessed using array subscripts (e.g., `data[n]`
at lines 60-63 and 137) but was typed as a single pointer. The size of
the buffer pointed to by `data` is determined by the `width` and
`height` parameters, calculated as `data_size = height * width` at
line 39. The loops accessing `data` iterate up to `data_size`. To fix
this, the `data` parameter in both the function definition and its
declaration (`src/utils/quant_levels_utils.h` line 30) was annotated
with `WEBP_COUNTED_BY((size_t)width * height)`.
Bug: 432511821
Change-Id: Idfe8810eaaf1239d86e38eb661a1a987d817127c
Reasoning:
The function `NextTableBitSize` is only called by `BuildHuffmanTable`
(src/utils/huffman_utils.c:196). In `BuildHuffmanTable`, `count` is
defined as a local array `int count[MAX_ALLOWED_CODE_LENGTH + 1]`
(line 93). This array decays to a pointer when passed to
`NextTableBitSize`. The maximum index accessed within `NextTableBitSize`
is `MAX_ALLOWED_CODE_LENGTH - 1` (line 75, inside a loop where `len`
starts <= `MAX_ALLOWED_CODE_LENGTH` and increments up to
`MAX_ALLOWED_CODE_LENGTH`).
Since the caller provides an array of size `MAX_ALLOWED_CODE_LENGTH + 1`
and the accesses are within bounds, the `count` parameter in
`NextTableBitSize` is annotated with
`WEBP_COUNTED_BY(MAX_ALLOWED_CODE_LENGTH + 1)`. This resolves the
compiler error.
Bug: 432511821
Change-Id: I29a55dd7f09c04aa3a2394996cf0e123d985fcd0
Reasoning:
The `histogram` parameter in `GenerateOptimalTree`
(src/utils/huffman_encode_utils.c:170) is used as an array indexed up
to `histogram_size`. It was annotated with
`__counted_by(histogram_size)` to fix bounds-safety errors. This
required annotating the caller `VP8LCreateHuffmanTree`
(src/utils/huffman_encode_utils.c:411). However, the size of
`histogram` in `VP8LCreateHuffmanTree` depends on another parameter
(`huff_code->num_symbols`), preventing direct annotation in the
signature. Instead, a local `__bidi_indexable` pointer
(`bounded_histogram`) was forged inside `VP8LCreateHuffmanTree` using
`__unsafe_forge_bidi_indexable` with the correct size and passed to
`GenerateOptimalTree`. This also required annotating parameters
`good_for_rle` and `counts` in `OptimizeHuffmanForRle`
(src/utils/huffman_encode_utils.c:37) and forging a bounded pointer
for `buf_rle` (`bounded_buf_rle`) in `VP8LCreateHuffmanTree` to pass
to `OptimizeHuffmanForRle`.
Bug: 432511821
Change-Id: I5905ff93f4cc0a9ff03c5786b5ac20c6bfa965ff
Reasoning:
The `palette` parameter in `GetColorPalette` (src/utils/palette.c:100)
was annotated with `WEBP_COUNTED_BY_OR_NULL(MAX_PALETTE_SIZE)` to fix
an array subscript error at src/utils/palette.c:146. This annotation
reflects the function's contract, documented in src/utils/palette.h,
which states that if `palette` is not NULL, it must point to memory
allocated for at least `MAX_PALETTE_SIZE` elements. To make
`MAX_PALETTE_SIZE` visible, `src/webp/format_constants.h` was included
in `src/utils/palette.h`. Consequently, the wrapper function
`WebPGetColorPalette` (src/utils/utils.c:273) and its declaration in
`src/utils/utils.h` were also annotated similarly, requiring the
inclusion of `src/webp/format_constants.h` in `src/utils/utils.h` as
well. This resolved a compilation error caused by the type mismatch
when calling the annotated `GetColorPalette` from `WebPGetColorPalette`.
Bug: 432511821
Change-Id: Iceebf2facf9558dd49889f056f028d9a3fb22d41
Reasoning:
The struct `VP8LBitWriter` in `src/utils/bit_writer_utils.h` uses
`buf`, `cur`, and `end` pointers to manage a dynamic buffer. Pointer
arithmetic on `buf` and `cur` caused bounds safety errors.
Initial attempts using `WEBP_ENDED_BY(end)` for both `buf` and `cur`
failed due to compiler limitations (one field cannot be the upper
bound for multiple other fields). Using `WEBP_INDEXABLE` for `buf` and
`cur` resolved the arithmetic errors but caused ABI incompatibility
issues, as `VP8LBitWriter` is used in multiple compilation units with
different settings.
The final approach annotates `buf` with `WEBP_ENDED_BY(end)` and `cur`
with `WEBP_UNSAFE_INDEXABLE`. This resolves the arithmetic errors for
both pointers without changing the ABI. `WEBP_UNSAFE_INDEXABLE` is
used for `cur` as a workaround for the `WEBP_ENDED_BY` limitation and
ABI constraints.
Additionally, the `VP8LBitWriterResize` function in
`src/utils/bit_writer_utils.c` was modified:
- The assignments to `bw->buf`, `bw->end`, and `bw->cur` (lines
231-233) were reordered and updated to use the local
`allocated_buf` variable on the right-hand side. This satisfies the
consecutive assignment requirement imposed by `WEBP_ENDED_BY(end)`
on `bw->buf`.
- The local variable `allocated_buf` (line 207) was annotated with
`WEBP_BIDI_INDEXABLE`.
- The allocation for `allocated_buf` (line 222) now uses
`WEBP_UNSAFE_FORGE_BIDI_INDEXABLE` to create a safe, bounded
pointer from the result of `WebPSafeMalloc`, fixing potential type
mismatches when `suppress_fbounds_errors=yes`.
Bug: 432511821
Change-Id: I603a6a7d3ff3bf2a8edf9749a9898ea227c32982
Reasoning:
Analysis of `GetLE32` and its called function `GetLE16`
(src/utils/utils.h:95) showed that `GetLE32` requires access to 4
bytes starting from `data`. `GetLE16` accesses 2 bytes.
Therefore, `WEBP_COUNTED_BY(4)` was added to the `data` parameter of
`GetLE32`. Similarly, `WEBP_COUNTED_BY(2)` was added to `GetLE16`.
During this analysis, related functions `GetLE24`, `PutLE16`,
`PutLE24`, and `PutLE32` (src/utils/utils.h lines 99-123) were also
identified as needing similar annotations. They were annotated with
`WEBP_COUNTED_BY(3)`, `WEBP_COUNTED_BY(2)`, `WEBP_COUNTED_BY(3)`, and
`WEBP_COUNTED_BY(4)` respectively, based on the number of bytes they
access or modify.
Bug: 432511821
Change-Id: I5783392bc8dcaa2f346a81928ef496fc52da3a30
Reasoning:
The `pool` parameter in the static function `SetBitDepths`
(`src/utils/huffman_encode_utils.c:140`) is indexed as an array. Since
the function is static and the exact size is not available in the
signature, `pool` was annotated as `__bidi_indexable`. This required
updating the caller, `GenerateOptimalTree` (also static), where the
corresponding argument `tree_pool` is derived from the `tree` parameter.
Consequently, both the `tree` parameter and the local variable
`tree_pool` in `GenerateOptimalTree`
(`src/utils/huffman_encode_utils.c:170`) were also annotated as
`__bidi_indexable`. Finally, the call to `GenerateOptimalTree` from the
ABI-visible function `VP8LCreateHuffmanTree`
(`src/utils/huffman_encode_utils.c:417`) required adapting the
`huff_tree` pointer. Tracing the allocation in `src/enc/vp8l_enc.c:473`
revealed that `huff_tree` is allocated with enough space for
`3 * max_num_symbols`. Therefore, `__unsafe_forge_bidi_indexable` was
used to cast `huff_tree` to `__bidi_indexable` with the calculated
bounds (`3 * num_symbols`).
Bug: 432511821
Change-Id: I53c6965c3fd708256241a225ec9223085a1ec2d4
