[FFmpeg-devel] [PATCH] avcodec/svq1enc: Workaround GCC bug 102513

[J. Dekker]

On 25 Oct 2022, at 14:48, Andreas Rheinhardt wrote:

> GCC 11 has a bug: When it creates clones of recursive functions
> (to inline some parameters), it clones a recursive function
> eight times by default, even when this exceeds the recursion
> depth. This happens with encode_block() in libavcodec/svq1enc.c
> where a parameter level is always in the range 0..5;
> but GCC 11 also creates functions corresponding to level UINT_MAX
> and UINT_MAX - 1 (on -O3; -O2 is fine).
>
> Using such levels would produce undefined behaviour and because
> of this GCC emits bogus -Warray-bounds warnings for these clones.
>
> Since commit d08b2900a9f0935959303da668cb00a8a7245228, certain
> symbols that are accessed like ff_svq1_inter_multistage_vlc[level]
> are declared with hidden visibility, which allows compilers
> to bake the offset implied by level into the instructions
> if level is a compile-time constant as it is in the clones.
> Yet this leads to insane offsets for level == UINT_MAX which
> can be incompatible with the supported offset ranges of relocations.
> This happens in the small code model (the default code model for
> AArch64).
>
> This commit therefore works around this bug by disabling cloning
> recursive functions for GCC 10 and 11. GCC 10 is affected by the
> underlying bug (see
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102513), so the workaround
> also targets it, although it only produces three versions of
> encode_block(), so it does not seem to trigger the actual issue here.
>
> The issue has been mitigated in GCC 12.1 (it no longer creates clones
> for impossible values; see also commit
> 1cb7fd317c84117bbb13b14851d62f77f57bb9ce), so the workaround
> does not target it.
>
> Reported-by: Josh Dekker <josh at itanimul.li>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> ---
>  libavcodec/svq1enc.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/libavcodec/svq1enc.c b/libavcodec/svq1enc.c
> index 75adbe7ea0..7c9430a137 100644
> --- a/libavcodec/svq1enc.c
> +++ b/libavcodec/svq1enc.c
> @@ -46,6 +46,12 @@
>  #include "libavutil/frame.h"
>  #include "libavutil/mem_internal.h"
>
> +// Workaround for GCC bug 102513
> +#if AV_GCC_VERSION_AT_LEAST(10, 0) && AV_GCC_VERSION_AT_MOST(12, 0) \
> +    && !defined(__clang__) && !defined(__INTEL_COMPILER)
> +#pragma GCC optimize ("no-ipa-cp-clone")
> +#endif
> +
>  typedef struct SVQ1EncContext {
>      /* FIXME: Needed for motion estimation, should not be used for anything
>       * else, the idea is to make the motion estimation eventually independent

Discussed on IRC, LGTM & pushed.

-- 
jd