Runtime buffers' pointer/size arithmetic ignores type.lanes

[alexreinking]

Note: I, @alexreinking, found this bug on my own while investigating the Halide type system. I prompted Claude Code to help me write this report and produce the MREs, but the bug itself is my own discovery. I have read all the text below and stand by its findings.

Summary

halide_buffer_t's pointer/size arithmetic in HalideRuntime.h (begin_offset, end_offset, begin, end, size_in_bytes, address_of) silently assumes type.lanes == 1. None of them account for lanes, even though halide_type_t explicitly supports vector types (lanes > 1) and halide_type_t::bytes() is documented as "Size in bytes for a single element, even if width is not 1" — i.e. callers are expected to multiply by lanes themselves when they want a true per-value byte count.

The only place in HalideRuntime.h that does this multiplication correctly is the trace-packet accessor code (halide_trace_packet_t::func()/value(), which compute type.lanes * type.bytes()). Every halide_buffer_t method a normal user would call uses type.bytes() alone.

HalideBuffer.h (Halide::Runtime::Buffer<>) compounds this: the string "lanes" does not appear in the file at all. There is no assertion anywhere preventing construction of a buffer whose element type has lanes > 1, and Buffer<>::allocate() sizes its malloc() via the buggy size_in_bytes().

Practical trigger: hand-building a halide_buffer_t/Buffer<> with an explicit halide_type_t that has lanes > 1 — e.g. accidentally reusing a vectorized Expr::type() instead of calling .element_of() first, or intentionally packing N-channel data as a single vector-typed buffer element. Halide-compiled pipelines themselves shouldn't produce such buffers (vectorization is unrolled into real buffer dimensions during lowering), but nothing stops a user from doing it, and nothing documents that it's unsupported.

Impact

• size_in_bytes() under-reports the true footprint by exactly a factor of lanes.
• Buffer<>::allocate() under-allocates the backing heap memory by the same factor — a real, ASan-confirmed heap-buffer-overflow (MRE 2 below).
• address_of() is internally inconsistent with the above: it advances by only one lane's worth of bytes per logical element instead of a full vector's worth, so adjacent logical elements alias on top of each other (MRE 3 below).

All three MREs below need only HalideRuntime.h + HalideBuffer.h (no libHalide link required).

MRE 1 — size_in_bytes() under-reports by the lanes factor

// MRE 1: Halide::Runtime::Buffer<void> silently under-allocates when given
// a halide_type_t with lanes > 1, then under-reports its own size.
//
// halide_type_t::bytes() is documented as "Size in bytes for a single
// element, even if width is not 1" -- i.e. it deliberately does NOT
// multiply by lanes. The trace-packet code in HalideRuntime.h knows this
// and does `type.lanes * type.bytes()` wherever it needs a true per-value
// byte count (see halide_trace_packet_t::func()/value() usage).
//
// halide_buffer_t::begin()/end()/size_in_bytes()/address_of(), however,
// use `type.bytes()` alone. If dim[].stride/extent are meant to count
// logical elements (each of which is a `lanes`-wide vector, as the type
// implies), all four of these are wrong by a factor of `lanes` whenever
// lanes > 1. HalideBuffer.h never mentions "lanes" and does not guard
// against this anywhere, so Buffer<void>::allocate() (which calls
// size_in_bytes()) simply allocates too little memory.
#include "HalideBuffer.h"
#include <cstdio>

int main() {
    // A "buffer of 100 float32x4 vectors" -- e.g. someone wrapping packed
    // 4-channel pixel data as a single vector-typed element per pixel.
    const int lanes = 4;
    const int extent = 100;
    halide_type_t vec_float32x4(halide_type_float, 32, lanes);

    Halide::Runtime::Buffer<void> buf(vec_float32x4, {extent});

    const size_t expected_bytes = (size_t)extent * lanes * sizeof(float);
    const size_t reported_bytes = buf.size_in_bytes();

    printf("lanes=%d extent=%d\n", lanes, extent);
    printf("expected size_in_bytes (extent * lanes * bytes_per_lane) = %zu\n", expected_bytes);
    printf("actual   buf.size_in_bytes()                             = %zu\n", reported_bytes);
    printf("under-allocation factor: %.1fx\n", (double)expected_bytes / (double)reported_bytes);

    // buf.allocate() (called implicitly by the constructor above) used
    // size_in_bytes() to size the underlying malloc, so the *actual*
    // heap allocation backing buf.data() is also `reported_bytes`, not
    // `expected_bytes`. See MRE 2 for a sanitizer-visible overflow using
    // exactly this undersized allocation.
    return reported_bytes == expected_bytes ? 0 : 1;
}

Output:

lanes=4 extent=100
expected size_in_bytes (extent * lanes * bytes_per_lane) = 1600
actual   buf.size_in_bytes()                             = 400
under-allocation factor: 4.0x

MRE 2 — ASan-confirmed heap-buffer-overflow from under-allocation

