Commit Graph

5058 Commits

Author SHA1 Message Date
James Zern
dab2cf21fa Merge "Add fbounds-safety annotations in palette.c/.h." into main 2025-08-20 15:10:00 -07:00
James Zern
b7d30cfd94 Merge "Add fbounds-safety annotations for sorted." into main 2025-08-20 15:04:27 -07:00
James Zern
17ba97c149 Merge "Add fbounds-safety annotations for data." into main 2025-08-20 14:07:31 -07:00
Arman Hasanzadeh
4cdb42070f Add fbounds-safety annotations for sorted.
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
2025-08-20 13:22:41 -07:00
Arman Hasanzadeh
baf42a5876 Add fbounds-safety annotations for data.
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
2025-08-20 13:04:51 -07:00
mxms
7ee251d3fd Add -fbounds-safety to webpdecodeutils
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
2025-08-20 12:47:15 -07:00
James Zern
69c8056c7a Merge "Add fbounds-safety annotations for data." into main 2025-08-20 12:28:49 -07:00
James Zern
5d3a9fc55b Merge "Add fbounds-safety annotations for count." into main 2025-08-20 12:24:13 -07:00
Arman Hasanzadeh
52135b8e00 Add fbounds-safety annotations in palette.c/.h.
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
2025-08-20 08:27:00 -07:00
Yannis Guyon
ddcdfa6a42 Refactor libwebp anim_encode curr prev rectangles
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
2025-08-20 07:52:53 +00:00
Arman Hasanzadeh
ddabb66f23 Add fbounds-safety annotations for data.
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
2025-08-19 22:33:39 -07:00
Arman Hasanzadeh
f66f1ee95c Add fbounds-safety annotations for count.
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
2025-08-19 22:15:25 -07:00
James Zern
f2061209d0 Merge "Add fbounds-safety annotations for histogram." into main 2025-08-19 13:58:27 -07:00
Arman Hasanzadeh
fde90a49e4 Add fbounds-safety annotations for histogram.
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
2025-08-19 12:34:27 -07:00
James Zern
2be405c472 Merge "Add fbounds-safety annotations for palette." into main 2025-08-19 12:24:38 -07:00
James Zern
ed02bfa963 Merge "Add fbounds-safety annotations for data." into main 2025-08-19 11:58:43 -07:00
James Zern
9d690dbf06 Merge "Add fbounds-safety annotations for VP8LBitWriter." into main 2025-08-19 11:55:47 -07:00
James Zern
603d4055bc Merge "Add fbounds-safety annotations for pool." into main 2025-08-19 11:54:47 -07:00
Arman Hasanzadeh
69b9b8525e Add fbounds-safety annotations for palette.
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
2025-08-19 11:52:40 -07:00
James Zern
9b3fc5f5e8 Merge "Add fbounds-safety annotations for tokens." into main 2025-08-19 11:36:44 -07:00
Arman Hasanzadeh
30b2c593c9 Add fbounds-safety annotations for VP8LBitWriter.
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
2025-08-19 07:38:06 -07:00
Arman Hasanzadeh
46d65e4a19 Add fbounds-safety annotations for data.
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
2025-08-18 23:41:03 -07:00
Arman Hasanzadeh
10d81e1ef0 Add fbounds-safety annotations for pool.
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
2025-08-18 23:31:14 -07:00
Arman Hasanzadeh
101e2b303f Add fbounds-safety annotations for tokens.
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
2025-08-18 19:23:03 -07:00
James Zern
7903644f24 Merge "Add fbounds-safety annotations for code_lengths." into main 2025-08-18 13:28:58 -07:00
James Zern
8a2b400352 Merge "Add fbounds-safety annotations for VP8BitReader." into main 2025-08-15 19:40:01 -07:00
James Zern
fb656b44f3 Merge "Adds fbounds annotations for VP8LColorCache." into main 2025-08-15 19:38:10 -07:00
Arman Hasanzadeh
ac865676a9 Adds fbounds annotations for VP8LColorCache.
Reasoning:

