From patchwork Sat Jan 13 00:57:16 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Timo Rothenpieler X-Patchwork-Id: 45590 Delivered-To: ffmpegpatchwork2@gmail.com Received: by 2002:a05:6a21:8199:b0:199:de12:6fa6 with SMTP id pd25csp201855pzb; Fri, 12 Jan 2024 16:57:37 -0800 (PST) X-Google-Smtp-Source: AGHT+IGeYbBA6vVgV6nBu8vfkpORt+h8uu8cmHr5Wi1xR4KeBWAygSDWC2AyXJY+BWy5lG7Qb3rU X-Received: by 2002:aa7:ca41:0:b0:557:c2b5:1e4f with SMTP id j1-20020aa7ca41000000b00557c2b51e4fmr1128349edt.67.1705107457447; Fri, 12 Jan 2024 16:57:37 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1705107457; cv=none; d=google.com; s=arc-20160816; b=sfZyXGO4uVxfEhX+jxmt9qUR0Nh2XoCMy4bjyfnLjyRq8PDTFz99t46Vw8Xom2FxNh SzH++XfE/8WvpeESkKM9nz3KH16i8rFSzGK0k9mCx4QUcxI6Fgnyhj2UAx5DhakutvJh fRJ901TcOBEP+dnMWEIj7WoLU50kbcbrRAFAJOv3Hf8F6pBlJqzw3hUtz1XVAF47Wp1j 0v82uli1bk2BumlMemA0iy6iQCh5zCylFXlnvgeodyQwR1VVstUpCg1xjcBWzbVxBi9K wbJUObvin94CZlT8FXryBJpsVGJzeayDURq44tn7zdO6cQZbzoWmTacvgOxNzz+3XQUx n48A== 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=1Uh91/4PCdRDW2qExiTPBjhduGW4IZnHYVkICZWa/tI=; fh=LnlYe9qYwgML7nWWXqAumr7YCmPjjpEPjQf6GasgJC0=; b=JFSQ0DHsXDVkop6FP6xHSLotMAxibulAC7MkSPLsuP5vj0Z406fVaqfL35Kp/mN9bq GBfXlXTA7+Lym7n6fq1UwY23sfUIOMTikvOYiziEQ19CHQs3As9v9IrzDXoefP4jx621 x1hR8GunerYLh27kKH+AT853vjhN0OM8A2Ks2YDW20laABCGbfrcIJ7NywpDz3MulyiL Jk2yqI/JMevyDpeE8dClP3JL2wPn9RCuF7350HXTUKNe7LQSUkHSh36ntMB30Tx3eRYP 4lIHxlfW7sozdGJOy7I1QPVu7bJQczGm9i7Lles4ltBOMX6FsEIMndMucGxjZJYOem7F DmIw== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@rothenpieler.org header.s=mail header.b=LJOj28Nk; 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 di19-20020a056402319300b00557917bfe9asi1812878edb.401.2024.01.12.16.57.36; Fri, 12 Jan 2024 16:57:37 -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=LJOj28Nk; 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 43B6868CCA8; Sat, 13 Jan 2024 02:57:33 +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 5E98568CCA8 for ; Sat, 13 Jan 2024 02:57:26 +0200 (EET) Received: from [authenticated] by btbn.de (Postfix) with ESMTPSA id 6064527FAA8DE; Sat, 13 Jan 2024 01:57:25 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rothenpieler.org; s=mail; t=1705107445; 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=5p/EIn3ysJ0DLh6QNa1f4xd41IixSd54tN0WCH4uC3c=; b=LJOj28Nk3bfMGmvYbl3PhoV6BdvheXy6smxTtf6fAAWb3RaTBmaAFQ4ZNvTjL0zXIhJxKw RnCwKaKdNLSpCRNn3q43qTjUyRdinW5R1W51kUKRBSmIBlHmPdSoBDjDe34stw+5TDBivE 6V41ozoqaWBCMFrb40pB113MVox4PtLei1bTiHuS6DT5+gLW7LesJ3k97y7tyjt368/XWu Aw08C4E/Q9SxDpOVyMKWWWkAPknQgNyb+qvf5JHSUYbNbOr+BwBRo347Zo7hlykUSip/AK uRuc/tkbE2GdSZKQjvJpamJAtmwCWmtBo6/OVYKZSM1S7BXsMA2VYGL2yI5ysw== From: Timo Rothenpieler To: ffmpeg-devel@ffmpeg.org Date: Sat, 13 Jan 2024 01:57:16 +0100 Message-Id: <20240113005716.16018-1-timo@rothenpieler.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <9ecc9fe5-d75b-41f7-8476-2e69c58951fd@rothenpieler.org> References: <9ecc9fe5-d75b-41f7-8476-2e69c58951fd@rothenpieler.org> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH] avutil/mem: limit alignment to maximum simg 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: 2jkBVsGAIOv4 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 | 2 +- libavutil/mem_internal.h | 20 +++++++++++--------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/libavutil/mem.c b/libavutil/mem.c index 36b8940a0c..62163b4cb3 100644 --- a/libavutil/mem.c +++ b/libavutil/mem.c @@ -62,7 +62,7 @@ void free(void *ptr); #endif /* MALLOC_PREFIX */ -#define ALIGN (HAVE_AVX512 ? 64 : (HAVE_AVX ? 32 : 16)) +#define ALIGN (HAVE_SIMD_ALIGN_64 ? 64 : (HAVE_SIMD_ALIGN_32 ? 32 : 16)) /* 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..ddd3c24806 100644 --- a/libavutil/mem_internal.h +++ b/libavutil/mem_internal.h @@ -75,22 +75,24 @@ * @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 - #define DECLARE_ASM_CONST(n,t,v) __declspec(align(n)) static const t v + #define DECLARE_ALIGNED(n,t,v) __declspec(align(FFMIN(n, MAX_ALIGNMENT))) t v + #define DECLARE_ASM_ALIGNED(n,t,v) __declspec(align(FFMIN(n, MAX_ALIGNMENT))) t v + #define DECLARE_ASM_CONST(n,t,v) __declspec(align(FFMIN(n, MAX_ALIGNMENT))) static const t v #else #define DECLARE_ALIGNED(n,t,v) t v #define DECLARE_ASM_ALIGNED(n,t,v) t v