// MRE 2: ASan-detectable heap-buffer-overflow caused by the same
// lanes-vs-bytes mismatch demonstrated in MRE 1.
//
// A user creates a Buffer<void> whose element type is a 4-wide float
// vector (halide_type_t(halide_type_float, 32, 4)) with 100 "pixels".
// From the buffer's own declared shape (100 elements) and type (4 lanes
// of 4-byte float each), the only sane amount of data to copy in is
// number_of_elements() * type.lanes() * type.bytes() == 100*4*4 == 1600
// bytes. HalideBuffer.h documents nothing to the contrary -- the string
// "lanes" does not appear anywhere in that file.
//
// But Buffer<void>::allocate() sized the underlying malloc() using
// halide_buffer_t::size_in_bytes(), which (see HalideRuntime.h) computes
// (end_offset() - begin_offset()) * type.bytes() -- omitting the lanes
// factor. So the real heap allocation is only 400 bytes: a 4x
// under-allocation, invisible until something writes or reads the
// buffer's full logical extent.
#include "HalideBuffer.h"
#include <cstdio>
#include <cstring>
#include <vector>

int main() {
    const int lanes = 4;
    const int extent = 100;
    halide_type_t vec_float32x4(halide_type_float, 32, lanes);

    Halide::Runtime::Buffer<void> buf(vec_float32x4, {extent});

    std::vector<float> src(extent * lanes, 1.0f);

    printf("buf.size_in_bytes() = %zu (actual heap allocation size)\n", buf.size_in_bytes());
    printf("writing %zu bytes (extent * lanes * sizeof(float)) into buf.data()...\n",
           src.size() * sizeof(float));

    // A completely unsurprising thing to do with a buffer whose declared
    // type has lanes=4 and whose shape says extent=100: fill it with its
    // full logical payload. This overflows the real (undersized) heap
    // allocation.
    memcpy(buf.data(), src.data(), src.size() * sizeof(float));

    printf("done (if you see this without ASan, corruption was silent)\n");
    return 0;
}

Compiled with -fsanitize=address, output:

==92857==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x617000000300 at pc 0x00010561abb4 bp 0x00016b00a650 sp 0x00016b009e00
WRITE of size 1600 at 0x617000000300 thread T0
    #0 0x00010561abb0 in __asan_memcpy+0x330 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3ebb0)
    #1 0x000104df4d20 in main mre2_heap_overflow.cpp:40
    #2 0x00018d873dfc in start+0x1b4c (dyld:arm64e+0x1fdfc)

0x617000000300 is located 0 bytes after 640-byte region [0x617000000080,0x617000000300)
allocated by thread T0 here:
    #0 0x00010561dff4 in __sanitizer_mz_memalign+0x80 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x41ff4)
    #1 0x00018da5e870 in _malloc_zone_memalign+0x128 (libsystem_malloc.dylib:arm64e+0x3a870)
    #2 0x000104df6d90 in Halide::Runtime::Buffer<void, -1, 4>::allocate(void* (*)(unsigned long), void (*)(void*)) HalideBuffer.h:918
    #3 0x000104df5cbc in Halide::Runtime::Buffer<void, -1, 4>::Buffer<void>(halide_type_t, int) HalideBuffer.h:986
    #4 0x000104df4fcc in Halide::Runtime::Buffer<void, -1, 4>::Buffer<void>(halide_type_t, int) HalideBuffer.h:975
    #5 0x000104df4c34 in main mre2_heap_overflow.cpp:28
    #6 0x00018d873dfc in start+0x1b4c (dyld:arm64e+0x1fdfc)

SUMMARY: AddressSanitizer: heap-buffer-overflow mre2_heap_overflow.cpp:40 in main

(The frame at HalideBuffer.h:918 is the malloc/aligned_alloc call inside Buffer<>::allocate(), sized from size_in_bytes().)

MRE 3 — root cause is in plain-C halide_buffer_t, and address_of() aliases adjacent elements

No Halide::Runtime::Buffer involved — a hand-built halide_buffer_t, as an AOT-compiled pipeline entry point or a user's own C-ABI code would see it.

// MRE 3: Root cause lives in plain-C halide_buffer_t (HalideRuntime.h),
// not just in the C++ HalideBuffer.h wrapper. No Halide::Runtime::Buffer
// involved here -- just a hand-built halide_buffer_t, as a pipeline's
// AOT-generated `int (*)(halide_buffer_t*, ...)` entry point would see,
// or as a user wiring up their own buffers for the C ABI.
//
// This also shows that address_of() is internally inconsistent with
// size_in_bytes(): consecutive logical elements along a lanes>1 buffer
// alias on top of each other (address_of() advances by only
// type.bytes(), i.e. one lane's worth of bytes, per logical element),
// while size_in_bytes() (also missing the lanes factor) reports a
// span that is simultaneously too small to hold the real payload.
// Both bugs share the same missing `* type.lanes`.
#include "HalideRuntime.h"
#include <cstdio>
#include <cstdint>