Analysis of `VP8LColorCacheInit`
(src/utils/color_cache_utils.c:29) revealed that `colors` is allocated
using `WebPSafeCalloc` with a size of `1 << hash_bits`. Therefore,
`colors` was annotated with `WEBP_COUNTED_BY_OR_NULL(1u << hash_bits)`.
To support this, `WebPSafeCalloc` (src/utils/utils.h:59,
src/utils/utils.c:214) was annotated to return
`WEBP_SIZED_BY_OR_NULL(nmemb * size)`. Since `WebPSafeCalloc` returns a
local pointer that defaults to unsafe when bounds safety is suppressed,
`WEBP_UNSAFE_FORGE_BIDI_INDEXABLE` was used on the return value
(src/utils/utils.c:222). Similarly, `VP8LColorCacheInit` required
`WEBP_UNSAFE_FORGE_BIDI_INDEXABLE` when assigning the allocated pointer
to the struct field (src/utils/color_cache_utils.c:47). Finally,
`VP8LColorCacheInit` and `VP8LColorCacheClear` were modified to perform
side-by-side assignments to `colors` and `hash_bits` as required by the
`WEBP_COUNTED_BY_OR_NULL` annotation, using self-assignment for
`hash_bits` when necessary to maintain functional equivalence with the
original code.

Bug: 432511821
Change-Id: I63cb46909d883a2e8932043ac3117b05b37e8d40
2025-08-15 12:32:50 -07:00
James Zern
fa93a9bb35 Merge "Reapply "dsp/lossless{,_enc}_sse2.c: reorder *_SSE assignments"" into main 2025-08-15 12:23:49 -07:00
Arman Hasanzadeh
15b1de13da Add fbounds-safety annotations for code_lengths.
Reasoning:

The compiler reported an error in `src/utils/huffman_utils.c` at line
230 because the array parameter `code_lengths` decayed to a `__single`
pointer, which disallows pointer arithmetic. The parameter
`code_lengths_size` represents the size of the `code_lengths` array.
To fix this, the `code_lengths` parameter in the definition of
`VP8LBuildHuffmanTable` (line 230) and its declaration in
`src/utils/huffman_utils.h` (line 108) were annotated with
`WEBP_COUNTED_BY(code_lengths_size)`.

Bug: 432511821
Change-Id: Id0b32ca283bba5938320abbdbe61846e2d538c5d
2025-08-15 10:08:16 -07:00
Arman Hasanzadeh
59dae3a508 Add fbounds-safety annotations for VP8BitReader.
Reasoning:

Analysis of initialization (`VP8BitReaderSetBuffer`) and usage sites
(`src/utils/bit_reader_utils.c:59-61`,
`src/utils/bit_reader_inl_utils.h:87`,
`src/utils/bit_reader_utils.c:102`) showed:
- `buf` is the current read pointer, iterating towards `buf_end`.
- `buf_end` points one past the end of valid data (`start + size`).
- `buf_max` points to the last position for a safe read of `lbit_t`.
- All three pointers relate to the same buffer and are adjusted together
  in `VP8RemapBitReader`.

Annotations applied:
- `buf` is annotated `__ended_by(buf_end)` as it iterates up to
  `buf_end`.
- Self-assignments `br->buf_end = br->buf_end;` are added where
  `br->buf` is modified to satisfy the compiler check for `__ended_by`
  when the pointer is updated but the end bound remains the same.
- `buf_max` is annotated `__unsafe_indexable` because its arithmetic in
  `VP8RemapBitReader` couldn't be easily expressed with other
  annotations without ABI changes, and it's mainly adjusted during
  setup/remapping.
- The `start` parameters in `VP8BitReaderSetBuffer`
  (`src/utils/bit_reader_utils.h:116`,
  `src/utils/bit_reader_utils.c:34`) `VP8InitBitReader`
  (`src/utils/bit_reader_utils.h:113`, and
  `src/utils/bit_reader_utils.c:45`) are annotated `__counted_by(size)`
  because they receive a buffer start and size, and `start` using
  `size` within `VP8BitReaderSetBuffer`. This resolved errors
  arithmetic is performed on within the function and a subsequent
  warning when calling it from `VP8InitBitReader`.

