Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: James Almer <jamrial@gmail.com>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH] avutil/mem: always align by at least 32 bytes
Date: Wed, 6 Dec 2023 09:31:31 -0300
Message-ID: <0931e72c-9cb3-47b3-8cd8-fb07c911a453@gmail.com> (raw)
In-Reply-To: <20231203201027.2255-1-timo@rothenpieler.org>

On 12/3/2023 5:10 PM, 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.

Wont we run into similar issues with avx512 eventually?

> 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.
> 
> Instead of now going through all instances of variables in structs
> being declared as 32 byte aligned, this patch bumps the minimum alignment
> to 32 bytes.
> ---
>   libavutil/mem.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavutil/mem.c b/libavutil/mem.c
> index 36b8940a0c..26a9b9753b 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_AVX512 ? 64 : 32)
>   
>   /* NOTE: if you want to override these functions with your own
>    * implementations (not recommended) you have to link libav* as
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

  parent reply	other threads:[~2023-12-06 12:31 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-03 20:10 Timo Rothenpieler
2023-12-06 12:27 ` Timo Rothenpieler
2023-12-06 12:31 ` James Almer [this message]
2023-12-06 12:56   ` Timo Rothenpieler
2023-12-06 12:50 ` Ronald S. Bultje
2023-12-06 12:54   ` James Almer
2023-12-06 13:25 ` Martin Storsjö
2023-12-06 13:27   ` Timo Rothenpieler
2023-12-06 13:29     ` Martin Storsjö
2023-12-08  0:15   ` Timo Rothenpieler
2023-12-08  5:57     ` Martin Storsjö
2023-12-08 10:01 ` Andreas Rheinhardt
2023-12-08 17:56   ` Timo Rothenpieler
2023-12-08 18:11     ` Nicolas George
2023-12-09  5:23     ` Andreas Rheinhardt
2024-01-12 23:10       ` Timo Rothenpieler
2024-01-13  0:57         ` [FFmpeg-devel] [PATCH] avutil/mem: limit alignment to maximum simg align Timo Rothenpieler
2024-01-13  1:00           ` Timo Rothenpieler
2024-01-13 15:24           ` Timo Rothenpieler
2024-01-13 15:46           ` [FFmpeg-devel] [PATCH v2] avutil/mem: limit alignment to maximum simd align Timo Rothenpieler
2024-02-09 19:22             ` Timo Rothenpieler
2024-02-11 14:05               ` Sam James
2024-02-11 14:22               ` Rémi Denis-Courmont
2024-02-11 15:47                 ` Timo Rothenpieler
2024-02-11 14:00             ` Andreas Rheinhardt
2024-02-11 16:06               ` Timo Rothenpieler
2024-02-11 17:40               ` [FFmpeg-devel] [PATCH] " Timo Rothenpieler
2024-02-26 16:58                 ` Timo Rothenpieler
2024-02-27 18:45                   ` Timo Rothenpieler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0931e72c-9cb3-47b3-8cd8-fb07c911a453@gmail.com \
    --to=jamrial@gmail.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git