From patchwork Sat Jan 13 15:46:00 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Timo Rothenpieler X-Patchwork-Id: 45592 Delivered-To: ffmpegpatchwork2@gmail.com Received: by 2002:a05:6a21:8199:b0:199:de12:6fa6 with SMTP id pd25csp509274pzb; Sat, 13 Jan 2024 07:46:20 -0800 (PST) X-Google-Smtp-Source: AGHT+IGb4ACvp/Bfl6BWXyvcxN/EQ5F82iNCD4zsf/VAvXOziJdUaicY6GxLl9NjUez1fek2TzTg X-Received: by 2002:a05:6512:2241:b0:50c:320:f159 with SMTP id i1-20020a056512224100b0050c0320f159mr871492lfu.7.1705160780606; Sat, 13 Jan 2024 07:46:20 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1705160780; cv=none; d=google.com; s=arc-20160816; b=XMSOqrBYR2WaARs/76YErpA6NfrM/zxfffKqfdnepTXG/FKDA0tUSuJV/C/vmubIla oOiPpetIc4cKqCcgiACos/XEsiPYbfLevVMXwEQMqmySocfzta73l70qday/xewBztfl 98Y0XwrqVgJnLXzVh6wHbrTC29j9UKiat+MwaSvOimIaHHQS6OfjXPdK00mmqWqUOji/ vfVEKu0IEkTiCdT9FponGWmL/nb96yOtdq52QPTrkSFot2qrgDK4uQlGiRIdwRb6Hdi0 un2t+vhRoRJ6niiYhQJCdm+DDuYh0d4LzTz0hk3UvuaPflGnq3HH1tU3Au60CMYTkj5H wSaw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:cc:reply-to :list-subscribe:list-help:list-post:list-archive:list-unsubscribe :list-id:precedence:subject:mime-version:references:in-reply-to :message-id:date:to:from:dkim-signature:delivered-to; bh=NFYFubiwA+Kj1nk6E4yfgACaBmYEpvJIFIoqlof1xDQ=; fh=LnlYe9qYwgML7nWWXqAumr7YCmPjjpEPjQf6GasgJC0=; b=qFmqrPjfLQ52z35ieNXp4CzX914GNWeNeKFfnMi9ikrPnJsDfcvyH5F2tOhaJqucoy tgsh0rd/7VgzbQ4Q9qc4Y/V6+X0QUi924DIvNkRPuO6KjZhS0waUB6QtjgVqwfLpsB4G Vpo/oyZVBZBZonT0d/h9jf5+hRf754qpls9BEu5znA/RJmHpAkkYDNDrRMx/yWj+Tn1P nBvLOYRsPAXgTmKhIy1fVlwyfQr9rt1wPL4rn1FWQKT1PG4IvucxSI7sU+OuZEF6hQeN P/D0ReFqnp+dvmBb8mXh1ZMHSL5fPB/1/294Y4waMOPZqxS6PJVOttxd8Rtf0hwtGi5J KoYA== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@rothenpieler.org header.s=mail header.b="QS0Pb4/r"; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=rothenpieler.org Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id pj10-20020a170906d78a00b00a2939eaf551si2273298ejb.595.2024.01.13.07.46.20; Sat, 13 Jan 2024 07:46:20 -0800 (PST) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@rothenpieler.org header.s=mail header.b="QS0Pb4/r"; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=rothenpieler.org Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id DC62968CF46; Sat, 13 Jan 2024 17:46:16 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from btbn.de (btbn.de [144.76.60.213]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 33B7668C1A5 for ; Sat, 13 Jan 2024 17:46:10 +0200 (EET) Received: from [authenticated] by btbn.de (Postfix) with ESMTPSA id 940722819FC21; Sat, 13 Jan 2024 16:46:07 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rothenpieler.org; s=mail; t=1705160767; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=s0QklDN4d0QsMtUOjsEQ/Xo84db5A1sNg+KAbRY6z/o=; b=QS0Pb4/rPRIF3YC80xOtb+f371eJwgHzxKhsII3fjXj2UUDw4+9zub1UiV7vQf/JgPOt/+ AdzMT3w/r7miibl8Cjrd0uvUKtwT9RP+qKP4K9pLAFigpcKDY7d4ej9KTn78bm09wODvly 5rXY0/jhvnUoOrixil3RjOvXedLsBzlQZiHWSS8WSH4olQTsvzXQyigHsTV5a3tlwBJBE0 2ZIDJrsiGFBM2JmyKsZnA8cU0R+NoukkYf/cacJPe0DgMmd7Npm+Gdv06vC9Dp4C5HOikh 3/o5cHuXzBXmjzx082vsQfajZGSUyTj7ihYLqO65/IW3QUZqaaz6rZnaU8Xhqw== From: Timo Rothenpieler To: ffmpeg-devel@ffmpeg.org Date: Sat, 13 Jan 2024 16:46:00 +0100 Message-Id: <20240113154600.23366-1-timo@rothenpieler.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240113005716.16018-1-timo@rothenpieler.org> References: <20240113005716.16018-1-timo@rothenpieler.org> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2] avutil/mem: limit alignment to maximum simd align X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Timo Rothenpieler Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" X-TUID: UQeSHLgPZ98n 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) +#else +#define ALIGN (HAVE_SIMD_ALIGN_64 ? 64 : (HAVE_SIMD_ALIGN_32 ? 32 : 16)) +#endif /* NOTE: if you want to override these functions with your own * implementations (not recommended) you have to link libav* as diff --git a/libavutil/mem_internal.h b/libavutil/mem_internal.h index 2448c606f1..e2911b5610 100644 --- a/libavutil/mem_internal.h +++ b/libavutil/mem_internal.h @@ -75,18 +75,20 @@ * @param v Name of the variable */ +#define MAX_ALIGNMENT (HAVE_SIMD_ALIGN_64 ? 64 : (HAVE_SIMD_ALIGN_32 ? 32 : 16)) + #if defined(__INTEL_COMPILER) && __INTEL_COMPILER < 1110 || defined(__SUNPRO_C) - #define DECLARE_ALIGNED(n,t,v) t __attribute__ ((aligned (n))) v - #define DECLARE_ASM_ALIGNED(n,t,v) t __attribute__ ((aligned (n))) v - #define DECLARE_ASM_CONST(n,t,v) const t __attribute__ ((aligned (n))) v + #define DECLARE_ALIGNED(n,t,v) t __attribute__ ((aligned (FFMIN(n, MAX_ALIGNMENT)))) v + #define DECLARE_ASM_ALIGNED(n,t,v) t __attribute__ ((aligned (FFMIN(n, MAX_ALIGNMENT)))) v + #define DECLARE_ASM_CONST(n,t,v) const t __attribute__ ((aligned (FFMIN(n, MAX_ALIGNMENT)))) v #elif defined(__DJGPP__) #define DECLARE_ALIGNED(n,t,v) t __attribute__ ((aligned (FFMIN(n, 16)))) v #define DECLARE_ASM_ALIGNED(n,t,v) t av_used __attribute__ ((aligned (FFMIN(n, 16)))) v #define DECLARE_ASM_CONST(n,t,v) static const t av_used __attribute__ ((aligned (FFMIN(n, 16)))) v #elif defined(__GNUC__) || defined(__clang__) - #define DECLARE_ALIGNED(n,t,v) t __attribute__ ((aligned (n))) v - #define DECLARE_ASM_ALIGNED(n,t,v) t av_used __attribute__ ((aligned (n))) v - #define DECLARE_ASM_CONST(n,t,v) static const t av_used __attribute__ ((aligned (n))) v + #define DECLARE_ALIGNED(n,t,v) t __attribute__ ((aligned (FFMIN(n, MAX_ALIGNMENT)))) v + #define DECLARE_ASM_ALIGNED(n,t,v) t av_used __attribute__ ((aligned (FFMIN(n, MAX_ALIGNMENT)))) v + #define DECLARE_ASM_CONST(n,t,v) static const t av_used __attribute__ ((aligned (FFMIN(n, MAX_ALIGNMENT)))) v #elif defined(_MSC_VER) #define DECLARE_ALIGNED(n,t,v) __declspec(align(n)) t v #define DECLARE_ASM_ALIGNED(n,t,v) __declspec(align(n)) t v