Bug: 432511821
Change-Id: Ica436d9764b5bd1f7dcf1b54a280b6f60ecef4df
2025-08-15 08:39:12 -07:00
James Zern
3f96cbffa2 Merge "Add fbounds-safety annotations for VP8BitWriter." into main 2025-08-14 20:43:58 -07:00
James Zern
903b6d816f Merge "configure: add -Wself-assign" into main 2025-08-14 16:15:30 -07:00
Arman Hasanzadeh
cdaac01490 Add fbounds-safety annotations for VP8BitWriter.
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
2025-08-14 14:06:51 -07:00
James Zern
456e2cbce1 VP8LInitBitReader: remove dead store
`br->val` was initialized to 0 followed later by `value` without being
read in between.

Change-Id: Ifdd06becb5a0de0ffb2641102c73985b077ed778
2025-08-14 13:30:44 -07:00
James Zern
de6aee46e1 Reapply "dsp/lossless{,_enc}_sse2.c: reorder *_SSE assignments"
This reverts commit 61e5c391d6.

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.

In the previous commit (2246828b) not all indices of
`VP8LPredictorsAdd_SSE[]` were assigned a value, namely 14 and 15. These
are used in handling invalid bitstreams to avoid a branch in a hot
function. The indices are now assigned to `PredictorAdd0_SSE2` which
mimics `VP8LPredictorsSub[]` in lossless_enc_sse2.c.

Bug: 435213378, 438295348, 438294044, 438264629, 438294033
Change-Id: I3623717597f0ac6b0d60429adfbb20c611fe6742
2025-08-14 13:22:56 -07:00
James Zern
f52d646039 Merge "Add fbounds-safety annotations for bit_depths." into main 2025-08-14 12:50:36 -07:00
James Zern
9ab7a640e4 Merge "Add fbounds-safety annotations for VP8LBitReader." into main 2025-08-14 12:48:30 -07:00
James Zern
011c600dca Merge "Add WEBP_SELF_ASSIGN macro" into main 2025-08-14 12:47:53 -07:00
James Zern
9769bb767c configure: add -Wself-assign
Bug: 432511225
Change-Id: I4e074909221ac4c0a26613d4441ea00c430bcb94
2025-08-14 12:03:57 -07:00
mxms
47a744d582 Add WEBP_SELF_ASSIGN macro
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
2025-08-14 18:55:46 +00:00
James Zern
7d857a1edc Merge "Add fbounds-safety annotations for start." into main 2025-08-14 11:47:20 -07:00
Arman Hasanzadeh
19f28b7889 Add fbounds-safety annotations for VP8LBitReader.
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
2025-08-14 11:43:26 -07:00
Arman Hasanzadeh
4cc27eb808 Add fbounds-safety annotations for bit_depths.
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
2025-08-14 11:37:06 -07:00
Arman Hasanzadeh
6805c246e3 Add fbounds-safety annotations for start.
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
2025-08-13 17:18:01 -07:00
James Zern
61e5c391d6 Revert "dsp/lossless{,_enc}_sse2.c: reorder *_SSE assignments"
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
2025-08-13 17:10:20 -07:00
James Zern
2246828be3 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
2025-08-07 13:50:16 -07:00
James Zern
4bcea04803 Merge "Implement pthread_mutex_t using SRWLOCK on Windows" into main 2025-08-06 19:02:21 -07:00
James Zern
6913049d6e Merge "Fix universal macOS binaries builds" into main 2025-08-06 18:42:19 -07:00
Wan-Teh Chang
6680723e66 Implement pthread_mutex_t using SRWLOCK on Windows
SRWLOCK is faster than CRITICAL_SECTION. MSVC implements the C11 mtx_t
and C++ std::mutex using SRWLOCK.

Change-Id: I144d2f53aeaa135030ea8c057a2ae8543e6c91b8
2025-08-06 18:27:58 -07:00