int main() {
    const int lanes = 4;
    const int extent = 100;

    halide_dimension_t dim{/*min=*/0, /*extent=*/extent, /*stride=*/1};

    halide_buffer_t buf{};
    buf.type = halide_type_t(halide_type_float, 32, lanes);
    buf.dimensions = 1;
    buf.dim = &dim;
    // host deliberately left null: begin_offset()/end_offset() are
    // documented as safe pre-allocation; we only need offsets here.

    const size_t expected_span_bytes = (size_t)extent * lanes * sizeof(float);
    printf("type = float32x%d, extent = %d\n", lanes, extent);
    printf("expected total span in bytes (extent * lanes * bytes/lane) = %zu\n", expected_span_bytes);
    printf("halide_buffer_t::size_in_bytes()                           = %zu\n", buf.size_in_bytes());

    // address_of() aliasing: the byte distance it computes between
    // logical element 0 and logical element 1 should equal one full
    // vector's worth of bytes (lanes * bytes-per-lane) if lanes really
    // means "this many scalars travel together per logical position."
    // Instead it only advances by a single lane's worth of bytes.
    int pos0[1] = {0};
    int pos1[1] = {1};
    ptrdiff_t byte_stride_between_elements =
        (uint8_t *)buf.address_of(pos1) - (uint8_t *)buf.address_of(pos0);

    printf("address_of(elem 1) - address_of(elem 0) = %td bytes\n", byte_stride_between_elements);
    printf("expected (one full float32x%d vector)     = %zu bytes\n", lanes, lanes * sizeof(float));

    bool ok = (size_t)byte_stride_between_elements == lanes * sizeof(float) &&
              buf.size_in_bytes() == expected_span_bytes;
    return ok ? 0 : 1;
}

Output:

type = float32x4, extent = 100
expected total span in bytes (extent * lanes * bytes/lane) = 1600
halide_buffer_t::size_in_bytes()                           = 400
address_of(elem 1) - address_of(elem 0) = 4 bytes
expected (one full float32x4 vector)     = 16 bytes

Suggested fix

This depends on whether halide_buffer_t with type.lanes > 1 is meant to be a supported configuration at all. Either way, the status quo — silently wrong arithmetic with no documentation and no guard — should not remain.

1. If lanes > 1 is intended to be supported:

Fix the address and size calculations so they account for lanes everywhere a byte count or byte offset is derived from type.bytes(). Concretely, in HalideRuntime.h:

• halide_buffer_t::begin_offset() / end_offset(): these return offsets "in elements, not bytes" per their own doc comments, so they can stay as-is if every caller that converts them to bytes multiplies by type.bytes() * type.lanes instead of type.bytes() alone.
• halide_buffer_t::begin() / end() / size_in_bytes() / address_of(): each currently multiplies an element offset by type.bytes(); all of them need type.bytes() * type.lanes (or equivalently, an elem_size_in_bytes()-style helper that folds in lanes, used consistently) to match what halide_trace_packet_t::func()/value() already do correctly.
• HalideBuffer.h would need no changes beyond inheriting the corrected size_in_bytes()/address_of() behavior, since it does not duplicate this arithmetic itself — but it should probably gain a doc comment and/or test coverage confirming lanes > 1 buffers are supported, since none exists today.

2. If lanes > 1 is intended to be unsupported:

Remove lanes from halide_type_t entirely and move that information onto the trace packets directly, since trace packets are the only place in HalideRuntime.h that legitimately need to represent "N scalars of this type, packed together" independent of a halide_buffer_t's own shape. Concretely:

• Drop the lanes field from halide_type_t (or otherwise enforce lanes == 1 as an invariant, e.g. via a debug assertion in the constructor and in halide_buffer_t construction paths).
• Add an explicit lanes/width field directly to halide_trace_event_t / halide_trace_packet_t (or keep it as a separate parameter threaded through the tracing API) so tracing of vectorized values keeps working without halide_type_t itself needing to carry it.
• Document the lanes == 1 invariant on halide_buffer_t::type explicitly (it is not documented anywhere today), and consider a debug-mode assertion in halide_buffer_t-consuming code (or in HalideBuffer.h's Buffer<> constructors) that rejects lanes != 1 at construction time, so misuse fails loudly instead of silently corrupting memory.

Environment

• Reproduced against current main of halide/Halide
• macOS/arm64, Apple Clang, both with and without -fsanitize=address
• MREs only require HalideRuntime.h + HalideBuffer.h (no libHalide link needed): c++ -std=c++17 -I src/runtime mre1_size_in_bytes.cpp -o mre1 && ./mre1

[abadams]

Halide pipelines already reject lanes != 1 on buffer_ts. For the purpose of buffer_t, the lanes field is intentionally ignored, so the faulting code is erroneous. We should add assertions that lanes == 1 into Buffer methods too. Previous dev meeting discussions have favored deprecating that field entirely on halide_type_t, along the lines of option 2, mostly so we can support higher lane counts. The recent change to not use the lanes field in IRMatch.h was done partially for this purpose.

[alexreinking]

I definitely agree with removing the lanes field on the runtime type structure, leaving only tag+bits. It should be simple enough to keep that information on the trace structure (it's already de-structured for Call::trace)

When you say higher lane counts, you mean above 2^16? When has that come up in practice?

[abadams]

A single-Expr matmul of a 64x64 tile with another 64x64 tile is a vector reduce node of a 64x64x64-lane intermediate vector (which is never actually materialized).