Skip to content

fix: resolve alignment violations #3054

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
uwezkhan wants to merge 1 commit into from
Closed

fix: resolve alignment violations #3054

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions include/avif/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

#include "avif/avif.h" // IWYU pragma: export

#include <string.h> // For memcpy, used by avifLoadU16/avifStoreU16.

#ifdef __cplusplus
extern "C" {
#endif
Expand All @@ -19,6 +21,22 @@ extern "C" {
#define AVIF_MIN(a, b) (((a) < (b)) ? (a) : (b))
#define AVIF_MAX(a, b) (((a) > (b)) ? (a) : (b))

// Alignment-safe load/store for 16-bit values from potentially unaligned byte pointers.
Copy link
Copy Markdown
Collaborator

@wantehchang wantehchang Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for reporting this issue. The libavi API assumes that the uint8_t * buffers, if they contain high bit depth (> 8) pixels, point to uint16_t arrays, so that it is safe to cast the uint8_t * buffers for high bit depth pixels to uint16_t *. This is an API contract.

I did not check this pull request in its entirety (because it is very long). Under this API contract, I believe none of the changes in this pull request are needed. Could you please go over your pull request and see if there are any changes that should be kept? Thanks.

Copy link
Copy Markdown
Collaborator

@wantehchang wantehchang Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can consider checking this API contract on entry to the relevant functions.

avifImage structs are managed by libavif, so we can safely assume their pixels buffers are aligned.

The pixels buffers in avifRGBImage structs may be allocated by libavif, but they can also point to buffers allocated by the caller. So I think it suffices to validate that the pixels buffers in avifRGBImage structs are aligned.

But this is an extremely unlikely issue in practice because it would be inconvenient to the callers themselves if they allocated unaligned buffers for high bit depth pixels.

// Using memcpy avoids undefined behavior from casting uint8_t* to uint16_t* without
// alignment guarantees (C standard §6.3.2.3). Modern compilers optimize these to a single
// load/store instruction on platforms that support unaligned access (e.g., x86/x64).
Copy link
Copy Markdown
Collaborator

@wantehchang wantehchang Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is correct. libavif takes care to use memcpy for unaligned integers. You can search for "memcpy" in src/stream.c to find the memcpy calls used to read or write unaligned 16-bit, 32-bit, or 64-bit integers.

static inline uint16_t avifLoadU16(const uint8_t * p)
{
uint16_t v;
memcpy(&v, p, sizeof(v));
return v;
}

static inline void avifStoreU16(uint8_t * p, uint16_t v)
{
memcpy(p, &v, sizeof(v));
}

// Used for debugging. Define AVIF_BREAK_ON_ERROR to catch the earliest failure during encoding or decoding.
#if defined(AVIF_BREAK_ON_ERROR)
static inline void avifBreakOnError()
Expand Down
140 changes: 70 additions & 70 deletions src/alpha.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ void avifFillAlpha(const avifAlphaParams * params)
for (uint32_t j = 0; j < params->height; ++j) {
uint8_t * dstPixel = dstRow;
for (uint32_t i = 0; i < params->width; ++i) {
*((uint16_t *)dstPixel) = maxChannel;
avifStoreU16(dstPixel, maxChannel);
dstPixel += params->dstPixelBytes;
}
dstRow += params->dstRowBytes;
Expand Down Expand Up @@ -53,7 +53,7 @@ void avifReformatAlpha(const avifAlphaParams * params)
const uint8_t * srcPixel = srcRow;
uint8_t * dstPixel = dstRow;
for (uint32_t i = 0; i < params->width; ++i) {
*((uint16_t *)dstPixel) = *((const uint16_t *)srcPixel);
avifStoreU16(dstPixel, avifLoadU16(srcPixel));
srcPixel += params->srcPixelBytes;
dstPixel += params->dstPixelBytes;
}
Expand Down Expand Up @@ -90,11 +90,11 @@ void avifReformatAlpha(const avifAlphaParams * params)
const uint8_t * srcPixel = srcRow;
uint8_t * dstPixel = dstRow;
for (uint32_t i = 0; i < params->width; ++i) {
int srcAlpha = *((const uint16_t *)srcPixel);
int srcAlpha = avifLoadU16(srcPixel);
float alphaF = (float)srcAlpha / srcMaxChannelF;
int dstAlpha = (int)(0.5f + (alphaF * dstMaxChannelF));
dstAlpha = AVIF_CLAMP(dstAlpha, 0, dstMaxChannel);
*((uint16_t *)dstPixel) = (uint16_t)dstAlpha;
avifStoreU16(dstPixel, (uint16_t)dstAlpha);
srcPixel += params->srcPixelBytes;
dstPixel += params->dstPixelBytes;
}
Expand All @@ -110,7 +110,7 @@ void avifReformatAlpha(const avifAlphaParams * params)
const uint8_t * srcPixel = srcRow;
uint8_t * dstPixel = dstRow;
for (uint32_t i = 0; i < params->width; ++i) {
int srcAlpha = *((const uint16_t *)srcPixel);
int srcAlpha = avifLoadU16(srcPixel);
float alphaF = (float)srcAlpha / srcMaxChannelF;
int dstAlpha = (int)(0.5f + (alphaF * dstMaxChannelF));
dstAlpha = AVIF_CLAMP(dstAlpha, 0, dstMaxChannel);
Expand All @@ -137,7 +137,7 @@ void avifReformatAlpha(const avifAlphaParams * params)
float alphaF = (float)srcAlpha / srcMaxChannelF;
int dstAlpha = (int)(0.5f + (alphaF * dstMaxChannelF));
dstAlpha = AVIF_CLAMP(dstAlpha, 0, dstMaxChannel);
*((uint16_t *)dstPixel) = (uint16_t)dstAlpha;
avifStoreU16(dstPixel, (uint16_t)dstAlpha);
srcPixel += params->srcPixelBytes;
dstPixel += params->dstPixelBytes;
}
Expand Down Expand Up @@ -174,78 +174,78 @@ avifResult avifRGBImagePremultiplyAlpha(avifRGBImage * rgb)
if (rgb->format == AVIF_RGB_FORMAT_RGBA || rgb->format == AVIF_RGB_FORMAT_BGRA) {
uint8_t * row = rgb->pixels;
for (uint32_t j = 0; j < rgb->height; ++j) {
uint16_t * pixel = (uint16_t *)row;
uint8_t * pixel = row;
for (uint32_t i = 0; i < rgb->width; ++i) {
uint16_t a = pixel[3];
uint16_t a = avifLoadU16(&pixel[6]);
if (a >= max) {
// opaque is no-op
} else if (a == 0) {
// result must be zero
pixel[0] = 0;
pixel[1] = 0;
pixel[2] = 0;
avifStoreU16(&pixel[0], 0);
avifStoreU16(&pixel[2], 0);
avifStoreU16(&pixel[4], 0);
} else {
// a < maxF is always true now, so we don't need clamp here
pixel[0] = (uint16_t)avifRoundf((float)pixel[0] * (float)a / maxF);
pixel[1] = (uint16_t)avifRoundf((float)pixel[1] * (float)a / maxF);
pixel[2] = (uint16_t)avifRoundf((float)pixel[2] * (float)a / maxF);
avifStoreU16(&pixel[0], (uint16_t)avifRoundf((float)avifLoadU16(&pixel[0]) * (float)a / maxF));
avifStoreU16(&pixel[2], (uint16_t)avifRoundf((float)avifLoadU16(&pixel[2]) * (float)a / maxF));
avifStoreU16(&pixel[4], (uint16_t)avifRoundf((float)avifLoadU16(&pixel[4]) * (float)a / maxF));
}
pixel += 4;
pixel += 8;
}
row += rgb->rowBytes;
}
} else if (rgb->format == AVIF_RGB_FORMAT_ARGB || rgb->format == AVIF_RGB_FORMAT_ABGR) {
uint8_t * row = rgb->pixels;
for (uint32_t j = 0; j < rgb->height; ++j) {
uint16_t * pixel = (uint16_t *)row;
uint8_t * pixel = row;
for (uint32_t i = 0; i < rgb->width; ++i) {
uint16_t a = pixel[0];
uint16_t a = avifLoadU16(&pixel[0]);
if (a >= max) {
} else if (a == 0) {
pixel[1] = 0;
pixel[2] = 0;
pixel[3] = 0;
avifStoreU16(&pixel[2], 0);
avifStoreU16(&pixel[4], 0);
avifStoreU16(&pixel[6], 0);
} else {
pixel[1] = (uint16_t)avifRoundf((float)pixel[1] * (float)a / maxF);
pixel[2] = (uint16_t)avifRoundf((float)pixel[2] * (float)a / maxF);
pixel[3] = (uint16_t)avifRoundf((float)pixel[3] * (float)a / maxF);
avifStoreU16(&pixel[2], (uint16_t)avifRoundf((float)avifLoadU16(&pixel[2]) * (float)a / maxF));
avifStoreU16(&pixel[4], (uint16_t)avifRoundf((float)avifLoadU16(&pixel[4]) * (float)a / maxF));
avifStoreU16(&pixel[6], (uint16_t)avifRoundf((float)avifLoadU16(&pixel[6]) * (float)a / maxF));
}
pixel += 4;
pixel += 8;
}
row += rgb->rowBytes;
}
} else if (rgb->format == AVIF_RGB_FORMAT_GRAYA) {
uint8_t * row = rgb->pixels;
for (uint32_t j = 0; j < rgb->height; ++j) {
uint16_t * pixel = (uint16_t *)row;
uint8_t * pixel = row;
for (uint32_t i = 0; i < rgb->width; ++i) {
uint16_t a = pixel[1];
uint16_t a = avifLoadU16(&pixel[2]);
if (a >= max) {
// opaque is no-op
} else if (a == 0) {
// result must be zero
pixel[0] = 0;
avifStoreU16(&pixel[0], 0);
} else {
// a < maxF is always true now, so we don't need clamp here
pixel[0] = (uint16_t)avifRoundf((float)pixel[0] * (float)a / maxF);
avifStoreU16(&pixel[0], (uint16_t)avifRoundf((float)avifLoadU16(&pixel[0]) * (float)a / maxF));
}
pixel += 2;
pixel += 4;
}
row += rgb->rowBytes;
}
} else if (rgb->format == AVIF_RGB_FORMAT_AGRAY) {
uint8_t * row = rgb->pixels;
for (uint32_t j = 0; j < rgb->height; ++j) {
uint16_t * pixel = (uint16_t *)row;
uint8_t * pixel = row;
for (uint32_t i = 0; i < rgb->width; ++i) {
uint16_t a = pixel[0];
uint16_t a = avifLoadU16(&pixel[0]);
if (a >= max) {
} else if (a == 0) {
pixel[1] = 0;
avifStoreU16(&pixel[2], 0);
} else {
pixel[1] = (uint16_t)avifRoundf((float)pixel[1] * (float)a / maxF);
avifStoreU16(&pixel[2], (uint16_t)avifRoundf((float)avifLoadU16(&pixel[2]) * (float)a / maxF));
}
pixel += 2;
pixel += 4;
}
row += rgb->rowBytes;
}
Expand Down Expand Up @@ -361,84 +361,84 @@ avifResult avifRGBImageUnpremultiplyAlpha(avifRGBImage * rgb)
if (rgb->format == AVIF_RGB_FORMAT_RGBA || rgb->format == AVIF_RGB_FORMAT_BGRA) {
uint8_t * row = rgb->pixels;
for (uint32_t j = 0; j < rgb->height; ++j) {
uint16_t * pixel = (uint16_t *)row;
uint8_t * pixel = row;
for (uint32_t i = 0; i < rgb->width; ++i) {
uint16_t a = pixel[3];
uint16_t a = avifLoadU16(&pixel[6]);
if (a >= max) {
// opaque is no-op
} else if (a == 0) {
// prevent division by zero
pixel[0] = 0;
pixel[1] = 0;
pixel[2] = 0;
avifStoreU16(&pixel[0], 0);
avifStoreU16(&pixel[2], 0);
avifStoreU16(&pixel[4], 0);
} else {
float c1 = avifRoundf((float)pixel[0] * maxF / (float)a);
float c2 = avifRoundf((float)pixel[1] * maxF / (float)a);
float c3 = avifRoundf((float)pixel[2] * maxF / (float)a);
pixel[0] = (uint16_t)AVIF_MIN(c1, maxF);
pixel[1] = (uint16_t)AVIF_MIN(c2, maxF);
pixel[2] = (uint16_t)AVIF_MIN(c3, maxF);
float c1 = avifRoundf((float)avifLoadU16(&pixel[0]) * maxF / (float)a);
float c2 = avifRoundf((float)avifLoadU16(&pixel[2]) * maxF / (float)a);
float c3 = avifRoundf((float)avifLoadU16(&pixel[4]) * maxF / (float)a);
avifStoreU16(&pixel[0], (uint16_t)AVIF_MIN(c1, maxF));
avifStoreU16(&pixel[2], (uint16_t)AVIF_MIN(c2, maxF));
avifStoreU16(&pixel[4], (uint16_t)AVIF_MIN(c3, maxF));
}
pixel += 4;
pixel += 8;
}
row += rgb->rowBytes;
}
} else if (rgb->format == AVIF_RGB_FORMAT_ARGB || rgb->format == AVIF_RGB_FORMAT_ABGR) {
uint8_t * row = rgb->pixels;
for (uint32_t j = 0; j < rgb->height; ++j) {
uint16_t * pixel = (uint16_t *)row;
uint8_t * pixel = row;
for (uint32_t i = 0; i < rgb->width; ++i) {
uint16_t a = pixel[0];
uint16_t a = avifLoadU16(&pixel[0]);
if (a >= max) {
} else if (a == 0) {
pixel[1] = 0;
pixel[2] = 0;
pixel[3] = 0;
avifStoreU16(&pixel[2], 0);
avifStoreU16(&pixel[4], 0);
avifStoreU16(&pixel[6], 0);
} else {
float c1 = avifRoundf((float)pixel[1] * maxF / (float)a);
float c2 = avifRoundf((float)pixel[2] * maxF / (float)a);
float c3 = avifRoundf((float)pixel[3] * maxF / (float)a);
pixel[1] = (uint16_t)AVIF_MIN(c1, maxF);
pixel[2] = (uint16_t)AVIF_MIN(c2, maxF);
pixel[3] = (uint16_t)AVIF_MIN(c3, maxF);
float c1 = avifRoundf((float)avifLoadU16(&pixel[2]) * maxF / (float)a);
float c2 = avifRoundf((float)avifLoadU16(&pixel[4]) * maxF / (float)a);
float c3 = avifRoundf((float)avifLoadU16(&pixel[6]) * maxF / (float)a);
avifStoreU16(&pixel[2], (uint16_t)AVIF_MIN(c1, maxF));
avifStoreU16(&pixel[4], (uint16_t)AVIF_MIN(c2, maxF));
avifStoreU16(&pixel[6], (uint16_t)AVIF_MIN(c3, maxF));
}
pixel += 4;
pixel += 8;
}
row += rgb->rowBytes;
}
} else if (rgb->format == AVIF_RGB_FORMAT_GRAYA) {
uint8_t * row = rgb->pixels;
for (uint32_t j = 0; j < rgb->height; ++j) {
uint16_t * pixel = (uint16_t *)row;
uint8_t * pixel = row;
for (uint32_t i = 0; i < rgb->width; ++i) {
uint16_t a = pixel[1];
uint16_t a = avifLoadU16(&pixel[2]);
if (a >= max) {
// opaque is no-op
} else if (a == 0) {
// prevent division by zero
pixel[0] = 0;
avifStoreU16(&pixel[0], 0);
} else {
float c1 = avifRoundf((float)pixel[0] * maxF / (float)a);
pixel[0] = (uint16_t)AVIF_MIN(c1, maxF);
float c1 = avifRoundf((float)avifLoadU16(&pixel[0]) * maxF / (float)a);
avifStoreU16(&pixel[0], (uint16_t)AVIF_MIN(c1, maxF));
}
pixel += 2;
pixel += 4;
}
row += rgb->rowBytes;
}
} else if (rgb->format == AVIF_RGB_FORMAT_AGRAY) {
uint8_t * row = rgb->pixels;
for (uint32_t j = 0; j < rgb->height; ++j) {
uint16_t * pixel = (uint16_t *)row;
uint8_t * pixel = row;
for (uint32_t i = 0; i < rgb->width; ++i) {
uint16_t a = pixel[0];
uint16_t a = avifLoadU16(&pixel[0]);
if (a >= max) {
} else if (a == 0) {
pixel[1] = 0;
avifStoreU16(&pixel[2], 0);
} else {
float c1 = avifRoundf((float)pixel[1] * maxF / (float)a);
pixel[1] = (uint16_t)AVIF_MIN(c1, maxF);
float c1 = avifRoundf((float)avifLoadU16(&pixel[2]) * maxF / (float)a);
avifStoreU16(&pixel[2], (uint16_t)AVIF_MIN(c1, maxF));
}
pixel += 2;
pixel += 4;
}
row += rgb->rowBytes;
}
Expand Down
Loading
Loading