Skip to content

dsp/lossless: add -fbounds-safety annotations to predictor functions#18

Open
herdiyana256 wants to merge 1 commit into
webmproject:mainfrom
herdiyana256:fix/dsp-lossless-fbounds-safety
Open

dsp/lossless: add -fbounds-safety annotations to predictor functions#18
herdiyana256 wants to merge 1 commit into
webmproject:mainfrom
herdiyana256:fix/dsp-lossless-fbounds-safety

Conversation

@herdiyana256
Copy link
Copy Markdown

@herdiyana256 herdiyana256 commented May 15, 2026

This patch begins extending the -fbounds-safety annotations to src/dsp/lossless.c, following the pattern already established in src/dec/ and using the macro infrastructure in src/utils/bounds_safety.h.

Background

The -fbounds-safety adoption in libwebp started with src/dec/, where pointers are progressively being annotated with WEBP_SINGLE,WEBP_INDEXABLE, and WEBP_BIDI_INDEXABLE to make memory access semantics explicit and machine-verifiable. The src/dsp/ subsystem currently has no coverage under this model. This matters because dsp/ functions sit
directly on the pixel-processing hot path for VP8L lossless decoding they receive raw pointers derived from untrusted file data and are called in tight loops over image rows.

What this patch addresses

The first issue is the use of negative pointer offsets in seven VP8LPredictor*_C functions. Predictors 4, 6, 8, 10, 11, 12, and 13 all access top[-1], which refers to the pixel immediately to the left of the current position on the previous row. This access pattern is correct and intentional: these functions are never called when x == 0, so top[-1] is always within the allocated row buffer. The problem is that this invariant is entirely implicit. The function signatures declare top as a plain const uint32_t*, which gives the compiler and any bounds-checking toolchain no information about the valid access range. If a future change in the calling code in PredictorInverseTransform_C or
GENERATE_PREDICTOR_ADD were to violate the x > 0 precondition, the resulting out-of-bounds read would be silent and undetectable.

Marking top as WEBP_BIDI_INDEXABLE converts this implicit assumption into an explicit, verifiable constraint. When compiled with-DWEBP_SUPPORT_FBOUNDS_SAFETY on Apple Clang or a compatible toolchain, the runtime will trap any access outside the annotated bounds before it can produce memory corruption.

The left parameter is a simpler case: it is always accessed only as *left (a single dereference), never as left[n] for any n. WEBP_SINGLE makes this precise.

The second issue is in PredictorAdd1_C. This function reads out[-1] before the loop begins, to initialize the running left-neighbor value for delta decoding. The header comment in lossless.h has always documented this precondition explicitly:
"These Add/Sub functions expect upper[-1] and out[-1] to be readable."
But that comment has no enforcement mechanism. With WEBP_BIDI_INDEXABLE on out and upper, the type system now carries the same information that the comment carries, and the toolchain can verify it.

VP8LAddGreenToBlueAndRed_C and VP8LTransformColorInverse_C are annotated with WEBP_BIDI_INDEXABLE on src and dst. Both functions iterate over num_pixels elements using index-based access, and the annotation correctly describes the valid access range relative to the pointer.

Changes in detail

In src/dsp/lossless.h:

  • Added #include "src/utils/bounds_safety.h"
  • Updated VP8LPredictorFunc typedef: left takes WEBP_SINGLE, top takes
    WEBP_BIDI_INDEXABLE
  • Updated VP8LPredictorAddSubFunc typedef: in, upper, and out all take
    WEBP_BIDI_INDEXABLE (upper and out due to documented [-1] access;
    in is bidi-annotated for consistency since it is passed offset into
    the predictor as upper+x)
  • Updated VP8LProcessDecBlueAndRedFunc typedef: src and dst take
    WEBP_BIDI_INDEXABLE
  • Updated all twelve VP8LPredictor*_C declarations to match

In src/dsp/lossless.c:

  • Added #include "src/utils/bounds_safety.h"
  • Added a block comment before VP8LPredictor0_C explaining the annotation
    rationale for the entire predictor group
  • All fourteen VP8LPredictor*_C implementations updated; added inline
    comments on the specific lines performing top[-1] access to make the
    reason for WEBP_BIDI_INDEXABLE self-evident during code review
  • PredictorAdd0_C and PredictorAdd1_C updated; inline comment on the
    out[-1] read in Add1 explains why bidi access is needed
  • VP8LAddGreenToBlueAndRed_C and VP8LTransformColorInverse_C updated

Testing

The patch was built with the existing GCC-based CMake configuration (the build_asan/ target). All files compiled without warnings or errors. The annotations are zero-overhead no-ops in this configuration — they only activate when the code is compiled with -DWEBP_SUPPORT_FBOUNDS_SAFETY on a toolchain that supports __ptrcheck_abi extensions (Apple Clang with -fbounds-safety, or compatible).
No logic was changed. This is purely a type-level annotation patch.

Scope and follow-up

This PR covers src/dsp/lossless.c and its header. The SIMD specializations (lossless_sse2.c, lossless_sse41.c, lossless_avx2.c, lossless_neon.c) define their own versions of the PredictorAdd functions and will need matching annotations in follow-up work. The same applies to lossless_enc.c and the remaining files in src/dsp/.

Extend the ongoing -fbounds-safety adoption to src/dsp/lossless.c.
This is the first step toward annotating the src/dsp/ subsystem, which
currently has no bounds-safety coverage despite processing untrusted
image data.

Key changes:

VP8LPredictor* functions:
- 'left' is single-dereferenced (*left only) -> annotated WEBP_SINGLE
- 'top' is accessed bidirectionally (top[-1], top[0], top[1]) in
  predictors 4, 6, 8, 10, 11, 12, 13 -> annotated WEBP_BIDI_INDEXABLE

  The caller guarantees top[-1] is in bounds (never called on x==0),
  but without WEBP_BIDI_INDEXABLE this invariant is invisible to the
  compiler/sanitizer.

PredictorAdd0_C / PredictorAdd1_C:
- 'out' uses out[-1] as the left-neighbor pixel -> WEBP_BIDI_INDEXABLE
- 'upper' is passed as 'upper + x' to predictors that access top[-1]
  -> WEBP_BIDI_INDEXABLE

VP8LAddGreenToBlueAndRed_C / VP8LTransformColorInverse_C:
- src/dst arrays annotated WEBP_BIDI_INDEXABLE for indexed iteration

Update lossless.h typedefs (VP8LPredictorFunc, VP8LPredictorAddSubFunc,
VP8LProcessDecBlueAndRedFunc) and function declarations to match.

No functional changes. All annotations are no-ops without
-DWEBP_SUPPORT_FBOUNDS_SAFETY. Verified: GCC build passes clean.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant