Le perjantaina 9. helmikuuta 2024, 21.22.17 EET Timo Rothenpieler a écrit : > On 13.01.2024 16:46, Timo Rothenpieler wrote: > > FFmpeg has instances of DECLARE_ALIGNED(32, ...) in a lot of structs, > > which then end up heap-allocated. > > By declaring any variable in a struct, or tree of structs, to be 32 byte > > aligned, it allows the compiler to safely assume the entire struct > > itself is also 32 byte aligned. > > > > This might make the compiler emit code which straight up crashes or > > misbehaves in other ways, and at least in one instances is now > > documented to actually do (see ticket 10549 on trac). > > The issue there is that an unrelated variable in SingleChannelElement is > > declared to have an alignment of 32 bytes. So if the compiler does a copy > > in decode_cpe() with avx instructions, but ffmpeg is built with > > --disable-avx, this results in a crash, since the memory is only 16 byte > > aligned. > > > > Mind you, even if the compiler does not emit avx instructions, the code > > is still invalid and could misbehave. It just happens not to. Declaring > > any variable in a struct with a 32 byte alignment promises 32 byte > > alignment of the whole struct to the compiler. > > > > This patch limits the maximum alignment to the maximum possible simd > > alignment according to configure. > > While not perfect, it at the very least gets rid of a lot of UB, by > > matching up the maximum DECLARE_ALIGNED value with the alignment of heap > > allocations done by lavu. > > --- > > > > libavutil/mem.c | 8 +++++++- > > libavutil/mem_internal.h | 14 ++++++++------ > > 2 files changed, 15 insertions(+), 7 deletions(-) > > > > diff --git a/libavutil/mem.c b/libavutil/mem.c > > index 36b8940a0c..b5bcaab164 100644 > > --- a/libavutil/mem.c > > +++ b/libavutil/mem.c > > @@ -62,7 +62,13 @@ void free(void *ptr); > > > > #endif /* MALLOC_PREFIX */ > > > > -#define ALIGN (HAVE_AVX512 ? 64 : (HAVE_AVX ? 32 : 16)) > > +#if defined(_MSC_VER) > > +/* MSVC does not support conditionally limiting alignment. > > + Set minimum value here to maximum used throughout the codebase. */ > > +#define ALIGN (HAVE_SIMD_ALIGN_64 ? 64 : 32)
Not that I care whatsoever, but are we assuming that MSVC supports only x86? Otherwise, this conditional definition does not make much sense and seems very sketchy. In fact, I don't see the point in making this distinction at all (*unlike* below). -- レミ・デニ-クールモン http://www.remlab.net/ _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".